diff --git a/Lib/socket.py b/Lib/socket.py index be019dbc51a..a2df89132f4 100644 --- a/Lib/socket.py +++ b/Lib/socket.py @@ -287,8 +287,8 @@ def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT): is used. """ - msg = "getaddrinfo returns an empty list" host, port = address + err = None for res in getaddrinfo(host, port, 0, SOCK_STREAM): af, socktype, proto, canonname, sa = res sock = None @@ -299,9 +299,12 @@ def create_connection(address, timeout=_GLOBAL_DEFAULT_TIMEOUT): sock.connect(sa) return sock - except error as err: - msg = err + except error as _: + err = _ if sock is not None: sock.close() - raise error(msg) + if err is not None: + raise err + else: + raise error("getaddrinfo returns an empty list") diff --git a/Lib/test/support.py b/Lib/test/support.py index 11bb16826b5..8ef87c28b49 100644 --- a/Lib/test/support.py +++ b/Lib/test/support.py @@ -17,19 +17,21 @@ import unittest import importlib import collections -__all__ = ["Error", "TestFailed", "ResourceDenied", "import_module", - "verbose", "use_resources", "max_memuse", "record_original_stdout", - "get_original_stdout", "unload", "unlink", "rmtree", "forget", - "is_resource_enabled", "requires", "find_unused_port", "bind_port", - "fcmp", "is_jython", "TESTFN", "HOST", "FUZZ", "findfile", "verify", - "vereq", "sortdict", "check_syntax_error", "open_urlresource", - "check_warnings", "CleanImport", "EnvironmentVarGuard", - "TransientResource", "captured_output", "captured_stdout", - "time_out", "socket_peer_reset", "ioerror_peer_reset", - "run_with_locale", - "set_memlimit", "bigmemtest", "bigaddrspacetest", "BasicTestRunner", - "run_unittest", "run_doctest", "threading_setup", "threading_cleanup", - "reap_children", "cpython_only", "check_impl_detail", "get_attribute"] +__all__ = [ + "Error", "TestFailed", "ResourceDenied", "import_module", + "verbose", "use_resources", "max_memuse", "record_original_stdout", + "get_original_stdout", "unload", "unlink", "rmtree", "forget", + "is_resource_enabled", "requires", "find_unused_port", "bind_port", + "fcmp", "is_jython", "TESTFN", "HOST", "FUZZ", "findfile", "verify", + "vereq", "sortdict", "check_syntax_error", "open_urlresource", + "check_warnings", "CleanImport", "EnvironmentVarGuard", + "TransientResource", "captured_output", "captured_stdout", + "time_out", "socket_peer_reset", "ioerror_peer_reset", + "run_with_locale", "transient_internet", + "set_memlimit", "bigmemtest", "bigaddrspacetest", "BasicTestRunner", + "run_unittest", "run_doctest", "threading_setup", "threading_cleanup", + "reap_children", "cpython_only", "check_impl_detail", "get_attribute", + ] class Error(Exception): """Base class for regression test exceptions.""" @@ -604,23 +606,63 @@ class TransientResource(object): else: raise ResourceDenied("an optional resource is not available") - # Context managers that raise ResourceDenied when various issues # with the Internet connection manifest themselves as exceptions. +# XXX deprecate these and use transient_internet() instead time_out = TransientResource(IOError, errno=errno.ETIMEDOUT) socket_peer_reset = TransientResource(socket.error, errno=errno.ECONNRESET) ioerror_peer_reset = TransientResource(IOError, errno=errno.ECONNRESET) @contextlib.contextmanager -def transient_internet(): +def transient_internet(resource_name, *, timeout=30.0, errnos=()): """Return a context manager that raises ResourceDenied when various issues with the Internet connection manifest themselves as exceptions.""" - time_out = TransientResource(IOError, errno=errno.ETIMEDOUT) - socket_peer_reset = TransientResource(socket.error, errno=errno.ECONNRESET) - ioerror_peer_reset = TransientResource(IOError, errno=errno.ECONNRESET) - with time_out, socket_peer_reset, ioerror_peer_reset: + default_errnos = [ + ('ECONNREFUSED', 111), + ('ECONNRESET', 104), + ('ENETUNREACH', 101), + ('ETIMEDOUT', 110), + ] + + denied = ResourceDenied("Resource '%s' is not available" % resource_name) + captured_errnos = errnos + if not captured_errnos: + captured_errnos = [getattr(errno, name, num) + for (name, num) in default_errnos] + + def filter_error(err): + if (isinstance(err, socket.timeout) or + getattr(err, 'errno', None) in captured_errnos): + if not verbose: + sys.stderr.write(denied.args[0] + "\n") + raise denied from err + + old_timeout = socket.getdefaulttimeout() + try: + if timeout is not None: + socket.setdefaulttimeout(timeout) yield + except IOError as err: + # urllib can wrap original socket errors multiple times (!), we must + # unwrap to get at the original error. + while True: + a = err.args + if len(a) >= 1 and isinstance(a[0], IOError): + err = a[0] + # The error can also be wrapped as args[1]: + # except socket.error as msg: + # raise IOError('socket error', msg).with_traceback(sys.exc_info()[2]) + elif len(a) >= 2 and isinstance(a[1], IOError): + err = a[1] + else: + break + filter_error(err) + raise + # XXX should we catch generic exceptions and look for their + # __cause__ or __context__? + finally: + socket.setdefaulttimeout(old_timeout) @contextlib.contextmanager diff --git a/Lib/test/test_robotparser.py b/Lib/test/test_robotparser.py index fd007065e19..2a6d047eef5 100644 --- a/Lib/test/test_robotparser.py +++ b/Lib/test/test_robotparser.py @@ -235,23 +235,24 @@ class NetworkTestCase(unittest.TestCase): def testPasswordProtectedSite(self): support.requires('network') - # XXX it depends on an external resource which could be unavailable - url = 'http://mueblesmoraleda.com' - parser = urllib.robotparser.RobotFileParser() - parser.set_url(url) - try: - parser.read() - except URLError: - self.skipTest('%s is unavailable' % url) - self.assertEqual(parser.can_fetch("*", url+"/robots.txt"), False) + with support.transient_internet('mueblesmoraleda.com'): + url = 'http://mueblesmoraleda.com' + parser = urllib.robotparser.RobotFileParser() + parser.set_url(url) + try: + parser.read() + except URLError: + self.skipTest('%s is unavailable' % url) + self.assertEqual(parser.can_fetch("*", url+"/robots.txt"), False) def testPythonOrg(self): support.requires('network') - parser = urllib.robotparser.RobotFileParser( - "http://www.python.org/robots.txt") - parser.read() - self.assertTrue(parser.can_fetch("*", - "http://www.python.org/robots.txt")) + with support.transient_internet('www.python.org'): + parser = urllib.robotparser.RobotFileParser( + "http://www.python.org/robots.txt") + parser.read() + self.assertTrue( + parser.can_fetch("*", "http://www.python.org/robots.txt")) def test_main(): support.run_unittest(NetworkTestCase) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 5e340bf67f2..2415d539abc 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -14,6 +14,7 @@ import queue import sys import os import array +import contextlib from weakref import proxy import signal @@ -1026,12 +1027,48 @@ class BasicTCPTest2(NetworkConnectionTest, BasicTCPTest): class NetworkConnectionNoServer(unittest.TestCase): - def testWithoutServer(self): + class MockSocket(socket.socket): + def connect(self, *args): + raise socket.timeout('timed out') + + @contextlib.contextmanager + def mocked_socket_module(self): + """Return a socket which times out on connect""" + old_socket = socket.socket + socket.socket = self.MockSocket + try: + yield + finally: + socket.socket = old_socket + + def test_connect(self): port = support.find_unused_port() - self.assertRaises( - socket.error, - lambda: socket.create_connection((HOST, port)) - ) + cli = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + cli.connect((HOST, port)) + except socket.error as err: + self.assertEqual(err.errno, errno.ECONNREFUSED) + else: + self.fail("socket.error not raised") + + def test_create_connection(self): + # Issue #9792: errors raised by create_connection() should have + # a proper errno attribute. + port = support.find_unused_port() + try: + socket.create_connection((HOST, port)) + except socket.error as err: + self.assertEqual(err.errno, errno.ECONNREFUSED) + else: + self.fail("socket.error not raised") + + def test_create_connection_timeout(self): + # Issue #9792: create_connection() should not recast timeout errors + # as generic socket errors. + with self.mocked_socket_module(): + with self.assertRaises(socket.timeout): + socket.create_connection((HOST, 1234)) + class NetworkConnectionAttributesTest(SocketTCPTest, ThreadableTest): diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 193978cb3d5..4904bdad854 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -218,10 +218,10 @@ class NetworkedTests(unittest.TestCase): # NOTE: https://sha256.tbs-internet.com is another possible test host remote = ("sha2.hboeck.de", 443) sha256_cert = os.path.join(os.path.dirname(__file__), "sha256.pem") - s = ssl.wrap_socket(socket.socket(socket.AF_INET), - cert_reqs=ssl.CERT_REQUIRED, - ca_certs=sha256_cert,) - with support.transient_internet(): + with support.transient_internet("sha2.hboeck.de"): + s = ssl.wrap_socket(socket.socket(socket.AF_INET), + cert_reqs=ssl.CERT_REQUIRED, + ca_certs=sha256_cert,) try: s.connect(remote) if support.verbose: diff --git a/Misc/NEWS b/Misc/NEWS index 24dbb0e8c4b..3a588ad6fb3 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -105,6 +105,11 @@ C-API Library ------- +- Issue #9792: In case of connection failure, socket.create_connection() + would swallow the exception and raise a new one, making it impossible + to fetch the original errno, or to filter timeout errors. Now the + original error is re-raised. + - Issue #9758: When fcntl.ioctl() was called with mutable_flag set to True, and the passed buffer was exactly 1024 bytes long, the buffer wouldn't be updated back after the system call. Original patch by Brian Brazil.