From df42e6ea80ca7c9212a39646ba30b9d8136e1e2c Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Fri, 17 May 2019 21:40:43 +0530 Subject: [PATCH 1/9] Fix NameError in urllib.request.URLopener.retrieve --- Lib/test/test_urllib.py | 20 +++++++++++++++++++- Lib/urllib/request.py | 10 +++++----- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index 7214492eca9d88..dec5c73ae9d690 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -1445,7 +1445,7 @@ def test_thishost(self): self.assertIsInstance(urllib.request.thishost(), tuple) -class URLopener_Tests(unittest.TestCase): +class URLopener_Tests(unittest.TestCase, FakeHTTPMixin): """Testcase to test the open method of URLopener class.""" def test_quoted_open(self): @@ -1463,6 +1463,24 @@ def open_spam(self, url): "spam://c:|windows%/:=&?~#+!$,;'@()*[]|/path/"), "//c:|windows%/:=&?~#+!$,;'@()*[]|/path/") + def test_urlopener_retrieve_file(self): + with tempfile.NamedTemporaryFile() as file_obj: + with self.assertWarns(DeprecationWarning): + url = f'file://{file_obj.name}' + filename, _ = urllib.request.URLopener().retrieve(url) + self.assertEqual(filename, file_obj.name) + + def test_urlopener_retrieve_remote(self): + with self.assertWarns(DeprecationWarning): + try: + self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello!") + url = "http://www.python.org/file.txt" + filename, _ = urllib.request.URLopener().retrieve(url) + self.assertEqual(os.path.splitext(filename)[1], '.txt') + finally: + self.unfakehttp() + + # Just commented them out. # Can't really tell why keep failing in windows and sparc. # Everywhere else they work ok, but on those machines, sometimes diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py index df2ff06f0fc9a2..230ac390abb332 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -1783,7 +1783,7 @@ def retrieve(self, url, filename=None, reporthook=None, data=None): fp = self.open_local_file(url1) hdrs = fp.info() fp.close() - return url2pathname(splithost(url1)[1]), hdrs + return url2pathname(_splithost(url1)[1]), hdrs except OSError as msg: pass fp = self.open(url, data) @@ -1792,10 +1792,10 @@ def retrieve(self, url, filename=None, reporthook=None, data=None): if filename: tfp = open(filename, 'wb') else: - garbage, path = splittype(url) - garbage, path = splithost(path or "") - path, garbage = splitquery(path or "") - path, garbage = splitattr(path or "") + garbage, path = _splittype(url) + garbage, path = _splithost(path or "") + path, garbage = _splitquery(path or "") + path, garbage = _splitattr(path or "") suffix = os.path.splitext(path)[1] (fd, filename) = tempfile.mkstemp(suffix) self.__tempfiles.append(filename) From 9eafb47d604256be15dd237b36e3fb82749a4f23 Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Fri, 17 May 2019 21:43:10 +0530 Subject: [PATCH 2/9] Add NEWS entry --- .../next/Library/2019-05-17-21-42-58.bpo-36948.vnUDvk.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-05-17-21-42-58.bpo-36948.vnUDvk.rst diff --git a/Misc/NEWS.d/next/Library/2019-05-17-21-42-58.bpo-36948.vnUDvk.rst b/Misc/NEWS.d/next/Library/2019-05-17-21-42-58.bpo-36948.vnUDvk.rst new file mode 100644 index 00000000000000..c8cfa54067fd35 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-05-17-21-42-58.bpo-36948.vnUDvk.rst @@ -0,0 +1,2 @@ +Fix :exc:`NameError` in :meth:`urllib.request.URLopener.retrieve`. Patch by +Karthikeyan Singaravelan. From 63ef750c1b78900ada86d43a91af7522d71288ab Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Fri, 17 May 2019 22:06:28 +0530 Subject: [PATCH 3/9] Use localhost in local file test for Windows --- Lib/test/test_urllib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index dec5c73ae9d690..8882d912c77b37 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -1466,7 +1466,7 @@ def open_spam(self, url): def test_urlopener_retrieve_file(self): with tempfile.NamedTemporaryFile() as file_obj: with self.assertWarns(DeprecationWarning): - url = f'file://{file_obj.name}' + url = f'file://localhost{file_obj.name}' filename, _ = urllib.request.URLopener().retrieve(url) self.assertEqual(filename, file_obj.name) From ea3599c634dd6635a5100480b4e70b8e3bffc08c Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Sat, 18 May 2019 01:43:43 +0530 Subject: [PATCH 4/9] Refactor test for windows --- Lib/test/test_urllib.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index 8882d912c77b37..b8c860e7e76cbb 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -1464,11 +1464,15 @@ def open_spam(self, url): "//c:|windows%/:=&?~#+!$,;'@()*[]|/path/") def test_urlopener_retrieve_file(self): - with tempfile.NamedTemporaryFile() as file_obj: - with self.assertWarns(DeprecationWarning): - url = f'file://localhost{file_obj.name}' - filename, _ = urllib.request.URLopener().retrieve(url) - self.assertEqual(filename, file_obj.name) + with self.assertWarns(DeprecationWarning): + try: + fd, tmp_file = tempfile.mkstemp() + fileurl = 'file:' + urllib.request.pathname2url(tmp_file) + filename, _ = urllib.request.URLopener().retrieve(fileurl) + self.assertEqual(filename, tmp_file) + finally: + os.close(fd) + os.unlink(tmp_file) def test_urlopener_retrieve_remote(self): with self.assertWarns(DeprecationWarning): From 5e5d91492ba306c98d229144bd53b763df3ff0f5 Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Sun, 19 May 2019 07:23:49 +0530 Subject: [PATCH 5/9] Fix review comments --- Lib/test/test_urllib.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index b8c860e7e76cbb..a466c24c10d9b1 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -1463,26 +1463,20 @@ def open_spam(self, url): "spam://c:|windows%/:=&?~#+!$,;'@()*[]|/path/"), "//c:|windows%/:=&?~#+!$,;'@()*[]|/path/") + @support.ignore_warnings(category=DeprecationWarning) def test_urlopener_retrieve_file(self): - with self.assertWarns(DeprecationWarning): - try: - fd, tmp_file = tempfile.mkstemp() - fileurl = 'file:' + urllib.request.pathname2url(tmp_file) - filename, _ = urllib.request.URLopener().retrieve(fileurl) - self.assertEqual(filename, tmp_file) - finally: - os.close(fd) - os.unlink(tmp_file) + with tempfile.NamedTemporaryFile() as fileobj: + fileurl = "file:" + urllib.request.pathname2url(fileobj.name) + filename, _ = urllib.request.URLopener().retrieve(fileurl) + self.assertEqual(filename, fileobj.name) + @support.ignore_warnings(category=DeprecationWarning) def test_urlopener_retrieve_remote(self): - with self.assertWarns(DeprecationWarning): - try: - self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello!") - url = "http://www.python.org/file.txt" - filename, _ = urllib.request.URLopener().retrieve(url) - self.assertEqual(os.path.splitext(filename)[1], '.txt') - finally: - self.unfakehttp() + url = "http://www.python.org/file.txt" + self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello!") + self.addCleanup(self.unfakehttp) + filename, _ = urllib.request.URLopener().retrieve(url) + self.assertEqual(os.path.splitext(filename)[1], ".txt") # Just commented them out. From 21059430c767e161c1da9be1e47e27c61b9d05a5 Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Sun, 19 May 2019 07:40:57 +0530 Subject: [PATCH 6/9] Add mkstemp for windows --- Lib/test/test_urllib.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index a466c24c10d9b1..3270fc40e21ee6 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -1465,10 +1465,12 @@ def open_spam(self, url): @support.ignore_warnings(category=DeprecationWarning) def test_urlopener_retrieve_file(self): - with tempfile.NamedTemporaryFile() as fileobj: - fileurl = "file:" + urllib.request.pathname2url(fileobj.name) - filename, _ = urllib.request.URLopener().retrieve(fileurl) - self.assertEqual(filename, fileobj.name) + fd, tmpfile = tempfile.mkstemp() + self.addCleanup(os.close, fd) + self.addCleanup(os.unlink, tmpfile) + fileurl = "file:" + urllib.request.pathname2url(tmpfile) + filename, _ = urllib.request.URLopener().retrieve(fileurl) + self.assertEqual(filename, tmpfile) @support.ignore_warnings(category=DeprecationWarning) def test_urlopener_retrieve_remote(self): From 15675e38545e3d954e602092f565b121686e52bc Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Sun, 19 May 2019 08:09:42 +0530 Subject: [PATCH 7/9] Add combination of tmpdir and mkstemp to avoid file being used error in Windows --- Lib/test/test_urllib.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index 3270fc40e21ee6..46fdbd61f7fbfa 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -1465,12 +1465,12 @@ def open_spam(self, url): @support.ignore_warnings(category=DeprecationWarning) def test_urlopener_retrieve_file(self): - fd, tmpfile = tempfile.mkstemp() - self.addCleanup(os.close, fd) - self.addCleanup(os.unlink, tmpfile) - fileurl = "file:" + urllib.request.pathname2url(tmpfile) - filename, _ = urllib.request.URLopener().retrieve(fileurl) - self.assertEqual(filename, tmpfile) + with support.temp_dir() as tmpdir: + fd, tmpfile = tempfile.mkstemp(dir=tmpdir) + self.addCleanup(os.close, fd) + fileurl = "file:" + urllib.request.pathname2url(tmpfile) + filename, _ = urllib.request.URLopener().retrieve(fileurl) + self.assertEqual(filename, tmpfile) @support.ignore_warnings(category=DeprecationWarning) def test_urlopener_retrieve_remote(self): From 6061de07152aab4a86901411ef2b613718cc0b76 Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Sun, 19 May 2019 08:22:43 +0530 Subject: [PATCH 8/9] Close file descriptor since context manager needs to remove the temp directory. See if used by other process error is fixed with this in Windows. --- Lib/test/test_urllib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index 46fdbd61f7fbfa..4b8ce109e879c2 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -1467,7 +1467,7 @@ def open_spam(self, url): def test_urlopener_retrieve_file(self): with support.temp_dir() as tmpdir: fd, tmpfile = tempfile.mkstemp(dir=tmpdir) - self.addCleanup(os.close, fd) + os.close(fd) fileurl = "file:" + urllib.request.pathname2url(tmpfile) filename, _ = urllib.request.URLopener().retrieve(fileurl) self.assertEqual(filename, tmpfile) From 7defa55f059e80e8a9c31193e371ace097b01f85 Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Sun, 19 May 2019 18:44:31 +0530 Subject: [PATCH 9/9] Mixin before TestCase --- Lib/test/test_urllib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index 4b8ce109e879c2..74b19fbdcd8dbe 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -1445,7 +1445,7 @@ def test_thishost(self): self.assertIsInstance(urllib.request.thishost(), tuple) -class URLopener_Tests(unittest.TestCase, FakeHTTPMixin): +class URLopener_Tests(FakeHTTPMixin, unittest.TestCase): """Testcase to test the open method of URLopener class.""" def test_quoted_open(self):