Issue #17272: Making the urllib.request's Request.full_url a descriptor. Fixes

bugs with assignment to full_url. Patch by Demian Brecht.

4 files changed, 48 insertions, 7 deletions

diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
index 089e04c..b4f940c 100644
--- a/Lib/test/test_urllib2.py
+++ b/Lib/test/test_urllib2.py
@@ -904,6 +904,30 @@ class HandlerTests(unittest.TestCase):
             p_ds_req = h.do_request_(ds_req)
             self.assertEqual(p_ds_req.unredirected_hdrs["Host"],"example.com")
 
+    def test_full_url_setter(self):
+        # Checks to ensure that components are set correctly after setting the
+        # full_url of a Request object
+
+        urls = [
+            'http://example.com?foo=bar#baz',
+            'http://example.com?foo=bar&spam=eggs#bash',
+            'http://example.com',
+        ]
+
+        # testing a reusable request instance, but the url parameter is
+        # required, so just use a dummy one to instantiate
+        r = Request('http://example.com')
+        for url in urls:
+            r.full_url = url
+            self.assertEqual(r.get_full_url(), url)
+
+    def test_full_url_deleter(self):
+        r = Request('http://www.example.com')
+        del r.full_url
+        self.assertIsNone(r.full_url)
+        self.assertIsNone(r.fragment)
+        self.assertEqual(r.selector, '')
+
     def test_fixpath_in_weirdurls(self):
         # Issue4493: urllib2 to supply '/' when to urls where path does not
         # start with'/'
diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
index 17c9251..fb86335 100644
--- a/Lib/urllib/request.py
+++ b/Lib/urllib/request.py
@@ -259,9 +259,7 @@ class Request:
     def __init__(self, url, data=None, headers={},
                  origin_req_host=None, unverifiable=False,
                  method=None):
-        # unwrap('<URL:type://host/path>') --> 'type://host/path'
-        self.full_url = unwrap(url)
-        self.full_url, self.fragment = splittag(self.full_url)
+        self.full_url = url
         self.headers = {}
         self.unredirected_hdrs = {}
         self._data = None
@@ -274,8 +272,24 @@ class Request:
         self.origin_req_host = origin_req_host
         self.unverifiable = unverifiable
         self.method = method
+
+    @property
+    def full_url(self):
+        return self._full_url
+
+    @full_url.setter
+    def full_url(self, url):
+        # unwrap('<URL:type://host/path>') --> 'type://host/path'
+        self._full_url = unwrap(url)
+        self._full_url, self.fragment = splittag(self._full_url)
         self._parse()
 
+    @full_url.deleter
+    def full_url(self):
+        self._full_url = None
+        self.fragment = None
+        self.selector = ''
+
     @property
     def data(self):
         return self._data
@@ -295,7 +309,7 @@ class Request:
         self.data = None
 
     def _parse(self):
-        self.type, rest = splittype(self.full_url)
+        self.type, rest = splittype(self._full_url)
         if self.type is None:
             raise ValueError("unknown url type: %r" % self.full_url)
         self.host, self.selector = splithost(rest)
@@ -313,9 +327,8 @@ class Request:
 
     def get_full_url(self):
         if self.fragment:
-            return '%s#%s' % (self.full_url, self.fragment)
-        else:
-            return self.full_url
+            return '{}#{}'.format(self.full_url, self.fragment)
+        return self.full_url
 
     def set_proxy(self, host, type):
         if self.type == 'https' and not self._tunnel_host:
diff --git a/Misc/ACKS b/Misc/ACKS
index 3520e96..997496a 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -153,6 +153,7 @@ Terrence Brannon
 Sven Brauch
 Erik Bray
 Brian Brazil
+Demian Brecht
 Dave Brennan
 Tom Bridgman
 Anthony Briggs
diff --git a/Misc/NEWS b/Misc/NEWS
index 0222556..a7d7207 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -49,6 +49,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #17272: Making the urllib.request's Request.full_url a descriptor.
+  Fixes bugs with assignment to full_url. Patch by Demian Brecht.
+
 - Issue #17353: Plistlib emitted empty data tags with deeply nested datastructures
 
 - Issue #11714: Use 'with' statements to assure a Semaphore releases a