classification
Title: http.client.HTTPMessage.getallmatchingheaders() always returns []
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.5, Python 3.4
process
Status: open Resolution:
Dependencies: 5054 Superseder:
Assigned To: Nosy List: berker.peksag, catalin.iacob, demian.brecht, ezio.melotti, ggenellina, martin.panter, mwatkins, orsenthil, petri.lehtinen, r.david.murray, v+python
Priority: normal Keywords: patch

Created on 2009-01-25 15:24 by mwatkins, last changed 2017-03-19 05:40 by v+python.

Files
File name Uploaded Description Edit
http.client.py.patch mwatkins, 2009-01-25 19:29 http.client.py.patch
remove-HTTPMessage.patch martin.panter, 2015-02-15 04:58 review
Messages (12)
msg80509 - (view) Author: Mike Watkins (mwatkins) Date: 2009-01-25 15:24
HTTPMessage.getallmatchingheaders() stopped working sometime after 
Python 3.0 release. In a recent (1 day ago) svn update the 
implementation says the method was copied from rfc822.message; the 
Python 3.x implementation is broken (iterates through self.keys instead 
of header values). I've not looked back to see where the change was 
introduced but I do know that it broke post 3.0 release.

But more importantly than the flaw in this method, the functionality is 
duplicated elsewhere in Python 3.x.

I propose either deprecating getallmatchingheaders() or if it is to be 
kept for compatibility, the fix can be simply replacing the method body 
with:

self.get_all(name)

The docstring for get_all (defined in email.message) affirms that its 
output is indeed compatible with that which was intended for 
getallmatchingheaders().

get_all(self, name, failobj=None) method of client.HTTPMessage instance
    Return a list of all the values for the named field.
    
    These will be sorted in the order they appeared in the original
    message, and may contain duplicates.  Any fields deleted and
    re-inserted are always appended to the header list.
    
    If no such fields exist, failobj is returned (defaults to None).

I've tested the use of get_all against one web server (QP) which runs on 
both Python 2.x and 3.x.

