Make urllib.request.Request.has_header() etc case-insensitive

@facundobatista

BPO	2275
Nosy	@facundobatista, @jcea, @orsenthil, @merwok, @bitdancer, @vadmium, @jayvdb
Files	urllib2-cap-headers.diff: Patch proposal issue2275-py26.diff issue2275-py3k.diff issue2775.patch: Make .has_header() and .get_header() case-insensitive issue2775-problems.patch: Test to show some problems with issue2275-py26.diff insensitive-lookup.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/orsenthil'
closed_at = None
created_at = <Date 2008-03-11.21:22:19.568>
labels = ['type-feature', 'library']
title = 'Make urllib.request.Request.has_header() etc case-insensitive'
updated_at = <Date 2015-11-09.06:07:10.589>
user = 'https://bugs.python.org/frispete'

bugs.python.org fields:

activity = <Date 2015-11-09.06:07:10.589>
actor = 'martin.panter'
assignee = 'orsenthil'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2008-03-11.21:22:19.568>
creator = 'frispete'
dependencies = []
files = ['9658', '11023', '11024', '11886', '11887', '40985']
hgrepos = []
issue_num = 2275
keywords = ['patch']
message_count = 38.0
messages = ['63466', '64740', '64747', '64748', '64749', '64760', '64761', '66311', '69084', '69225', '69410', '69430', '69453', '69454', '69521', '69524', '69528', '69536', '69560', '69561', '69562', '70136', '70162', '70163', '70542', '70643', '71040', '71070', '71071', '75206', '124480', '124486', '126341', '126365', '175015', '175023', '254088', '254369']
nosy_count = 11.0
nosy_names = ['facundobatista', 'jjlee', 'jcea', 'orsenthil', 'frispete', 'BitTorment', 'eric.araujo', 'r.david.murray', 'Rui.Carmo', 'martin.panter', 'jayvdb']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue2275'
versions = ['Python 3.6']

The urllib2 behavior related to headers is - hmm - improvable.
It simply capitalize() the key, which leads to funny results like:
Accept-charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
while this is seemingly conforming to the specs, it's simply different
to every other implementation of such things..

And we can do better. How about:
--- /usr/lib/python/urllib2.py 2008-01-10 19:03:55.000000000 +0100
+++ urllib2.py 2008-03-11 21:25:33.523890670 +0100
@@ -261,13 +261,16 @@ class Request:
def is_unverifiable(self):
return self.unverifiable

+    def _cap_header_key(self, key):
+        return '-'.join((ck.capitalize() for ck in key.split('-')))
+
     def add_header(self, key, val):
         # useful for something like authentication
-        self.headers[key.capitalize()] = val
+        self.headers[self._cap_header_key(key)] = val

     def add_unredirected_header(self, key, val):
         # will not be added to a redirected request
-        self.unredirected_hdrs[key.capitalize()] = val
+        self.unredirected_hdrs[self._cap_header_key(key)] = val

     def has_header(self, header_name):
         return (header_name in self.headers or

I'm not happy with the _cap_header_key name, but you get the idea.
The patch is optimized to operate with maximum locality. It's also
attached.

I would be very grateful, if something similar could be applied.

Opinions?

I understand your implementation of _cap_header_key function.
But should not this patch be handled in a way wherein.
key.capitalize() is just replaced by key.upper()?

Should not that suffice?
What is the difference between _cap_header_key and key.upper()?

Thank you,
Senthil

But should not this patch be handled in a way wherein.
key.capitalize() is just replaced by key.upper()?

Hmm, are you sure?
>>> "hello".upper()
'HELLO'
>>> 

but the issue is with values containing dashes:
>>> 'accept-charset'.capitalize()
'Accept-charset'
whereas the rest of the world would expect:
'Accept-Charset'
        ^

This is especially useful, if you try to emulate the behavior of a
typical browser as close as possible.

urllib2.Request.headers is, in practice, an undocumented public
interface. Did you run the tests? There is room for improvement here,
but not in the way you suggest.

python[1]$ python2.6
iPython 2.6a1+ (trunk:62045M, Mar 30 2008, 03:07:23) 
[GCC 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import test.test_urllib2
>>> print test.test_urllib2.test_request_headers_dict.__doc__

The Request.headers dictionary is not a documented interface.  It should
stay that way, because the complete set of headers are only accessible
through the .get_header(), .has_header(), .header_items() interface.
However, .headers pre-dates those methods, and so real code will be

using
the dictionary.

The introduction in 2.4 of those methods was a mistake for the same

reason:
code that previously saw all (urllib2 user)-provided headers in .headers
now sees only a subset (and the function interface is ugly and
incomplete).
A better change would have been to replace .headers dict with a dict
subclass (or UserDict.DictMixin instance?) that preserved the .headers
interface and also provided access to the "unredirected" headers. It's
probably too late to fix that, though.

Check .capitalize() case normalization:

    >>> url = "http://example.com"
    >>> Request(url, headers={"Spam-eggs": "blah"}).headers["Spam-eggs"]
    'blah'
    >>> Request(url, headers={"spam-EggS": "blah"}).headers["Spam-eggs"]
    'blah'

Currently, Request(url, "Spam-eggs").headers["Spam-Eggs"] raises

KeyError,
but that could be changed in future.

>>

Specifically, these improvements could be made:

the headers actually sent to httplib could be normalized to
Standard-Http-Case by urllib2
the urllib2.Request.headers interface could support case-insensitive
key lookup

Hi John, Greetings!
I agree with both of your suggestions.
Attached is the patch which aims to implement both in one go.
Please provide your comments on that.
If this method is okay, I shall go ahead with patches for tests and
attach it also.

Thanks,
Senthil

Hi Senthil,

that looks promising, and the title() trick is nice, as it fixes my
issue..

Thanks,
Pete

This looks good. I would suggest that the unredirected_hdrs would use
the CaseInsensitiveDict too.

There is still the problem (from the test documentation) that accessing
.headers directly will only show a subset of headers i.e. it won't show
any headers from .unredirected_hdrs. Have you any suggestions as to how
they both can be accessed from the same interface?

The test documentation also says:

"Note the case normalization of header names here, to
.capitalize()-case. This should be preserved for
backwards-compatibility. (In the HTTP case, normalization to
.title()-case is done by urllib2 before sending headers to
httplib)."

It suggests that capitalize() should be kept for backwards
compatibility. I have tested and the headers actually sent to the
server are in title()-case even though they are stored in the Request
object as captitalize()-case.

This would initially suggest that the case-normalization should be
removed from the patch. However, as .headers would now be using a
case-insensitive dictionary, this would still ensure backwards compatibily.

Issue applicable to Py2.6 and Py3K. Previous patch attached was wrong.
Removed it.

Senthil: We would need some tests to assure this will keep working ok in
the future

Also as this is (somehow) a new functionality, we'd need to modify the
NEWS file and maybe even the docs (a comment about this case insensitivity?

Could you please send a patch for these? Thank you!

Please have a look at this patch.

Included a CaseInsensitiveDict Lookup for Headers interface.
Headers will now be .title()-ed instead of .capitalized() ed.
Included Tests for the changes made.

In the test_urllib2, I have not removed this line (yet).
"The Request.headers dictionary is not a documented interface.."

I shall attach the patch to the documentation next. Will this suffice
to remove the declaration of "not a documented interface"?

Please provide your comments on the attached patch. If this is fine, I
shall do the same modifications for py3k and patch docs as well.

Thanks!

Senthil: patch is fine.

Remember to provide not only a modification for docs, but also to the
Misc/NEWS file.

Thank you!!

Title case is not the right thing for all headers: TE, WWW-Authenticate, etc.

While we're changing this, why not implement something to return headers in the order recommended in the RFC? (Probably another feature request)

Should one look into the source and make effort to submit a patch, or is
this being handled somehow?

It looks like there is a patch, so a patch review would be the next step (as indicated by the stage, not that we always remember to update that...)

Reading the issue carefully and making sure that all the concerns are addressed is also required. A comitter will ultimately have to do that, but anyone can do such a review and point out any issues, and that will help get this resolved sooner.

Rui: See bpo-12455 about changing the letter case of field names sent to the server. Unless someone wants to write a patch to fix all the bugs at once, I suggest to keep this thread focussed on Request.has_header() etc.

I closed bpo-5550 as a duplicate of this one. There is a patch there from 2014 which does the same sort of thing as John’s bpo-2775.patch from 2008, but IMO John’s patch would be more efficient. It needs updating for Python 3.6 though.

The remove_header() method (added in 3.4) should also be adjusted to be case-insensitive.

It may also be worth clarifying in the header_items() documentation that the keys have been converted as if by str.capitalize().

FTR: John’s bpo-2775.patch seems to be Patch Set 1 in the code review, despite being third in the list above.

Please review insensitive-lookup.patch:

Adapt John’s patch to current code
Apply similar changes to Request.remove_header()
Expanded on case-insensitivity, previous capitalize() requirements, and header_items() casing in the documentation
Dropped testing of unredirected_hdrs and of headers being case-sensitive. I think these are implementation details, and are not part of the implicit API from before the has_header() etc methods.
Reverted unnecessary test removal in test case
Added What’s New entry

update: yt-dlp-web-console

Navigation Menu

Repository navigation

Make urllib.request.Request.has_header() etc case-insensitive #46528

25 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Make urllib.request.Request.has_header() etc case-insensitive #46528

Description

Activity

frispete commented on Mar 11, 2008

orsenthil commented on Mar 30, 2008

frispete commented on Mar 30, 2008

jjlee commented on Mar 30, 2008

jjlee commented on Mar 30, 2008

orsenthil commented on Mar 30, 2008

frispete commented on Mar 30, 2008

BitTorment commented on May 6, 2008

orsenthil commented on Jul 2, 2008

facundobatista commented on Jul 3, 2008

orsenthil commented on Jul 8, 2008

facundobatista commented on Jul 8, 2008

25 remaining items

merwok commented on Jan 16, 2011

RuiCarmo commented on Nov 6, 2012

bitdancer commented on Nov 6, 2012

vadmium commented on Nov 5, 2015

vadmium commented on Nov 9, 2015

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions