Patchwork do not replace the request header names with lower case

login
register
mail settings
Submitter domruf
Date May 22, 2014, 10:27 a.m.
Message ID <1400754437265-4010751.post@n3.nabble.com>
Download mbox | patch
Permalink /patch/4849/
State Changes Requested
Headers show

Comments

domruf - May 22, 2014, 10:27 a.m.
# HG changeset patch
# User domruf <dominikruf@gmail.com>
# Date 1400699187 -7200
#      Wed May 21 21:06:27 2014 +0200
# Branch stable
# Node ID afc72aa3019ecd07f947ccfb8dca6ae20ada06dd
# Parent  54d7657d7d1e6a62315eea53f4498657e766bb60
do not replace the request header names with lower case

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"

                 h.putrequest('GET', req.get_selector(), **skipheaders)




--
View this message in context: http://mercurial.808500.n3.nabble.com/PATCH-do-not-replace-the-request-header-names-with-lower-case-tp4010751.html
Sent from the Development mailing list archive at Nabble.com.
Mads Kiilerich - May 22, 2014, 5:52 p.m.
On 05/22/2014 12:27 PM, domruf wrote:
> # HG changeset patch
> # User domruf <dominikruf@gmail.com>
> # Date 1400699187 -7200
> #      Wed May 21 21:06:27 2014 +0200
> # Branch stable
> # Node ID afc72aa3019ecd07f947ccfb8dca6ae20ada06dd
> # Parent  54d7657d7d1e6a62315eea53f4498657e766bb60
> do not replace the request header names with lower case
>
> 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"

The standards are very clear: the header names are case insensitive. 
This patch adds a bit to code size and readability and execution time 
and doesn't really make Mercurial "better". I do not see a compelling 
reason why we should want it ... unless you can convince us that the 
potential issue you mention is so common that it is a de facto standard.

/Mads
domruf - May 23, 2014, 11:03 a.m.
I see your point.
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?

BTW The web server where I struggle with this is github.
I try to make the hg-git plugin work with github via https



--
View this message in context: http://mercurial.808500.n3.nabble.com/PATCH-do-not-replace-the-request-header-names-with-lower-case-tp4010751p4010767.html
Sent from the Development mailing list archive at Nabble.com.

Patch

diff -r 54d7657d7d1e -r afc72aa3019e mercurial/keepalive.py
--- a/mercurial/keepalive.py	Mon May 05 16:54:15 2014 +0200
+++ b/mercurial/keepalive.py	Wed May 21 21:06:27 2014 +0200
@@ -329,19 +329,18 @@ 
         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())
         skipheaders = {}
         for n in ('host', 'accept-encoding'):
-            if n in headers:
+            if n in [hdr.lower() for hdr in headers]:
                 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 [hdr.lower() for hdr in headers]:
                     h.putheader('Content-type',
                                 'application/x-www-form-urlencoded')
-                if 'content-length' not in headers:
+                if 'content-length' not in [hdr.lower() for hdr in
headers]:
                     h.putheader('Content-length', '%d' % len(data))
             else: