[PATCH] do not replace the request header names with lower case

[Augie Fackler]

On Fri, May 23, 2014 at 04:34:41PM +0200, Mads Kiilerich wrote:
>
> >I see your point.
>
> mercurial-devel is a mailing list. nabble is terrible and very unofficial
> frontend for posting to it. 99% of the people posting through it violate the
> mailing list etiquette, for instance by not quoting properly. Nabble will
> probably also mangle some of the patches you send. We recommend using the
> patchbomb extension.
>
> >Since in the comment it says it is a reimplementation of
> >HTTPConnection.request() (respectifly HTTPConnection._send_request()( I
> >looked into that method and created a n alternative based on _send_request.
> >What do you think about it?
>
> The implementation here is better but I still don't buy it as a workaround
> for bugs in other systems. There could be other reasons to take it, though.
>
> >BTW The web server where I struggle with this is github.
> >I try to make the hg-git plugin work with github via https
>
> Do you have a link the issue you have filed at github? I assume they in
> general are clueful and wants to be standard compliant and will fix the
> issue ASAP. We should assume it will be fixed before this code could be
> released.
>
>
> On 05/23/2014 12:57 PM, domruf wrote:
> ># HG changeset patch
> ># User domruf <dominikruf at gmail.com>
> ># Date 1400792192 -7200
> >#      Thu May 22 22:56:32 2014 +0200
> ># Branch stable
> ># Node ID 023ef1daa66809071a1d2261bb548404e7560368
> ># Parent  54d7657d7d1e6a62315eea53f4498657e766bb60
> >do not replace the request header names with lower case
>
> See http://mercurial.selenic.com/wiki/ContributingChanges point 1.1 . The
> topic could be "http" or "keepalive".
>
> >If a webserver expects a header in a certain case the request might fail.
> >i.e. if a (not so well writen) authentication module only accepts
> >"Authorization" and fails with "authorization"
>
> It seems like you some words.
>
> If github is the reason you want to make the change, then you should mention
> it here. (But I don't think you should make the change for that reason and
> it is thus not relevant.)
>
> I would like the patch if you described it as something like:
>
> The code lowercased all header names that passed through it. HTTP headers
> are case insensitive so it was thus totally valid. But there is also no
> reason to change it unless we have to ... and it turns out that there is no
> reason to do that. Preserving the casing can for instance make debugging
> simpler. It might also make a difference for other buggy systems.

I'd be +1 on this change - this justification is obvious to me, but
I've spent more time than is healthy in RFC 2616. I eagerly await a
resend with that kind of documentation.

>
> >diff -r 54d7657d7d1e -r 023ef1daa668 mercurial/keepalive.py
> >--- a/mercurial/keepalive.py	Mon May 05 16:54:15 2014 +0200
> >+++ b/mercurial/keepalive.py	Thu May 22 22:56:32 2014 +0200
> >@@ -329,19 +329,19 @@
> >          if sys.version_info >= (2, 4):
> >              headers.update(req.unredirected_hdrs)
> >          headers.update(self.parent.addheaders)
> >-        headers = dict((n.lower(), v) for n, v in headers.items())
> >+        headersnames = dict((n.lower(), v) for n, v in headers.items())
>
> You do not use the values of the dict so a set would be more appropriate. (I
> would have a slight preference for a name like headernames or headerslow.)
>
> >          skipheaders = {}
> >          for n in ('host', 'accept-encoding'):
> >-            if n in headers:
> >+            if n in headersnames:
> >                  skipheaders['skip_' + n.replace('-', '_')] = 1
> >          try:
> >              if req.has_data():
> >                  data = req.get_data()
> >                  h.putrequest('POST', req.get_selector(), **skipheaders)
> >-                if 'content-type' not in headers:
> >+                if 'content-type' not in headersnames:
> >                      h.putheader('Content-type',
> >                                  'application/x-www-form-urlencoded')
> >-                if 'content-length' not in headers:
> >+                if 'content-length' not in headersnames:
> >                      h.putheader('Content-length', '%d' % len(data))
> >              else:
> >                  h.putrequest('GET', req.get_selector(), **skipheaders)
>
> /Mads
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel