classification
Title: urllib2 localnet Changed test to lookup IP-address of localhost
Type: behavior Stage: patch review
Components: Tests Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ezio.melotti, hakan, larstiq, puppet, r.david.murray
Priority: normal Keywords: patch

Created on 2014-10-28 14:47 by hakan, last changed 2014-11-03 11:58 by hakan.

Files
File name Uploaded Description Edit
issue_22753.diff hakan, 2014-10-28 14:51 A patch that does this: localhost_ip = socket.gethostbyname('localhost') review
Messages (6)
msg230156 - (view) Author: Håkan Lövdahl (hakan) * Date: 2014-10-28 14:47
Running this test gave me an error: 
./python -m test -v test_urllib2_localnet

======================================================================
ERROR: test_proxy_qop_auth_works (test.test_urllib2_localnet.ProxyAuthTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/hakan/kod/cpython/cpython/Lib/urllib/request.py", line 1182, in do_open
    h.request(req.get_method(), req.selector, req.data, headers)
  File "/home/hakan/kod/cpython/cpython/Lib/http/client.py", line 1154, in request
    self._send_request(method, url, body, headers)
  File "/home/hakan/kod/cpython/cpython/Lib/http/client.py", line 1192, in _send_request
    self.endheaders(body)
  File "/home/hakan/kod/cpython/cpython/Lib/http/client.py", line 1150, in endheaders
    self._send_output(message_body)
  File "/home/hakan/kod/cpython/cpython/Lib/http/client.py", line 988, in _send_output
    self.send(msg)
  File "/home/hakan/kod/cpython/cpython/Lib/http/client.py", line 923, in send
    self.connect()
  File "/home/hakan/kod/cpython/cpython/Lib/http/client.py", line 900, in connect
    self.timeout, self.source_address)
  File "/home/hakan/kod/cpython/cpython/Lib/socket.py", line 710, in create_connection
    raise err
  File "/home/hakan/kod/cpython/cpython/Lib/socket.py", line 701, in create_connection
    sock.connect(sa)
ConnectionRefusedError: [Errno 111] Connection refused

Changing "localhost" to "127.0.0.1" made the test pass. My friend puppet made a patch for it so it looks up the IP-address.
msg230157 - (view) Author: Håkan Lövdahl (hakan) * Date: 2014-10-28 15:01
Ubuntu Linux 12.04
64bit
msg230158 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-28 15:49
Your description of the solution ("change localhost to 127.0.0.1") does not match the content of the patch.  The patch appears to be looking up the IP address associated with 'localhost' and using that.  On most systems that would be 127.0.0.1, but perhaps on yours it is not.

I suspect your system has a configuration problem that has nothing to do with Python, but it isn't clear.

In any case the patch is incorrect, since one of the tests it is changing is specifically testing 127.0.0.1, from what I can see.
msg230451 - (view) Author: Wouter van Heyst (larstiq) * Date: 2014-11-01 15:52
What the patch does is replace the hard-coded address 127.0.0.1 for the loopback interface with a one-time lookup.  I'd argue the hunk @@ -315,7 +316,7 @@ should be dropped.

The two arguments that motivate this change are:

  1) The address of the loopback interface is not always 127.0.0.1, binding to it may fail.
  2) The tests seem to assume localhost resolves to the same address where the server is running.


Working on this patch started with the ConnectionRefusedError that Håkan mentioned. 
Applying the patch made the ConnectionRefusedError go away, but I think we may
have solved a different problem and suppressed the real problem as a side
effect.

@r.david.murray: would the patch be acceptable on dropping said hunk, and changing the description to 
"Do not assume localhost resolves to 127.0.0.1"?


That still leaves, I think, a hidden bug somewhere in the proxy code that will surface in some corner case.
msg230456 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-11-01 18:15
The problem is that there are several places in the test suite (and the stdlib itself!) that assume that 127.0.0.1 is a valid loopback address.  This is the de-facto Internet standard, and while I know not all systems make 127.0.0.1 a valid loopback and/or do not map localhost to it, I'm not sure this is something it is practical for the stdlib or the test suite to compensate for.  (I had this problem with linux-vserver on my buildbots, see issue 3972).  It is true that the test suite in in a couple places assumes that 'localhost' maps to 127.0.0.1, which is an assumption that breaks more often than that of 127.0.0.;1 being a a valid loopback, since it depends on the "correct" configuration of the local host.

We've got a catch 22: if we assume localhost maps to a valid loopback address, the tests will fail on systems that are not configured correctly, while if we assume that 127.0.0.1 is a valid loopback address but may not be 'localhost', the tests will fail on systems for which 127.0.0.1 is not a valid loopback.  Since the stdlib itself assumes that 127.0.0.1 is valid, the former wins.  (As far as I know we don't have any open issues against the stdlib for this assumption.  If it is problematic on your system, then you should open a separate issue about that so we can consider it.)

If this test fails on your system because 127.0.0.1 is not a loopback address, I would expect many other tests to fail as well (ex: test_ssl).  If they do not, then I suspect your real problem is that localhost doesn't map to 127.0.0.1 even though 127.0.0.1 is a valid loopback address.  So, it looks to me like the better "fix" here would be to change 'URL' to be 'http://127.0.0.1'.  Can you confirm if that works for you, please?
msg230534 - (view) Author: Håkan Lövdahl (hakan) * Date: 2014-11-03 11:58
Yes, I can confirm that changing 'URL' to be 'http://127.0.0.1' works. I think that would be a solution to this.
History
Date User Action Args
2014-11-03 11:58:35hakansetmessages: + msg230534
2014-11-01 18:15:41r.david.murraysetmessages: + msg230456
2014-11-01 15:52:54larstiqsetmessages: + msg230451
2014-10-28 15:49:17r.david.murraysetnosy: + r.david.murray
messages: + msg230158
2014-10-28 15:01:56hakansetmessages: + msg230157
2014-10-28 14:51:35hakansetfiles: + issue_22753.diff
keywords: + patch
2014-10-28 14:50:11ezio.melottisetnosy: + ezio.melotti, larstiq

type: behavior
stage: patch review
2014-10-28 14:47:35puppetsetnosy: + puppet
2014-10-28 14:47:08hakancreate