Issue1697

classification
Title: Wrong error message when http connection can not be established
Type: Severity: normal
Components: Versions:
Milestone:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: amak Nosy List: alex.gronholm, amak, basti1302, otmarhumbel, pekka.klarck, pjenvey
Priority: Keywords:

Created on 2011-01-14.08:22:39 by basti1302, last changed 2011-02-11.14:12:24 by amak.

Files
File name Uploaded Description Edit Remove
jython-issue-1210-patch.txt basti1302, 2011-01-14.08:22:39 Simple patch for this issue
test.py basti1302, 2011-01-14.08:24:24 A simple test to reproduce the issue (change port and url to your liking)
1697-patch.txt otmarhumbel, 2011-01-21.00:51:41 proposed patch
1697-patch2.txt otmarhumbel, 2011-01-23.14:53:56 2nd patch, blocking IPv6 addresses from getaddrinfo()
Messages
msg6323 (view) Author: Bastian Krol (basti1302) Date: 2011-01-14.08:22:39
When trying to connect via httplib, like this

conn = httplib.HTTPConnection('localhost', 8080)
...
conn.request("GET", "/index.html", body, headers)

when there is nothing running on localhost:8080, a rather confusing error message is printed: 

"Only AF_INET sockets are currently supported on jython"

With the provided patch, the error message reads

"socket.error: (10061, 'Connection refused')"

which is what I would expect in this case.

IMHO when (whoever) decided to not support IPv6 he/she just overlooked this particular piece of code in httplib.py. As long as IPv6 is not supported, httplib should not use

socket.getaddrinfo(self.host, self.port, socket.AF_UNSPEC, socket.SOCK_STREAM)

(or socket.getaddrinfo(self.host, self.port, 0, socket.SOCK_STREAM) which is the same) 

but instead should use 
 
socket.getaddrinfo(self.host, self.port, socket.AF_INET, socket.SOCK_STREAM)

This is related to Issue 1210.
msg6324 (view) Author: Pekka Klärck (pekka.klarck) Date: 2011-01-15.10:58:37
Due to this bug, code trying to handle connection problems by catching socket.error doesn't work because AssertionError is risen instead. An example code affected by this problem is below:

while time.time() < MAX_TIME:
    try:
        conn = httplib.HTTPConnection('localhost', 8080)
    except socket.error:
        time.sleep(2)

In our code we worked around this by replacing `except socket.error` with `except (socket.error, AssertionError)`.
msg6329 (view) Author: Oti Humbel (otmarhumbel) Date: 2011-01-21.00:51:41
1697-patch.txt contains an alternative fix
msg6331 (view) Author: Bastian Krol (basti1302) Date: 2011-01-21.06:33:32
otmarhumbels patch is much better than mine.
msg6332 (view) Author: Philip Jenvey (pjenvey) Date: 2011-01-22.00:09:51
I'd rather not patch httplib to solve this, but the underlying issue is that our socket.getaddrinfo supports ipv6, but the rest of the socket module pretty much doesn't

Removing ipv6 support from getaddrinfo would solve this (that's kind of what Oti's patch is doing), but that functionality is already present in 2.5.1. So that's really a worse option than patching httplib
msg6337 (view) Author: Oti Humbel (otmarhumbel) Date: 2011-01-23.14:53:56
After some discussions on the IRC channel, the consensus was to block IPv6 addresses from socket.getaddrinfo().

This is reflected in 1697-patch2.txt
msg6338 (view) Author: Oti Humbel (otmarhumbel) Date: 2011-01-23.17:09:46
committed the second patch in revision 7189 and 7190
msg6339 (view) Author: Alan Kennedy (amak) Date: 2011-01-23.18:33:30
Sorry Oti, I think you've jumped the gun on this one. There are ways forward. Next time, please wait until I've had a chance to review any submissions: I assigned this bug to myself for a reason.
msg6340 (view) Author: Alan Kennedy (amak) Date: 2011-01-23.18:33:48
OK, as with all things socket-related, there is a complex picture here, which depends on operating system and version, java version, whether or not the host supports IPv6, and design of the jython socket module, which necessarily uses a mixture of java.net and java.nio calls to implement sockets which can have both synchronous and asynchronous behaviour.

One of the primary problems is this java bug, which effectively prevents the use of IPv6 in jython on Windows version older than Vista, i.e. it affects XP/2003.

