classification
Title: sock_connect fails for bluetooth (and probably others)
Type: behavior Stage: resolved
Components: asyncio, Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: emptysquare, gvanrossum, martin.panter, pyptr2, python-dev, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2016-05-27 11:02 by pyptr2, last changed 2016-06-14 04:36 by martin.panter. This issue is now closed.

Files
File name Uploaded Description Edit
check_resolved.patch pyptr2, 2016-05-27 11:02 patch against tip
test_async_btsock.py pyptr2, 2016-05-27 12:17 minimal test case
Messages (22)
msg266486 - (view) Author: (pyptr2) Date: 2016-05-27 11:02
Base{Selector,Proactor}EventLoop each have a sock_connect() method, both of which unconditionally "try" to run base_events' _check_resolved_address() which is apparently meant to raise an exception when called with an unresolved IP hostname. (because on some OSes it cannot be done asynchronously? WTF?)

Anyhow, the current implementation prevents ANY address family other than IP (and UNIX, which is special cased) to raise the exception, no matter if it's resolved or not.

So please, call _ipaddr_info() for IP only.
msg266488 - (view) Author: (pyptr2) Date: 2016-05-27 11:57
s/prevents/causes/
msg266504 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-05-27 17:54
I cannot run your test program. Can you explain what it does and what you expected?

Also, watch your language.
msg266514 - (view) Author: (pyptr2) Date: 2016-05-27 21:18
The test program should try to connect to a bluetooth device, using asyncio.
The bluetooth address is obviously made up, so it would likely fail to do so, but it should not raise an exception before even trying:

$ python3.5 test_async_btsock.py 
Traceback (most recent call last):
  File "test_async_btsock.py", line 9, in <module>
    loop.run_until_complete(coro)
  File "/usr/lib/python3.5/asyncio/base_events.py", line 373, in run_until_complete
    return future.result()
  File "/usr/lib/python3.5/asyncio/futures.py", line 274, in result
    raise self._exception
  File "/usr/lib/python3.5/asyncio/selector_events.py", line 400, in sock_connect
    base_events._check_resolved_address(sock, address)
  File "/usr/lib/python3.5/asyncio/base_events.py", line 151, in _check_resolved_address
    " got host %r" % host)
ValueError: address must be resolved (IP address), got host '00:01:02:03:04:05'
msg266516 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-05-27 21:56
pyptr2, can you reproduce this with latest asyncio (from github.com/python/asyncio)?
msg266518 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-05-27 22:18
Maybe the form requirements for Bluetooth addresses are different than for IP or ipv6? I guess the validation should be protocol-specific.
msg266519 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-05-27 22:21
> Maybe the form requirements for Bluetooth addresses are different than for IP or ipv6? I guess the validation should be protocol-specific.

Right, I've just looked at the code [1].  We should return from _ipaddr_info if family is not an AF_UNSPEC, AF_INET or AF_INET6.

[1] https://github.com/python/asyncio/blob/master/asyncio/base_events.py#L90
msg266520 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-05-27 22:32
>> Maybe the form requirements for Bluetooth addresses are different than for IP or ipv6? I guess the validation should be protocol-specific.
> 
> Right, I've just looked at the code [1].  We should return from _ipaddr_info if family is not an AF_UNSPEC, AF_INET or AF_INET6.

Actually, we should probably return in case of AF_UNSPEC too.
msg266553 - (view) Author: (pyptr2) Date: 2016-05-28 13:55
Thanks for looking into this. Some comments:

1.
yuri, I haven't tried the github repo. Is that where the development happens? The description suggests it is outdated since the adoption, but there are some commits that aren't in hg...?
Anyhow, the relevant code looks identical to me.

2.
guido, yes validation would be domain specific.
But, just to understand: why validate at all?
If I give create_connection() or sock_connect() an unresolved address, then I would expect some "yield from resolve(domain, name)" to happen, which would be getaddrinfo() for IP-domain. If that would hang the event loop on some platform, do it in an executor. This is exactly what create_server() already does, by the way.

