[Patches] [ python-Patches-1668132 ] Fix for 767111, 'AttributeError thrown by urllib.open_http'

[SourceForge.net]

Patches item #1668132, was opened at 2007-02-24 17:53
Message generated for change (Comment added) made by jseutter
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1668132&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Atul Varma (varmaa)
Assigned to: Nobody/Anonymous (nobody)
Summary: Fix for 767111, 'AttributeError thrown by urllib.open_http'

Initial Comment:
This is a fix for bug 767111:

http://sourceforge.net/tracker/index.php?func=detail&aid=767111&group_id=5470&atid=105470

I am also attaching a unit test that reliably reproduces the bug using an in-process web server.

I didn't want to modify httplib because it would mean altering its interface such that the HTTP.getfile() method would never return None, and that could break existing code.  So instead I modified urllib at a point that it appears to assume that HTTP.getfile() will always return a file object.  I hope this is a reasonable place to fix the problem, and I don't think it can break any existing code.  I also ran the existing "test/test_urllib.py" and "test/test_urllibnet.py" test suites and they both work fine with this patch.

Looking at the code, I also believe that a "parallel bug" exists with urllib.open_https(), and I fixed that as well (though I don't have a unit test to prove this).


----------------------------------------------------------------------

Comment By: Jerry Seutter (jseutter)
Date: 2007-03-06 00:34

Message:
Logged In: YES 
user_id=1727609
Originator: NO

Hi Atul,

I took a quick look at your patch and verified that the test case produces
an AttributeError on trunk.  After manually patching urllib.py the test
case passes.  A few observations:

1. Your patch changes the code from giving an AttributeError to giving an
empty file back when the remote webserver gives a bad http status line. 
The official Python documentation says that urlopen either generates an
IOError when it can't connect to the server, or returns a file-like object
when all goes well.  In this case all didn't go well, so the documentation
is incomplete.  Would it be more appropriate to raise an exception rather
than return an empty file?

2. I believe your test file will not work on systems without networking
support.  Other Python tests that require networking support have this at
the top of them:
test_support.requires('network')
See Lib/test/test_socketserver.py for example.

3. Consider merging your test into Lib/test/test_urllib.py and supplying a
patch against that file.

4. Could the test be performed without actually getting a network
connection?  Some of the tests in test_urllib.py use a FakeSocket class
that might also work for your test.

5. Whitespace police - foo(arg1) instead of foo( arg1 ).  See PEP 8 at
http://www.python.org/dev/peps/pep-0008/

Other than that, things look good.

----------------------------------------------------------------------

Comment By: Atul Varma (varmaa)
Date: 2007-02-24 17:56

Message:
Logged In: YES 
user_id=863202
Originator: YES

File Added: UrllibBug767111Tester.py

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1668132&group_id=5470