NIO channels with IPv6 on Windows
http://bugs.sun.com/view_bug.do?bug_id=6230761

This bug affects jython sockets because of the way that of the way such sockets are created, i.e. using java.nio. This is necessary because it is the only way that jython sockets can have asynchronnous behaviour.

Another problem is that whatever behaviour jython sockets exhibit has to be testable on multiple platforms. See here for where I checked in a "test_socket_ipv6.py" module, which had to be disabled because the test results were too unreliable across platforms.

https://fisheye3.atlassian.com/changelog/jython/?cs=4949

I will write about workarounds in the following messages.
msg6341 (view) Author: Oti Humbel (otmarhumbel) Date: 2011-01-23.19:04:39
Alan,
i certainly did not want to make things worse than they already were.
The goal was to get 'some' fix to be able to release 2.5.2
msg6342 (view) Author: Alex Grönholm (alex.gronholm) Date: 2011-01-23.19:12:14
Surely we aren't supposed to support IPv6 on Windows earlier than Vista? Vista was the first Windows OS with "real" IPv6 support anyway. Supporting the half-assed IPv6 implementations on older Windows versions is not worth the effort IMO.
msg6343 (view) Author: Alan Kennedy (amak) Date: 2011-01-23.20:41:30
Oti, what's the timetable for the next release.

If you can give me a day or two, I'll get a better fix in.
msg6344 (view) Author: Philip Jenvey (pjenvey) Date: 2011-01-23.20:46:31
We were just aiming to get 2.5.2 out ASAP (it's been a while since 2.5.1, the 2.5.2 release cycle has taken a while, etc). We probably would have released it today if this ticket was resolved.

No problem waiting a couple more days for a better fix, though. If it takes longer than that, just keep in mind you are the sole 2.5.2 release blocker =]
msg6345 (view) Author: Oti Humbel (otmarhumbel) Date: 2011-01-23.20:54:01
Alan, sure take your days for a better fix - we appreciate your help!
msg6351 (view) Author: Philip Jenvey (pjenvey) Date: 2011-01-24.17:58:21
BTW this problem should also affect the {ftp,pop,smtp,telnet}lib modules. They all use AF_UNSPEC getaddrinfo results to create new sockets. The socket module also describes the same technique with a code sample
msg6356 (view) Author: Alan Kennedy (amak) Date: 2011-01-27.20:33:06
Has anyone had a chance to review my most recent commit on this issue. I think it is a better solution because

1. It re-enables the use of IPV6 where possible.
2. It catches the situations where IPV6 cannot be used safely.
3. Provides a workaround for such situations
4. Documents this as a known issue.

If people agree, I'll close this issue.
msg6357 (view) Author: Alan Kennedy (amak) Date: 2011-01-27.20:34:39
Should have mentioned, you can see that submission here

