Issue #16464: reset Request's Content-Length header on .data change.

It will be recalculated on sending request to HTTP server.

Patch by Alexey Kachayev

diff --git a/Doc/library/urllib.request.rst b/Doc/library/urllib.request.rst
index 84c387c8c821bc26835d5ce9b21505d610fca4f4..65637fd22d3774ce32e01c8505d26b751cc6c0ee 100644 (file)
--- a/Doc/library/urllib.request.rst
+++ b/Doc/library/urllib.request.rst
@@ -408,6 +408,10 @@ request.
 
    The entity body for the request, or None if not specified.
 
+   .. versionchanged:: 3.4
+      Changing value of :attr:`Request.data` now deletes "Content-Length"
+      header if it was previously set or calculated.
+
 .. attribute:: Request.unverifiable
 
    boolean, indicates whether the request is unverifiable as defined
@@ -456,6 +460,12 @@ request.
    unredirected).
 
 
+.. method:: Request.remove_header(header)
+
+   Remove named header from the request instance (both from regular and
+   unredirected headers).
+
+
 .. method:: Request.get_full_url()
 
    Return the URL given in the constructor.

diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
index 00ee66987422af240d91e0a93db7c65cb261f123..2261d5704757313fad3643a997253b811e913722 100644 (file)
--- a/Lib/test/test_urllib2.py
+++ b/Lib/test/test_urllib2.py
@@ -124,6 +124,19 @@ def test_request_headers_methods():
     >>> r.get_header("Not-there", "default")
     'default'
 
+    Method r.remove_header should remove items both from r.headers and
+    r.unredirected_hdrs dictionaries
+
+    >>> r.remove_header("Spam-eggs")
+    >>> r.has_header("Spam-eggs")
+    False
+    >>> r.add_unredirected_header("Unredirected-spam", "Eggs")
+    >>> r.has_header("Unredirected-spam")
+    True
+    >>> r.remove_header("Unredirected-spam")
+    >>> r.has_header("Unredirected-spam")
+    False
+
     """
 
 
@@ -1432,6 +1445,20 @@ class MiscTests(unittest.TestCase):
         self.opener_has_handler(o, MyHTTPHandler)
         self.opener_has_handler(o, MyOtherHTTPHandler)
 
+    def test_issue16464(self):
+        opener = urllib.request.build_opener()
+        request = urllib.request.Request("http://www.python.org/~jeremy/")
+        self.assertEqual(None, request.data)
+
+        opener.open(request, "1".encode("us-ascii"))
+        self.assertEqual(b"1", request.data)
+        self.assertEqual("1", request.get_header("Content-length"))
+
+        opener.open(request, "1234567890".encode("us-ascii"))
+        self.assertEqual(b"1234567890", request.data)
+        self.assertEqual("10", request.get_header("Content-length"))
+
+
     def opener_has_handler(self, opener, handler_class):
         self.assertTrue(any(h.__class__ == handler_class
                             for h in opener.handlers))
@@ -1455,6 +1482,16 @@ class RequestTests(unittest.TestCase):
         self.assertTrue(self.get.data)
         self.assertEqual("POST", self.get.get_method())
 
+    # issue 16464
+    # if we change data we need to remove content-length header
+    # (cause it's most probably calculated for previous value)
+    def test_setting_data_should_remove_content_length(self):
+        self.assertFalse("Content-length" in self.get.unredirected_hdrs)
+        self.get.add_unredirected_header("Content-length", 42)
+        self.assertEqual(42, self.get.unredirected_hdrs["Content-length"])
+        self.get.data = "spam"
+        self.assertFalse("Content-length" in self.get.unredirected_hdrs)
+
     def test_get_full_url(self):
         self.assertEqual("http://www.python.org/~jeremy/",
                          self.get.get_full_url())

diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
index 74725f98dda929eae8c6f247dbf02c4df0fbb18a..ef4bf7fb63b878677d7d7d6e2170a6f7331fca0f 100644 (file)
--- a/Lib/urllib/request.py
+++ b/Lib/urllib/request.py
@@ -266,12 +266,13 @@ class Request:
         # unwrap('<URL:type://host/path>') --> 'type://host/path'
         self.full_url = unwrap(url)
         self.full_url, self.fragment = splittag(self.full_url)
-        self.data = data
         self.headers = {}
+        self.unredirected_hdrs = {}
+        self._data = None
+        self.data = data
         self._tunnel_host = None
         for key, value in headers.items():
             self.add_header(key, value)
-        self.unredirected_hdrs = {}
         if origin_req_host is None:
             origin_req_host = request_host(self)
         self.origin_req_host = origin_req_host
@@ -279,6 +280,24 @@ class Request:
         self.method = method
         self._parse()
 
+    @property
+    def data(self):
+        return self._data
+
+    @data.setter
+    def data(self, data):
+        if data != self._data:
+            self._data = data
+            # issue 16464
+            # if we change data we need to remove content-length header
+            # (cause it's most probably calculated for previous value)
+            if self.has_header("Content-length"):
+                self.remove_header("Content-length")
+
+    @data.deleter
+    def data(self):
+        self._data = None
+
     def _parse(self):
         self.type, rest = splittype(self.full_url)
         if self.type is None:
@@ -374,6 +393,10 @@ class Request:
             header_name,
             self.unredirected_hdrs.get(header_name, default))
 
+    def remove_header(self, header_name):
+        self.headers.pop(header_name, None)
+        self.unredirected_hdrs.pop(header_name, None)
+
     def header_items(self):
         hdrs = self.unredirected_hdrs.copy()
         hdrs.update(self.headers)