From 49b00da9a5ae13c9ab1b08a4ab0a8ef55a7bebd9 Mon Sep 17 00:00:00 2001 From: Wheeler Law Date: Thu, 10 Nov 2022 16:14:14 -0600 Subject: [PATCH 1/5] bugfix: let the HTTP- and HTTPSHandlers respect the value of http.client.HTTPConnection.debuglevel --- Lib/urllib/request.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py index 151034e6a81bf9..19e2e5bd335627 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -1251,8 +1251,8 @@ def http_error_407(self, req, fp, code, msg, headers): class AbstractHTTPHandler(BaseHandler): - def __init__(self, debuglevel=0): - self._debuglevel = debuglevel + def __init__(self, debuglevel=None): + self._debuglevel = debuglevel if debuglevel is not None else http.client.HTTPConnection.debuglevel def set_http_debuglevel(self, level): self._debuglevel = level @@ -1378,7 +1378,8 @@ def http_open(self, req): class HTTPSHandler(AbstractHTTPHandler): - def __init__(self, debuglevel=0, context=None, check_hostname=None): + def __init__(self, debuglevel=None, context=None, check_hostname=None): + debuglevel = debuglevel if debuglevel is not None else http.client.HTTPSConnection.debuglevel AbstractHTTPHandler.__init__(self, debuglevel) if context is None: http_version = http.client.HTTPSConnection._http_vsn From 510cf5d79cfd4b0a37b8b26a7a1894450d1067d5 Mon Sep 17 00:00:00 2001 From: Wheeler Law Date: Thu, 10 Nov 2022 16:14:29 -0600 Subject: [PATCH 2/5] add tests --- Lib/test/test_urllib2.py | 58 +++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 633d596ac3de3f..530ac72463c233 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -483,8 +483,17 @@ def build_test_opener(*handler_instances): opener.add_handler(h) return opener +class MockHTTPHandler(urllib.request.HTTPHandler): + # Very simple mock HTTP handler with no special behavior other than using a mock HTTP connection -class MockHTTPHandler(urllib.request.BaseHandler): + def __init__(self, debuglevel=None): + super(MockHTTPHandler, self).__init__(debuglevel=debuglevel) + self.httpconn = MockHTTPClass() + + def http_open(self, req): + return self.do_open(self.httpconn, req) + +class MockHTTPHandlerRedirect(urllib.request.BaseHandler): # useful for testing redirections and auth # sends supplied headers and code as first response # sends 200 OK as second response @@ -512,12 +521,12 @@ def http_open(self, req): return MockResponse(200, "OK", msg, "", req.get_full_url()) -class MockHTTPSHandler(urllib.request.AbstractHTTPHandler): +class MockHTTPSHandler(urllib.request.HTTPSHandler): # Useful for testing the Proxy-Authorization request by verifying the # properties of httpcon - def __init__(self, debuglevel=0): - urllib.request.AbstractHTTPHandler.__init__(self, debuglevel=debuglevel) + def __init__(self, debuglevel=None, context=None, check_hostname=None): + super(MockHTTPSHandler, self).__init__(debuglevel, context, check_hostname) self.httpconn = MockHTTPClass() def https_open(self, req): @@ -1048,7 +1057,30 @@ def test_http_body_array(self): newreq = h.do_request_(req) self.assertEqual(int(newreq.get_header('Content-length')),16) - def test_http_handler_debuglevel(self): + def test_http_handler_global_debuglevel(self): + http.client.HTTPConnection.debuglevel = 1 + o = OpenerDirector() + h = MockHTTPHandler() + o.add_handler(h) + o.open("http://www.example.com") + self.assertEqual(h._debuglevel, 1) + + def test_http_handler_local_debuglevel(self): + o = OpenerDirector() + h = MockHTTPHandler(debuglevel=1) + o.add_handler(h) + o.open("http://www.example.com") + self.assertEqual(h._debuglevel, 1) + + def test_https_handler_global_debuglevel(self): + http.client.HTTPSConnection.debuglevel = 1 + o = OpenerDirector() + h = MockHTTPSHandler() + o.add_handler(h) + o.open("https://www.example.com") + self.assertEqual(h._debuglevel, 1) + + def test_https_handler_local_debuglevel(self): o = OpenerDirector() h = MockHTTPSHandler(debuglevel=1) o.add_handler(h) @@ -1289,7 +1321,7 @@ def test_cookie_redirect(self): cj = CookieJar() interact_netscape(cj, "http://www.example.com/", "spam=eggs") - hh = MockHTTPHandler(302, "Location: http://www.cracker.com/\r\n\r\n") + hh = MockHTTPHandlerRedirect(302, "Location: http://www.cracker.com/\r\n\r\n") hdeh = urllib.request.HTTPDefaultErrorHandler() hrh = urllib.request.HTTPRedirectHandler() cp = urllib.request.HTTPCookieProcessor(cj) @@ -1299,7 +1331,7 @@ def test_cookie_redirect(self): def test_redirect_fragment(self): redirected_url = 'http://www.example.com/index.html#OK\r\n\r\n' - hh = MockHTTPHandler(302, 'Location: ' + redirected_url) + hh = MockHTTPHandlerRedirect(302, 'Location: ' + redirected_url) hdeh = urllib.request.HTTPDefaultErrorHandler() hrh = urllib.request.HTTPRedirectHandler() o = build_test_opener(hh, hdeh, hrh) @@ -1484,7 +1516,7 @@ def check_basic_auth(self, headers, realm): password_manager = MockPasswordManager() auth_handler = urllib.request.HTTPBasicAuthHandler(password_manager) body = '\r\n'.join(headers) + '\r\n\r\n' - http_handler = MockHTTPHandler(401, body) + http_handler = MockHTTPHandlerRedirect(401, body) opener.add_handler(auth_handler) opener.add_handler(http_handler) self._test_basic_auth(opener, auth_handler, "Authorization", @@ -1544,7 +1576,7 @@ def test_proxy_basic_auth(self): password_manager = MockPasswordManager() auth_handler = urllib.request.ProxyBasicAuthHandler(password_manager) realm = "ACME Networks" - http_handler = MockHTTPHandler( + http_handler = MockHTTPHandlerRedirect( 407, 'Proxy-Authenticate: Basic realm="%s"\r\n\r\n' % realm) opener.add_handler(auth_handler) opener.add_handler(http_handler) @@ -1588,7 +1620,7 @@ def http_error_401(self, *args, **kwds): digest_handler = TestDigestAuthHandler(password_manager) basic_handler = TestBasicAuthHandler(password_manager) realm = "ACME Networks" - http_handler = MockHTTPHandler( + http_handler = MockHTTPHandlerRedirect( 401, 'WWW-Authenticate: Basic realm="%s"\r\n\r\n' % realm) opener.add_handler(basic_handler) opener.add_handler(digest_handler) @@ -1608,7 +1640,7 @@ def test_unsupported_auth_digest_handler(self): opener = OpenerDirector() # While using DigestAuthHandler digest_auth_handler = urllib.request.HTTPDigestAuthHandler(None) - http_handler = MockHTTPHandler( + http_handler = MockHTTPHandlerRedirect( 401, 'WWW-Authenticate: Kerberos\r\n\r\n') opener.add_handler(digest_auth_handler) opener.add_handler(http_handler) @@ -1618,7 +1650,7 @@ def test_unsupported_auth_basic_handler(self): # While using BasicAuthHandler opener = OpenerDirector() basic_auth_handler = urllib.request.HTTPBasicAuthHandler(None) - http_handler = MockHTTPHandler( + http_handler = MockHTTPHandlerRedirect( 401, 'WWW-Authenticate: NTLM\r\n\r\n') opener.add_handler(basic_auth_handler) opener.add_handler(http_handler) @@ -1705,7 +1737,7 @@ def test_basic_prior_auth_send_after_first_success(self): opener = OpenerDirector() opener.add_handler(auth_prior_handler) - http_handler = MockHTTPHandler( + http_handler = MockHTTPHandlerRedirect( 401, 'WWW-Authenticate: Basic realm="%s"\r\n\r\n' % None) opener.add_handler(http_handler) From c6f5ee425c10223bdc194ff689da59d69656c8c2 Mon Sep 17 00:00:00 2001 From: Wheeler Law Date: Thu, 10 Nov 2022 16:27:01 -0600 Subject: [PATCH 3/5] add news --- .../next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst diff --git a/Misc/NEWS.d/next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst b/Misc/NEWS.d/next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst new file mode 100644 index 00000000000000..814fc4413c1187 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst @@ -0,0 +1,3 @@ +Respect http.client.HTTPConnection.debuglevel flag in +urllib.request.AbstractHTTPHandler when the AbstractHTTPHandler's +constructor parameter 'debuglevel' is not set. From 450b8f22f3830a9919842eefeab83a876c14ddad Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 7 Feb 2023 11:09:21 -0800 Subject: [PATCH 4/5] ReSTify NEWS and reword a bit. --- .../Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst b/Misc/NEWS.d/next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst index 814fc4413c1187..1ad42d5c9aa53d 100644 --- a/Misc/NEWS.d/next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst +++ b/Misc/NEWS.d/next/Library/2022-11-10-16-26-47.gh-issue-99353.DQFjnt.rst @@ -1,3 +1,3 @@ -Respect http.client.HTTPConnection.debuglevel flag in -urllib.request.AbstractHTTPHandler when the AbstractHTTPHandler's -constructor parameter 'debuglevel' is not set. +Respect the :class:`http.client.HTTPConnection` ``.debuglevel`` flag +in :class:`urllib.request.AbstractHTTPHandler` when its constructor +parameter ``debuglevel`` is not set. And do the same for ``*HTTPS*``. From ad096d481d40b8794ce096b09176746902e2ac9f Mon Sep 17 00:00:00 2001 From: Senthil Kumaran Date: Thu, 20 Apr 2023 18:28:30 -0700 Subject: [PATCH 5/5] Address Review Comments. * Use mock.patch.object instead of settting the module level value. * Used test values to assert the debuglevel. --- Lib/test/test_urllib2.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 530ac72463c233..b7c6f6dd8f1b99 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -3,6 +3,7 @@ from test.support import os_helper from test.support import warnings_helper from test import test_urllib +from unittest import mock import os import io @@ -483,6 +484,7 @@ def build_test_opener(*handler_instances): opener.add_handler(h) return opener + class MockHTTPHandler(urllib.request.HTTPHandler): # Very simple mock HTTP handler with no special behavior other than using a mock HTTP connection @@ -493,6 +495,7 @@ def __init__(self, debuglevel=None): def http_open(self, req): return self.do_open(self.httpconn, req) + class MockHTTPHandlerRedirect(urllib.request.BaseHandler): # useful for testing redirections and auth # sends supplied headers and code as first response @@ -1058,34 +1061,34 @@ def test_http_body_array(self): self.assertEqual(int(newreq.get_header('Content-length')),16) def test_http_handler_global_debuglevel(self): - http.client.HTTPConnection.debuglevel = 1 - o = OpenerDirector() - h = MockHTTPHandler() - o.add_handler(h) - o.open("http://www.example.com") - self.assertEqual(h._debuglevel, 1) + with mock.patch.object(http.client.HTTPConnection, 'debuglevel', 6): + o = OpenerDirector() + h = MockHTTPHandler() + o.add_handler(h) + o.open("http://www.example.com") + self.assertEqual(h._debuglevel, 6) def test_http_handler_local_debuglevel(self): o = OpenerDirector() - h = MockHTTPHandler(debuglevel=1) + h = MockHTTPHandler(debuglevel=5) o.add_handler(h) o.open("http://www.example.com") - self.assertEqual(h._debuglevel, 1) + self.assertEqual(h._debuglevel, 5) def test_https_handler_global_debuglevel(self): - http.client.HTTPSConnection.debuglevel = 1 - o = OpenerDirector() - h = MockHTTPSHandler() - o.add_handler(h) - o.open("https://www.example.com") - self.assertEqual(h._debuglevel, 1) + with mock.patch.object(http.client.HTTPSConnection, 'debuglevel', 7): + o = OpenerDirector() + h = MockHTTPSHandler() + o.add_handler(h) + o.open("https://www.example.com") + self.assertEqual(h._debuglevel, 7) def test_https_handler_local_debuglevel(self): o = OpenerDirector() - h = MockHTTPSHandler(debuglevel=1) + h = MockHTTPSHandler(debuglevel=4) o.add_handler(h) o.open("https://www.example.com") - self.assertEqual(h._debuglevel, 1) + self.assertEqual(h._debuglevel, 4) def test_http_doubleslash(self): # Checks the presence of any unnecessary double slash in url does not