https://fisheye3.atlassian.com/changelog/jython/?cs=7191
msg6358 (view) Author: Alex Grönholm (alex.gronholm) Date: 2011-01-27.21:18:03
I'm not sure about this. Basically it requires developers to set a Jython specific flag. How will they know that a bizarre AssertionError requires setting this obscure flag to make it work? Could it at least be set True by default?
msg6359 (view) Author: Philip Jenvey (pjenvey) Date: 2011-01-28.00:48:04
Alex, it solves the bug by enabling creation of IPV6 sockets. That creates a new problem for the WinXP users with crappy ipv6 support: they'll eventually trigger a Java nio error about ipv6 not being supported. But Alan's having the socket module catch that error and translate it a socket.error exception with an error message pointing the user to our socket module docs on the wiki (where he's added mention of this issue and the workaround)

So the workaround added is intended for those WinXP users, it's a global switch that turns off IPV6 results from getaddrinfo

This all looks great Alan, thanks for taking care of it! My only question, does this mean we now have complete ipv6 support (except for WinXP users)? Does that mean we should switch socket.has_ipv6 to True?
msg6360 (view) Author: Bastian Krol (basti1302) Date: 2011-01-28.07:16:21
If IPv6 is now more supported than before (whatever that may mean) you might also want to review http://bugs.jython.org/issue1210 - that's the ticket that is about IPv6 support (this ticket here originally only was about the queer error message, not about IPv6 support as such).
msg6380 (view) Author: Alan Kennedy (amak) Date: 2011-02-03.21:57:50
Fixes checked in at r7191 and r7193.
msg6382 (view) Author: Philip Jenvey (pjenvey) Date: 2011-02-03.22:00:43
from r7193:

-# For now, we DO NOT have complete IPV6 support.
-has_ipv6 = False
+has_ipv6 = True # IPV6 FTW!

=D
msg6391 (view) Author: Philip Jenvey (pjenvey) Date: 2011-02-11.00:47:39
Alan - 

One small problem with this change, we really need a __repr__ on the new getaddrinfo results so they're easier to use (or why not continue using tuples)?

Jython 2.5.2rc3 (trunk:7195M, Feb 10 2011, 16:43:49) 
[Java HotSpot(TM) 64-Bit Server VM (Apple Inc.)] on java1.6.0_22
Type "help", "copyright", "credits" or "license" for more information.
>>> import socket
>>> socket.getaddrinfo('localhost', socket.AF_UNSPEC)
[(2, None, 0, 'localhost', <socket._ipv4_address_t instance at 0x2>)]
>>>
msg6392 (view) Author: Alan Kennedy (amak) Date: 2011-02-11.13:44:19
[Philip]
> One small problem with this change, we really need a __repr__ on the new getaddrinfo results so they're easier to use
>>> socket.getaddrinfo('localhost', socket.AF_UNSPEC)
[(2, None, 0, 'localhost', <socket._ipv4_address_t instance at 0x2>)]

OK I see the problem. I will check in a fix soon.

[Philip]
> (or why not continue using tuples)?

Because we need to be able differentiate the situation where the address has already been issued by getaddrinfo (and thus java.net.InetAddress.getAllByName), and thus does not need to be looked up again.

This is most important in the case of scoped IPV6 addresses, where the dedicated structure retains scope information.
msg6393 (view) Author: Alan Kennedy (amak) Date: 2011-02-11.14:12:24
Fix checked in at revision 7196.
History
Date User Action Args
2011-02-11 14:12:24amaksetmessages: + msg6393
2011-02-11 13:44:19amaksetmessages: + msg6392
2011-02-11 00:47:39pjenveysetmessages: + msg6391
2011-02-03 22:00:43pjenveysetmessages: + msg6382
2011-02-03 21:57:50amaksetstatus: open -> closed
resolution: accepted -> fixed
messages: + msg6380
2011-01-28 07:16:22basti1302setmessages: + msg6360
2011-01-28 00:48:05pjenveysetmessages: + msg6359
2011-01-27 21:18:03alex.gronholmsetmessages: + msg6358
2011-01-27 20:34:39amaksetmessages: + msg6357
2011-01-27 20:33:06amaksetmessages: + msg6356
2011-01-24 17:58:21pjenveysetmessages: + msg6351
2011-01-23 20:54:01otmarhumbelsetmessages: + msg6345
2011-01-23 20:46:31pjenveysetmessages: + msg6344
2011-01-23 20:41:30amaksetmessages: + msg6343
2011-01-23 19:12:14alex.gronholmsetnosy: + alex.gronholm
messages: + msg6342
2011-01-23 19:04:40otmarhumbelsetmessages: + msg6341
2011-01-23 18:33:50amaksetstatus: closed -> open
resolution: fixed -> accepted
messages: + msg6340
2011-01-23 18:33:31amaksetmessages: + msg6339
2011-01-23 17:09:46otmarhumbelsetstatus: open -> closed
resolution: fixed
messages: + msg6338
2011-01-23 14:53:56otmarhumbelsetfiles: + 1697-patch2.txt
messages: + msg6337
2011-01-22 00:09:51pjenveysetnosy: + pjenvey
messages: + msg6332
2011-01-21 06:33:33basti1302setmessages: + msg6331
2011-01-21 00:51:41otmarhumbelsetfiles: + 1697-patch.txt
nosy: + otmarhumbel
messages: + msg6329
2011-01-16 13:34:24amaksetassignee: amak
nosy: + amak
2011-01-15 10:58:38pekka.klarcksetmessages: + msg6324
2011-01-14 09:53:28pekka.klarcksetnosy: + pekka.klarck
2011-01-14 08:24:24basti1302setfiles: + test.py
2011-01-14 08:22:39basti1302create