From a631a28bd8529dbf3ca0394ee610252b9e39d312 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Tue, 5 Feb 2019 20:30:35 -0500 Subject: [PATCH 1/8] In http.server script, rely on getaddrinfo to bind to preferred address based on the bind parameter. --- Lib/http/server.py | 9 +++++++-- Lib/test/test_httpservers.py | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/Lib/http/server.py b/Lib/http/server.py index 29c720ea7ea87e..3b5a504b0b77f3 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -1224,6 +1224,12 @@ def run_cgi(self): self.log_message("CGI script exited OK") +def _get_best_family(address): + infos = socket.getaddrinfo(*address, type=socket.SOCK_STREAM) + family, type, proto, canonname, sockaddr = next(iter(infos)) + return family + + def test(HandlerClass=BaseHTTPRequestHandler, ServerClass=ThreadingHTTPServer, protocol="HTTP/1.0", port=8000, bind=""): @@ -1234,8 +1240,7 @@ def test(HandlerClass=BaseHTTPRequestHandler, """ server_address = (bind, port) - if ':' in bind: - ServerClass.address_family = socket.AF_INET6 + ServerClass.address_family = _get_best_family(server_address) HandlerClass.protocol_version = protocol with ServerClass(server_address, HandlerClass) as httpd: diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index 3d8e0af8b45c27..831c07f4775d69 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -1118,6 +1118,25 @@ def test_all(self): class ScriptTestCase(unittest.TestCase): + + @mock.patch('builtins.print') + def test_server_test_unspec(self, _): + mock_server = mock.MagicMock() + server.test(ServerClass=mock_server, bind="") + self.assertIn( + mock_server.address_family, + (socket.AF_INET6, socket.AF_INET), + ) + + @mock.patch('builtins.print') + def test_server_test_localhost(self, _): + mock_server = mock.MagicMock() + server.test(ServerClass=mock_server, bind="localhost") + self.assertIn( + mock_server.address_family, + (socket.AF_INET6, socket.AF_INET), + ) + @mock.patch('builtins.print') def test_server_test_ipv6(self, _): mock_server = mock.MagicMock() @@ -1130,10 +1149,23 @@ def test_server_test_ipv6(self, _): self.assertEqual(mock_server.address_family, socket.AF_INET6) mock_server.reset_mock() - server.test(ServerClass=mock_server, - bind="::1") + server.test(ServerClass=mock_server, bind="::1") self.assertEqual(mock_server.address_family, socket.AF_INET6) + @mock.patch('builtins.print') + def test_server_test_ipv4(self, _): + mock_server = mock.MagicMock() + server.test(ServerClass=mock_server, bind="0.0.0.0") + self.assertEqual(mock_server.address_family, socket.AF_INET) + + mock_server.reset_mock() + server.test(ServerClass=mock_server, bind="8.8.8.8") + self.assertEqual(mock_server.address_family, socket.AF_INET) + + mock_server.reset_mock() + server.test(ServerClass=mock_server, bind="127.0.0.1") + self.assertEqual(mock_server.address_family, socket.AF_INET) + def test_main(verbose=None): cwd = os.getcwd() From 83f2277bd8a31ef2ce52edb4efac5b64cbe42f55 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Wed, 6 Feb 2019 01:40:55 +0000 Subject: [PATCH 2/8] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NEWS.d/next/Library/2019-02-06-01-40-55.bpo-24209.awtwPD.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2019-02-06-01-40-55.bpo-24209.awtwPD.rst diff --git a/Misc/NEWS.d/next/Library/2019-02-06-01-40-55.bpo-24209.awtwPD.rst b/Misc/NEWS.d/next/Library/2019-02-06-01-40-55.bpo-24209.awtwPD.rst new file mode 100644 index 00000000000000..4d555fd4125a49 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-06-01-40-55.bpo-24209.awtwPD.rst @@ -0,0 +1 @@ +In http.server script, rely on getaddrinfo to bind to preferred address based on the bind parameter. Now default bind or binding to a name may bind to IPv6 or dual-stack, depending on the environment. \ No newline at end of file From 6913aec0639620dd636cd425d5cc0cffbb94658e Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Tue, 5 Feb 2019 20:53:34 -0500 Subject: [PATCH 3/8] Wrap bound address in square brackets if it's IPv6. --- Lib/http/server.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/http/server.py b/Lib/http/server.py index 3b5a504b0b77f3..9819cb27fc31ef 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -1244,9 +1244,12 @@ def test(HandlerClass=BaseHTTPRequestHandler, HandlerClass.protocol_version = protocol with ServerClass(server_address, HandlerClass) as httpd: - sa = httpd.socket.getsockname() - serve_message = "Serving HTTP on {host} port {port} (http://{host}:{port}/) ..." - print(serve_message.format(host=sa[0], port=sa[1])) + host, port = httpd.socket.getsockname()[:2] + url_host = f'[{host}]' if ':' in host else host + print( + f"Serving HTTP on {host} port {port} " + f"(http://{url_host}:{port}/) ..." + ) try: httpd.serve_forever() except KeyboardInterrupt: From 28ac4d1a6d4bcdde39ae5e104868d4b96ae32ac5 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Tue, 5 Feb 2019 22:27:22 -0500 Subject: [PATCH 4/8] Create a more sophisticated mock that's able to return a two-tuple --- Lib/test/test_httpservers.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index 831c07f4775d69..757c51b4f49c42 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -1119,9 +1119,22 @@ def test_all(self): class ScriptTestCase(unittest.TestCase): + def mock_server_class(self): + return mock.MagicMock( + return_value=mock.MagicMock( + __enter__=mock.MagicMock( + return_value=mock.MagicMock( + socket=mock.MagicMock( + getsockname=lambda: ('', 0), + ), + ), + ), + ), + ) + @mock.patch('builtins.print') def test_server_test_unspec(self, _): - mock_server = mock.MagicMock() + mock_server = self.mock_server_class() server.test(ServerClass=mock_server, bind="") self.assertIn( mock_server.address_family, @@ -1130,7 +1143,7 @@ def test_server_test_unspec(self, _): @mock.patch('builtins.print') def test_server_test_localhost(self, _): - mock_server = mock.MagicMock() + mock_server = self.mock_server_class() server.test(ServerClass=mock_server, bind="localhost") self.assertIn( mock_server.address_family, @@ -1139,7 +1152,7 @@ def test_server_test_localhost(self, _): @mock.patch('builtins.print') def test_server_test_ipv6(self, _): - mock_server = mock.MagicMock() + mock_server = self.mock_server_class() server.test(ServerClass=mock_server, bind="::") self.assertEqual(mock_server.address_family, socket.AF_INET6) @@ -1154,7 +1167,7 @@ def test_server_test_ipv6(self, _): @mock.patch('builtins.print') def test_server_test_ipv4(self, _): - mock_server = mock.MagicMock() + mock_server = self.mock_server_class() server.test(ServerClass=mock_server, bind="0.0.0.0") self.assertEqual(mock_server.address_family, socket.AF_INET) From b334e8914555246aa18ae93a74da60da039337b9 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Tue, 5 Feb 2019 22:28:50 -0500 Subject: [PATCH 5/8] Create new mocks rather than resetting the original. --- Lib/test/test_httpservers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index 757c51b4f49c42..99b21fd8334886 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -1156,12 +1156,12 @@ def test_server_test_ipv6(self, _): server.test(ServerClass=mock_server, bind="::") self.assertEqual(mock_server.address_family, socket.AF_INET6) - mock_server.reset_mock() + mock_server = self.mock_server_class() server.test(ServerClass=mock_server, bind="2001:0db8:85a3:0000:0000:8a2e:0370:7334") self.assertEqual(mock_server.address_family, socket.AF_INET6) - mock_server.reset_mock() + mock_server = self.mock_server_class() server.test(ServerClass=mock_server, bind="::1") self.assertEqual(mock_server.address_family, socket.AF_INET6) @@ -1171,11 +1171,11 @@ def test_server_test_ipv4(self, _): server.test(ServerClass=mock_server, bind="0.0.0.0") self.assertEqual(mock_server.address_family, socket.AF_INET) - mock_server.reset_mock() + mock_server = self.mock_server_class() server.test(ServerClass=mock_server, bind="8.8.8.8") self.assertEqual(mock_server.address_family, socket.AF_INET) - mock_server.reset_mock() + mock_server = self.mock_server_class() server.test(ServerClass=mock_server, bind="127.0.0.1") self.assertEqual(mock_server.address_family, socket.AF_INET) From d0d350672f8479a0f58fd1852889969216e93763 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Tue, 5 Feb 2019 22:32:53 -0500 Subject: [PATCH 6/8] Consolidate tests into paramterized calls --- Lib/test/test_httpservers.py | 43 +++++++++++++++++------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index 99b21fd8334886..bfc081e83c9f80 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -1150,34 +1150,31 @@ def test_server_test_localhost(self, _): (socket.AF_INET6, socket.AF_INET), ) + ipv6_addrs = ( + "::", + "2001:0db8:85a3:0000:0000:8a2e:0370:7334", + "::1", + ) + + ipv4_addrs = ( + "0.0.0.0", + "8.8.8.8", + "127.0.0.1", + ) + @mock.patch('builtins.print') def test_server_test_ipv6(self, _): - mock_server = self.mock_server_class() - server.test(ServerClass=mock_server, bind="::") - self.assertEqual(mock_server.address_family, socket.AF_INET6) - - mock_server = self.mock_server_class() - server.test(ServerClass=mock_server, - bind="2001:0db8:85a3:0000:0000:8a2e:0370:7334") - self.assertEqual(mock_server.address_family, socket.AF_INET6) - - mock_server = self.mock_server_class() - server.test(ServerClass=mock_server, bind="::1") - self.assertEqual(mock_server.address_family, socket.AF_INET6) + for bind in self.ipv6_addrs: + mock_server = self.mock_server_class() + server.test(ServerClass=mock_server, bind=bind) + self.assertEqual(mock_server.address_family, socket.AF_INET6) @mock.patch('builtins.print') def test_server_test_ipv4(self, _): - mock_server = self.mock_server_class() - server.test(ServerClass=mock_server, bind="0.0.0.0") - self.assertEqual(mock_server.address_family, socket.AF_INET) - - mock_server = self.mock_server_class() - server.test(ServerClass=mock_server, bind="8.8.8.8") - self.assertEqual(mock_server.address_family, socket.AF_INET) - - mock_server = self.mock_server_class() - server.test(ServerClass=mock_server, bind="127.0.0.1") - self.assertEqual(mock_server.address_family, socket.AF_INET) + for bind in self.ipv4_addrs: + mock_server = self.mock_server_class() + server.test(ServerClass=mock_server, bind=bind) + self.assertEqual(mock_server.address_family, socket.AF_INET) def test_main(verbose=None): From 967845f6c9aeac7f387a5cc6681e15561973ff10 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Wed, 6 Feb 2019 08:59:51 -0500 Subject: [PATCH 7/8] Prefer None for the bind parameter as the default for all interfaces, as that's what the docs suggest is used for getaddrinfo for all interfaces. --- Lib/http/server.py | 4 ++-- Lib/test/test_httpservers.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/http/server.py b/Lib/http/server.py index 9819cb27fc31ef..ad8c2880b5e961 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -1232,7 +1232,7 @@ def _get_best_family(address): def test(HandlerClass=BaseHTTPRequestHandler, ServerClass=ThreadingHTTPServer, - protocol="HTTP/1.0", port=8000, bind=""): + protocol="HTTP/1.0", port=8000, bind=None): """Test the HTTP request handler class. This runs an HTTP server on port 8000 (or the port argument). @@ -1262,7 +1262,7 @@ def test(HandlerClass=BaseHTTPRequestHandler, parser = argparse.ArgumentParser() parser.add_argument('--cgi', action='store_true', help='Run as CGI Server') - parser.add_argument('--bind', '-b', default='', metavar='ADDRESS', + parser.add_argument('--bind', '-b', metavar='ADDRESS', help='Specify alternate bind address ' '[default: all interfaces]') parser.add_argument('--directory', '-d', default=os.getcwd(), diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index bfc081e83c9f80..8357ee9145d7e5 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -1135,7 +1135,7 @@ def mock_server_class(self): @mock.patch('builtins.print') def test_server_test_unspec(self, _): mock_server = self.mock_server_class() - server.test(ServerClass=mock_server, bind="") + server.test(ServerClass=mock_server, bind=None) self.assertIn( mock_server.address_family, (socket.AF_INET6, socket.AF_INET), From 62dbe55c9d88c75868399de9d86bcd947e23951c Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Wed, 6 Feb 2019 09:10:16 -0500 Subject: [PATCH 8/8] In getaddrinfo, use AI_PASSIVE to indicate to get the wildcard address (all interfaces). --- Lib/http/server.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Lib/http/server.py b/Lib/http/server.py index ad8c2880b5e961..b247675ec45e81 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -1224,10 +1224,14 @@ def run_cgi(self): self.log_message("CGI script exited OK") -def _get_best_family(address): - infos = socket.getaddrinfo(*address, type=socket.SOCK_STREAM) +def _get_best_family(*address): + infos = socket.getaddrinfo( + *address, + type=socket.SOCK_STREAM, + flags=socket.AI_PASSIVE, + ) family, type, proto, canonname, sockaddr = next(iter(infos)) - return family + return family, sockaddr def test(HandlerClass=BaseHTTPRequestHandler, @@ -1238,12 +1242,10 @@ def test(HandlerClass=BaseHTTPRequestHandler, This runs an HTTP server on port 8000 (or the port argument). """ - server_address = (bind, port) - - ServerClass.address_family = _get_best_family(server_address) + ServerClass.address_family, addr = _get_best_family(bind, port) HandlerClass.protocol_version = protocol - with ServerClass(server_address, HandlerClass) as httpd: + with ServerClass(addr, HandlerClass) as httpd: host, port = httpd.socket.getsockname()[:2] url_host = f'[{host}]' if ':' in host else host print(