3.
AF_UNPSEC seems to mean (AF_INET or AF_INET6) to a lot of people these days, possibly because getaddrinfo() accepts these three AFs. I am not 100% sure, but that is why I think it is reasonable that _ipaddr_info() handles AF_UNSPEC in the way it does.

4.
For my use case, anything that stops check_resolve() from raising when given a resolved non-IP address would be enough for now.
msg266554 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-05-28 14:43
> Why validate at all?

Because (at least for the IP 4/6 protocols) when you give a host name
Python's socket code will do a synchronous DNS lookup which would hold
up the entire event loop until it's done. This would be a major
problem in some cases, so we want to disallow it completely.
msg266956 - (view) Author: A. Jesse Jiryu Davis (emptysquare) * Date: 2016-06-02 21:18
I can reproduce this. On Ubuntu 14.04 I installed "libbluetooth-dev" and then built Python 3.5.1 from source, confirmed the socket module has AF_BLUETOOTH and BTPROTO_RFCOMM. Running pyptr2's script gives the traceback pyptr2 reported.

Yury and I discussed a fix, which I'll propose as a patch in the asyncio GitHub repo.
msg267869 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-08 16:35
New changeset 3f49e89be8a9 by Yury Selivanov in branch '3.5':
Issue #27136: Fix DNS static resolution; don't use it in getaddrinfo
https://hg.python.org/cpython/rev/3f49e89be8a9

