From b6e43af669f61a37a29d8ff0785455108e6bc29d Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Mon, 29 Jan 2018 22:37:58 +0100 Subject: [PATCH] bpo-28134: Auto-detect socket values from file descriptor (#1349) Fix socket(fileno=fd) by auto-detecting the socket's family, type, and proto from the file descriptor. The auto-detection can be overruled by passing in family, type, and proto explicitly. Without the fix, all socket except for TCP/IP over IPv4 are basically broken: >>> s = socket.create_connection(('www.python.org', 443)) >>> s >>> socket.socket(fileno=s.fileno()) Signed-off-by: Christian Heimes --- Doc/library/socket.rst | 13 ++-- Lib/socket.py | 9 ++- Lib/test/_test_multiprocessing.py | 2 +- Lib/test/test_socket.py | 45 +++++++++++- .../2017-12-24-20-01-09.bpo-28134.HJ8Beb.rst | 2 + Modules/socketmodule.c | 71 ++++++++++++++++++- 6 files changed, 133 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-12-24-20-01-09.bpo-28134.HJ8Beb.rst diff --git a/Doc/library/socket.rst b/Doc/library/socket.rst index faa260e5d6c..7f0d4ede7cc 100644 --- a/Doc/library/socket.rst +++ b/Doc/library/socket.rst @@ -461,10 +461,15 @@ The following functions all create :ref:`socket objects `. :const:`SOCK_DGRAM`, :const:`SOCK_RAW` or perhaps one of the other ``SOCK_`` constants. The protocol number is usually zero and may be omitted or in the case where the address family is :const:`AF_CAN` the protocol should be one - of :const:`CAN_RAW`, :const:`CAN_BCM` or :const:`CAN_ISOTP`. If *fileno* is specified, the other - arguments are ignored, causing the socket with the specified file descriptor - to return. Unlike :func:`socket.fromfd`, *fileno* will return the same - socket and not a duplicate. This may help close a detached socket using + of :const:`CAN_RAW`, :const:`CAN_BCM` or :const:`CAN_ISOTP` + + If *fileno* is specified, the values for *family*, *type*, and *proto* are + auto-detected from the specified file descriptor. Auto-detection can be + overruled by calling the function with explicit *family*, *type*, or *proto* + arguments. This only affects how Python represents e.g. the return value + of :meth:`socket.getpeername` but not the actual OS resource. Unlike + :func:`socket.fromfd`, *fileno* will return the same socket and not a + duplicate. This may help close a detached socket using :meth:`socket.close()`. The newly created socket is :ref:`non-inheritable `. diff --git a/Lib/socket.py b/Lib/socket.py index 2d8aee3e904..cfa605a22ad 100644 --- a/Lib/socket.py +++ b/Lib/socket.py @@ -136,11 +136,18 @@ class socket(_socket.socket): __slots__ = ["__weakref__", "_io_refs", "_closed"] - def __init__(self, family=AF_INET, type=SOCK_STREAM, proto=0, fileno=None): + def __init__(self, family=-1, type=-1, proto=-1, fileno=None): # For user code address family and type values are IntEnum members, but # for the underlying _socket.socket they're just integers. The # constructor of _socket.socket converts the given argument to an # integer automatically. + if fileno is None: + if family == -1: + family = AF_INET + if type == -1: + type = SOCK_STREAM + if proto == -1: + proto = 0 _socket.socket.__init__(self, family, type, proto, fileno) self._io_refs = 0 self._closed = False diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 05166b91ba8..1e497a572a7 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -4204,7 +4204,7 @@ class TestCloseFds(unittest.TestCase): def close(self, fd): if WIN32: - socket.socket(fileno=fd).close() + socket.socket(socket.AF_INET, socket.SOCK_STREAM, fileno=fd).close() else: os.close(fd) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 365da36c421..89cf1fa5200 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -20,6 +20,7 @@ import math import pickle import struct import random +import shutil import string import _thread as thread import threading @@ -1284,6 +1285,7 @@ class GeneralModuleTests(unittest.TestCase): def testCloseException(self): sock = socket.socket() + sock.bind((socket._LOCALHOST, 0)) socket.socket(fileno=sock.fileno()).close() try: sock.close() @@ -1636,9 +1638,11 @@ class GeneralModuleTests(unittest.TestCase): ) + 1 with socket.socket( - family=unknown_family, type=unknown_type, fileno=fd) as s: + family=unknown_family, type=unknown_type, proto=23, + fileno=fd) as s: self.assertEqual(s.family, unknown_family) self.assertEqual(s.type, unknown_type) + self.assertEqual(s.proto, 23) @unittest.skipUnless(hasattr(os, 'sendfile'), 'test needs os.sendfile()') def test__sendfile_use_sendfile(self): @@ -1658,6 +1662,45 @@ class GeneralModuleTests(unittest.TestCase): with self.assertRaises(TypeError): sock._sendfile_use_sendfile(File(None)) + def _test_socket_fileno(self, s, family, stype): + self.assertEqual(s.family, family) + self.assertEqual(s.type, stype) + + fd = s.fileno() + s2 = socket.socket(fileno=fd) + self.addCleanup(s2.close) + # detach old fd to avoid double close + s.detach() + self.assertEqual(s2.family, family) + self.assertEqual(s2.type, stype) + self.assertEqual(s2.fileno(), fd) + + def test_socket_fileno(self): + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self.addCleanup(s.close) + s.bind((support.HOST, 0)) + self._test_socket_fileno(s, socket.AF_INET, socket.SOCK_STREAM) + + if hasattr(socket, "SOCK_DGRAM"): + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + self.addCleanup(s.close) + s.bind((support.HOST, 0)) + self._test_socket_fileno(s, socket.AF_INET, socket.SOCK_DGRAM) + + if support.IPV6_ENABLED: + s = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) + self.addCleanup(s.close) + s.bind((support.HOSTv6, 0, 0, 0)) + self._test_socket_fileno(s, socket.AF_INET6, socket.SOCK_STREAM) + + if hasattr(socket, "AF_UNIX"): + tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmpdir) + s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + self.addCleanup(s.close) + s.bind(os.path.join(tmpdir, 'socket')) + self._test_socket_fileno(s, socket.AF_UNIX, socket.SOCK_STREAM) + @unittest.skipUnless(HAVE_SOCKET_CAN, 'SocketCan required for this test.') class BasicCANTest(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2017-12-24-20-01-09.bpo-28134.HJ8Beb.rst b/Misc/NEWS.d/next/Library/2017-12-24-20-01-09.bpo-28134.HJ8Beb.rst new file mode 100644 index 00000000000..9c4c683db6e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-12-24-20-01-09.bpo-28134.HJ8Beb.rst @@ -0,0 +1,2 @@ +Sockets now auto-detect family, type and protocol from file descriptor by +default. diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 8744ea0be87..8cc5d14cbb2 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -102,7 +102,8 @@ Local naming conventions: /* Socket object documentation */ PyDoc_STRVAR(sock_doc, -"socket(family=AF_INET, type=SOCK_STREAM, proto=0, fileno=None) -> socket object\n\ +"socket(family=AF_INET, type=SOCK_STREAM, proto=0) -> socket object\n\ +socket(family=-1, type=-1, proto=-1, fileno=None) -> socket object\n\ \n\ Open a socket of the given type. The family argument specifies the\n\ address family; it defaults to AF_INET. The type argument specifies\n\ @@ -111,6 +112,9 @@ or datagram (SOCK_DGRAM) socket. The protocol argument defaults to 0,\n\ specifying the default protocol. Keyword arguments are accepted.\n\ The socket is created as non-inheritable.\n\ \n\ +When a fileno is passed in, family, type and proto are auto-detected,\n\ +unless they are explicitly set.\n\ +\n\ A socket object represents one endpoint of a network connection.\n\ \n\ Methods of socket objects (keyword arguments not allowed):\n\ @@ -4792,7 +4796,7 @@ sock_initobj(PyObject *self, PyObject *args, PyObject *kwds) PySocketSockObject *s = (PySocketSockObject *)self; PyObject *fdobj = NULL; SOCKET_T fd = INVALID_SOCKET; - int family = AF_INET, type = SOCK_STREAM, proto = 0; + int family = -1, type = -1, proto = -1; static char *keywords[] = {"family", "type", "proto", "fileno", 0}; #ifndef MS_WINDOWS #ifdef SOCK_CLOEXEC @@ -4842,9 +4846,72 @@ sock_initobj(PyObject *self, PyObject *args, PyObject *kwds) "can't use invalid socket value"); return -1; } + + if (family == -1) { + sock_addr_t addrbuf; + socklen_t addrlen = sizeof(sock_addr_t); + + memset(&addrbuf, 0, addrlen); + if (getsockname(fd, SAS2SA(&addrbuf), &addrlen) == 0) { + family = SAS2SA(&addrbuf)->sa_family; + } else { +#ifdef MS_WINDOWS + PyErr_SetFromWindowsErrWithFilename(0, "family"); +#else + PyErr_SetFromErrnoWithFilename(PyExc_OSError, "family"); +#endif + return -1; + } + } +#ifdef SO_TYPE + if (type == -1) { + int tmp; + socklen_t slen = sizeof(tmp); + if (getsockopt(fd, SOL_SOCKET, SO_TYPE, &tmp, &slen) == 0) { + type = tmp; + } else { +#ifdef MS_WINDOWS + PyErr_SetFromWindowsErrWithFilename(0, "type"); +#else + PyErr_SetFromErrnoWithFilename(PyExc_OSError, "type"); +#endif + return -1; + } + } +#else + type = SOCK_STREAM; +#endif +#ifdef SO_PROTOCOL + if (proto == -1) { + int tmp; + socklen_t slen = sizeof(tmp); + if (getsockopt(fd, SOL_SOCKET, SO_PROTOCOL, &tmp, &slen) == 0) { + proto = tmp; + } else { +#ifdef MS_WINDOWS + PyErr_SetFromWindowsErrWithFilename(0, "protocol"); +#else + PyErr_SetFromErrnoWithFilename(PyExc_OSError, "protocol"); +#endif + return -1; + } + } +#else + proto = 0; +#endif } } else { + /* No fd, default to AF_INET and SOCK_STREAM */ + if (family == -1) { + family = AF_INET; + } + if (type == -1) { + type = SOCK_STREAM; + } + if (proto == -1) { + proto = 0; + } #ifdef MS_WINDOWS /* Windows implementation */ #ifndef WSA_FLAG_NO_HANDLE_INHERIT