As a result of the broken getallmatchingheaders() the QP web server now 
uses for the same purpose as getallmatchingheaders() instead: 
get_all(name) (defined in email.message.Message in Py3k and 
getheaders(name) (defined in rfc822.Message).

See also issues on documentation and changed API in #4773, #3428
msg80529 - (view) Author: Mike Watkins (mwatkins) Date: 2009-01-25 19:29
Trivial patch for http.client attached.
msg80603 - (view) Author: Gabriel Genellina (ggenellina) Date: 2009-01-27 00:36
I think unified diffs are preferred.
Isn't there an existing test for this method?
msg80606 - (view) Author: Mike Watkins (mwatkins) Date: 2009-01-27 01:53
Re diffs, noted for the future. 
Re tests:

# py3k-devel/Lib/test % grep -r getallmatchingheaders *

... Returns nothing, so not only does the email package need a test for 
this but so does http.client. 

Incidentally test_mailbox.py has a test for the proposed alternative - 
get_all(), which I noted above. That's another good reason for ridding 
the world of getallmatchingheaders() or at least simply calling 
get_all() from within getallmatchingheaders() if compatibility is a 
legitimate concern.
msg80634 - (view) Author: Mike Watkins (mwatkins) Date: 2009-01-27 07:58
Further investigation ( grep -r getallmatchingheaders Lib/* ) reveals 
that in addition to having no tests, and being implemented incorrectly 
in http.client, getallmatchingheaders() is called only once, in 
http.server; that code is also broken (I reported this yesterday in  
#5053).

Maybe Python 3 is where getallmatchingheaders can make a graceful 
goodbye (and a 2to3 conversion added?).
msg148202 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-11-23 19:33
#13425 was marked as duplicate of this issue.
msg164790 - (view) Author: Catalin Iacob (catalin.iacob) * Date: 2012-07-07 08:23
So, how to move this further?

In #13425 Petri proposes 4 alternatives, copying them here:

1) Document the function to make it officially part of the public API
2) Rename and move the function to http.server
3) Leave it undocumented and just fix it
4) Deprecate the function to be removed in 3.4 or 3.5 and "fix" it to always return [].

I assume 4) meant:
4) Deprecate the function to be removed in 3.4 or 3.5 and fix to do what its docstring specifies.

My proposal is a more explicitly spelled out version 2):
5) Remove the function, replace its usage in http.server.CGIHTTPRequestHandler and add a test for http.server.CGIHTTPRequestHandler that exercises the part that currently uses getallmatchingheaders since that's obviously broken now.

The rationale for removal without deprecation is:
* the function duplicates get_all so there's no reason to have it
* it's probably not used by any (working) code because it just doesn't work

Mike can you tell us how you found out about this breakage? Were you using the function? Did you use something else to workaround it since it's broken now?

Senthil, Petri do you agree with option 5)? If so I can provide a patch.
msg164815 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-07-07 10:43
My 4) actually meant that it should always return []. This is what it currently does, so it could be spelled out clearly in the code.

IIRC, getallmatchingheaders() cannot be emulated one-to-one using get_all(), because it handles continuation lines differently. That's why I thought removing or deprecating without fixing it would be the best.

rfc822.Message.getallmatchingheaders() is documented in Python 2, so removing it could make it harder to port code from Python 2 to Python 3. On the other hand, it's broken, so having it removed could actually make things better by not introducing hard-to-find bugs.

All in all, I'm not sure what's the best thing to do.
msg164826 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-07-07 11:47
The CGIHTTPRequestHandler fix and test would be the best thing to start with, though, as it's not related to the eventual fate of getallmatchingheaders().
msg235798 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-02-12 05:11
I agree with Catalin’s proposal to remove it straight away. It just causes confusion, and to me the intended behaviour is not equivalent to Message.get_all(). I would remove the entire HTTPMessage class and replace it as an alias of email.message.Message.
msg236021 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-02-15 04:58
Posting a patch to remove the entire HTTPMessage class. This assumes my patch for Issue 5054 is accepted, which will remove the only existing reference to getallmatchingheaders().
msg289839 - (view) Author: Glenn Linderman (v+python) * Date: 2017-03-19 05:40
It is certainly true that getallmatchingheaders is broken... because the data it is looking at has changed format.

Here is a replacement that is as compatible as can be, based on the changed format.

        name = name.lower()
        n = len(name)
        lst = []
        for line, data in self.items():
            if line.lower() == name:
                lst.append(line + ': ' + data)
        return lst

The changed format has merged continuation lines, and separated keys and values into a list of duplet tuples. Iterators keys, values, and items exist, keys are not necessarily unique.
History
Date User Action Args
2017-03-19 05:40:00v+pythonsetnosy: + v+python
messages: + msg289839
2015-05-31 11:13:17martin.pantersetdependencies: + CGIHTTPRequestHandler.run_cgi() HTTP_ACCEPT improperly parsed
stage: needs patch -> patch review
2015-02-16 19:01:21demian.brechtsetnosy: + demian.brecht
2015-02-15 04:58:10martin.pantersetfiles: + remove-HTTPMessage.patch

messages: + msg236021
2015-02-13 01:23:44berker.peksagsetnosy: + berker.peksag
2015-02-12 06:00:07serhiy.storchakasetnosy: + r.david.murray

versions: + Python 3.4, Python 3.5, - Python 3.2, Python 3.3
2015-02-12 05:11:12martin.pantersetnosy: + martin.panter
messages: + msg235798
2012-07-07 11:47:46petri.lehtinensetmessages: + msg164826
stage: needs patch
2012-07-07 10:43:28petri.lehtinensetnosy: + ezio.melotti

messages: + msg164815
versions: + Python 3.3, - Python 3.1
2012-07-07 08:23:32catalin.iacobsetnosy: + catalin.iacob
messages: + msg164790
2011-11-23 19:33:25petri.lehtinensetmessages: + msg148202
2011-11-23 19:31:58petri.lehtinenlinkissue13425 superseder
2011-11-23 19:26:52petri.lehtinensetnosy: + petri.lehtinen
2010-06-26 02:21:35r.david.murraysetnosy: + orsenthil
2010-06-26 00:09:57terry.reedysetversions: + Python 3.2, - Python 3.0
2009-01-27 07:58:01mwatkinssetmessages: + msg80634
2009-01-27 01:53:56mwatkinssetmessages: + msg80606
2009-01-27 00:36:52ggenellinasetnosy: + ggenellina
messages: + msg80603
2009-01-26 13:51:32mwatkinssettitle: http.client.HTTPMessage.getallmatchingheaders() -> http.client.HTTPMessage.getallmatchingheaders() always returns []
2009-01-25 19:29:22mwatkinssetfiles: + http.client.py.patch
keywords: + patch
messages: + msg80529
2009-01-25 15:24:32mwatkinscreate