New changeset e46b23b6a068 by Yury Selivanov in branch 'default':
Merge 3.5 (issue #27136, asyncio)
https://hg.python.org/cpython/rev/e46b23b6a068
msg267870 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-06-08 16:35
Will be fixed in CPython 3.5.2.
msg267871 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-08 16:48
New changeset 149fbdbe4729 by Yury Selivanov in branch '3.5':
Issue #27136: Update asyncio docs
https://hg.python.org/cpython/rev/149fbdbe4729

New changeset 44767f603535 by Yury Selivanov in branch 'default':
Merge 3.5 (asyncio, issue #27136)
https://hg.python.org/cpython/rev/44767f603535
msg267921 - (view) Author: A. Jesse Jiryu Davis (emptysquare) * Date: 2016-06-08 22:51
The asyncio fix was proposed and reviewed here:

https://github.com/python/asyncio/pull/357
msg268313 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-12 02:14
Revision 3f49e89be8a9 seems to be the cause of an x86 Tiger buildbot failure:

http://buildbot.python.org/all/builders/x86%20Tiger%203.x/builds/10924/steps/test/logs/stdio
======================================================================
FAIL: test_create_connection_no_inet_pton (test.test_asyncio.test_base_events.BaseEventLoopWithSelectorTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/db3l/buildarea/3.x.bolen-tiger/build/Lib/unittest/mock.py", line 1180, in patched
    return func(*args, **keywargs)
  File "/Users/db3l/buildarea/3.x.bolen-tiger/build/Lib/test/test_asyncio/test_base_events.py", line 1210, in test_create_connection_no_inet_pton
    self._test_create_connection_ip_addr(m_socket, False)
  File "/Users/db3l/buildarea/3.x.bolen-tiger/build/Lib/test/test_asyncio/test_base_events.py", line 1195, in _test_create_connection_ip_addr
    self.assertRegex(host, r'::(0\.)*2')
AssertionError: Regex didn't match: '::(0\\.)*2' not found in '0.0.0.2'
msg268381 - (view) Author: A. Jesse Jiryu Davis (emptysquare) * Date: 2016-06-12 13:50
Thanks Martin. That test verifies behavior that I observe in Mac OS 10.10 and other modern platforms:

>>> socket.getaddrinfo('::2', 80, socket.AF_INET6, socket.SOCK_STREAM, socket.IPPROTO_TCP)
[(30, 1, 6, '', ('::0.0.0.2', 80, 0, 0))]

Investigating, I wrote a C program to call getaddrinfo on my Mac OS X Tiger x86 virtual machine, and indeed it resolves "::2" with family AF_INET6 in "hints" as an IPv4 address, "0.0.0.2". However, the same setup resolves "::1" as an IPv6 address, "::1".

Someone who knows more about IPv6 than I might guess the cause?

In any case, I wonder if replacing "::2" with "::1" at test_base_events.py:1188, and replacing the regex '::(0\.)*2' with '::(0\.)*1' at line 1195 would fix the test for Tiger.

Could you try that please? I'm having trouble compiling Python for Tiger so I don't know how to test it on my virtual machine.
msg268383 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-12 14:14
Okay changing to ::1 might have a decent chance of success. If nobody else comes forward, I can try committing it and see if it fixes the buildbot.
msg268404 - (view) Author: A. Jesse Jiryu Davis (emptysquare) * Date: 2016-06-12 22:33
Thanks, I'm not a core dev or I'd try it myself.

Yury, do you think that's a good approach, or should I mock getaddrinfo for that test? I fear mocking out too much and accidentally not testing anything, or of making tests that someone *else* could accidentally change so they don't test anything.
msg268486 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-14 00:03
New changeset e032ffd5ae84 by Martin Panter in branch '3.5':
Issue #27136: Change test to use ::1 for better OS X Tiger compatibility
https://hg.python.org/cpython/rev/e032ffd5ae84

New changeset f31b6c3e41f7 by Martin Panter in branch 'default':
Issue #27136: Merge test_asyncio fix from 3.5
https://hg.python.org/cpython/rev/f31b6c3e41f7
msg268489 - (view) Author: A. Jesse Jiryu Davis (emptysquare) * Date: 2016-06-14 00:29
Thank you Martin!
msg268514 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-14 04:36
Buildbots look happy with test_asyncio now, so closing again.
History
Date User Action Args
2016-06-14 04:36:18martin.pantersetstatus: open -> closed

messages: + msg268514
2016-06-14 00:29:39emptysquaresetmessages: + msg268489
2016-06-14 00:03:07python-devsetmessages: + msg268486
2016-06-12 22:33:22emptysquaresetmessages: + msg268404
2016-06-12 14:14:19martin.pantersetmessages: + msg268383
2016-06-12 13:50:24emptysquaresetmessages: + msg268381
2016-06-12 02:14:02martin.pantersetstatus: closed -> open
nosy: + martin.panter
messages: + msg268313

2016-06-08 22:51:04emptysquaresetmessages: + msg267921
2016-06-08 16:48:50python-devsetmessages: + msg267871
2016-06-08 16:35:43yselivanovsetstatus: open -> closed
resolution: fixed
messages: + msg267870

stage: resolved
2016-06-08 16:35:03python-devsetnosy: + python-dev
messages: + msg267869
2016-06-02 21:18:35emptysquaresetnosy: + emptysquare
messages: + msg266956
2016-05-28 14:43:00gvanrossumsetmessages: + msg266554
2016-05-28 13:55:09pyptr2setmessages: + msg266553
2016-05-27 22:32:43yselivanovsetmessages: + msg266520
2016-05-27 22:21:22yselivanovsetmessages: + msg266519
2016-05-27 22:18:12gvanrossumsetmessages: + msg266518
2016-05-27 21:56:06yselivanovsetmessages: + msg266516
2016-05-27 21:18:47pyptr2setmessages: + msg266514
2016-05-27 17:54:43gvanrossumsetmessages: + msg266504
2016-05-27 12:25:12pyptr2setcomponents: + Library (Lib)
versions: + Python 3.5
2016-05-27 12:17:50pyptr2setfiles: + test_async_btsock.py
2016-05-27 11:57:53pyptr2setmessages: + msg266488
2016-05-27 11:02:18pyptr2create