[issue8572] httplib getheader() does not work as documented
New submission from Walter Woods :
Calling HTTPResponse.getheader('Testabnew', 'Default') throws an exception
rather than returning default, python 3.1.2. Problem code appears to be line
601 of client.py:
def getheader(self, name, default=None):
if self.headers is None:
raise ResponseNotReady()
-return ', '.join(self.headers.get_all(name, default))
Which should probably changed to:
+result = self.headers.get_all(name, None)
+if result:
+return ', '.join(result)
+else:
+return default
Not as elegant, but what appears to happen is if default is not an array, then
get_all returns it, and the string join fails with a TypeError.
--
messages: 104532
nosy: Walter.Woods
priority: normal
severity: normal
status: open
title: httplib getheader() does not work as documented
type: crash
versions: Python 3.1
___
Python tracker
<http://bugs.python.org/issue8572>
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Changes by Walter Woods : -- title: httplib getheader() does not work as documented -> httplib getheader() throws error instead of default ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: Good point. Yeah, I could do that within a week or two at the latest. Just attach a .patch here when done? And, what should the base be for the patch . . . the Lib/http directory? Never submitted anything to python before. -- ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: David: Your example tests specifically for a string, but what about just converting default to a string always if it's not a list? Otherwise the string join is just going to fail anyway (suppose an integer is passed, which is the case with couchdb-python). Wouldn't if not isinstance(default, list): default = [str(default)] Be safer? -- ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: couchdb-python makes http requests, yes. Passing in a number might be mildly inappropriate, BUT I'd like to point out that the default might be used to find out if the header is not set (hence its default value of None). It should not assume that a string is the proper input; any sentinel value should work (especially as python is dynamic, and the default parameter is not actually used for the function's logic). Which goes back to the original patch, unless a list or tuple is provided, in which case the list is comma joined for backwards compatibility as suggested by David. -- ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment:
Little more info:
The actual snippet from couchdb-python is:
if int(resp.getheader('content-length', sys.maxsize)) < CHUNK_SIZE:
Which should be valid python . . . calling int() on an integer should work, and
calling int() on a string expected to be an integer should work.
--
___
Python tracker
<http://bugs.python.org/issue8572>
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: Sorry I'm just getting back to this . . . Senthil, doesn't list(None) throw an exception? That was the whole problem with list()ing the default argument. And I don't think the problem should be fixed in email.message.Message.get_all() . . . that function works exactly as it says it should. Its behavior is consistent. This issue should not change that. And even WITH changing that function, the patch would still need to fix http.client.HTTPResponse.getheader(). Just check python 2.6, and it looks like that function works correctly. If a number is passed, it returns a number as the default. We'd be preserving backwards compatibility, not destroying it, by returning the default parameter unchanged in 3.X when the specified header does not exist. I'll try attaching a patch before too long. -- ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: Senthil, you are correct, I gave a bad example. However, try list()ing an integer :) -- ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: Relevant part from trunk/Lib/rfc822.py illustrating that returning default unchanged is the legacy/defined behavior: return self.dict.get(name.lower(), default) See attached patch for py3k. This preserves backwards compatibility (aside from the comma-joined list) and uses the default argument appropriately (consistently with the default argument for dict.get, for instance). On another note, should we be patching Python 2.7 as well, since RFC 2616 states that the 3.X behavior of a comma-joined list is appropriate (as cited by Senthil)? -- Added file: http://bugs.python.org/file17222/issue8572-alt.diff ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment:
And if you really think that the unfortunate behavior of
>>> getheader('a', ['a', 'b'])
'a, b'
Is desired, issue8572-unfortunate.diff retains that behavior. I think that is
counter-intuitive though, and it is doubtful that many would be relying on this
functionality, given that the default value of None throws an exception at the
moment.
--
Added file: http://bugs.python.org/file17223/issue8572-unfortunate.diff
___
Python tracker
<http://bugs.python.org/issue8572>
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: I'll add unit tests later if no one else gets to it first; and they have the same functionality, but I could change the default back to None and use is None if that would be clearer (it would). -- ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: Hi David, I like most of your patch (especially since it has unit tests), and if people like yourself are actually using the current functionality then that's fine, but one recommendation: why not change this line: if not headers or isinstance(headers, str): To also include the clause ``or getattr(headers, '__iter__', False)``. That way, other default values (such as numbers) would work as expected rather than throw an error. What do you think of that? -- ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8572] httplib getheader() throws error instead of default
Walter Woods added the comment: Sigh. I meant ``or not`` instead of ``or``. Clearly, not having an iterator would be a case to not iterate over the default :) -- ___ Python tracker <http://bugs.python.org/issue8572> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
