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

[Mads Kiilerich]

> 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.

> 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