Issue #12319: Always send file request bodies using chunked encoding

The previous attempt to determine the file’s Content-Length gave a false
positive for pipes on Windows.

Also, drop the special case for sending zero-length iterable bodies.

2 files changed, 54 insertions, 29 deletions

diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index a1796123e4..359e0bb94a 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -381,6 +381,16 @@ class TransferEncodingTest(TestCase):
             # same request
             self.assertNotIn('content-length', [k.lower() for k in headers])
 
+    def test_empty_body(self):
+        # Zero-length iterable should be treated like any other iterable
+        conn = client.HTTPConnection('example.com')
+        conn.sock = FakeSocket(b'')
+        conn.request('POST', '/', ())
+        _, headers, body = self._parse_request(conn.sock.data)
+        self.assertEqual(headers['Transfer-Encoding'], 'chunked')
+        self.assertNotIn('content-length', [k.lower() for k in headers])
+        self.assertEqual(body, b"0\r\n\r\n")
+
     def _make_body(self, empty_lines=False):
         lines = self.expected_body.split(b' ')
         for idx, line in enumerate(lines):
@@ -652,7 +662,9 @@ class BasicTest(TestCase):
 
     def test_send_file(self):
         expected = (b'GET /foo HTTP/1.1\r\nHost: example.com\r\n'
-                    b'Accept-Encoding: identity\r\nContent-Length:')
+                    b'Accept-Encoding: identity\r\n'
+                    b'Transfer-Encoding: chunked\r\n'
+                    b'\r\n')
 
         with open(__file__, 'rb') as body:
             conn = client.HTTPConnection('example.com')
@@ -1717,7 +1729,7 @@ class RequestBodyTest(TestCase):
         self.assertEqual("5", message.get("content-length"))
         self.assertEqual(b'body\xc1', f.read())
 
-    def test_file_body(self):
+    def test_text_file_body(self):
         self.addCleanup(support.unlink, support.TESTFN)
         with open(support.TESTFN, "w") as f:
             f.write("body")
@@ -1726,10 +1738,8 @@ class RequestBodyTest(TestCase):
             message, f = self.get_headers_and_fp()
             self.assertEqual("text/plain", message.get_content_type())
             self.assertIsNone(message.get_charset())
-            # Note that the length of text files is unpredictable
-            # because it depends on character encoding and line ending
-            # translation.  No content-length will be set, the body
-            # will be sent using chunked transfer encoding.
+            # No content-length will be determined for files; the body
+            # will be sent using chunked transfer encoding instead.
             self.assertIsNone(message.get("content-length"))
             self.assertEqual("chunked", message.get("transfer-encoding"))
             self.assertEqual(b'4\r\nbody\r\n0\r\n\r\n', f.read())
@@ -1743,8 +1753,9 @@ class RequestBodyTest(TestCase):
             message, f = self.get_headers_and_fp()
             self.assertEqual("text/plain", message.get_content_type())
             self.assertIsNone(message.get_charset())
-            self.assertEqual("5", message.get("content-length"))
-            self.assertEqual(b'body\xc1', f.read())
+            self.assertEqual("chunked", message.get("Transfer-Encoding"))
+            self.assertNotIn("Content-Length", message)
+            self.assertEqual(b'5\r\nbody\xc1\r\n0\r\n\r\n', f.read())
 
 
 class HTTPResponseTest(TestCase):
diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
index 0eea0c7f98..34329f8716 100644
--- a/Lib/test/test_urllib2.py
+++ b/Lib/test/test_urllib2.py
@@ -913,40 +913,50 @@ class HandlerTests(unittest.TestCase):
             self.assertEqual(req.unredirected_hdrs["Spam"], "foo")
 
     def test_http_body_file(self):
-        # A regular file - Content Length is calculated unless already set.
+        # A regular file - chunked encoding is used unless Content Length is
+        # already set.
 
         h = urllib.request.AbstractHTTPHandler()
         o = h.parent = MockOpener()
 
         file_obj = tempfile.NamedTemporaryFile(mode='w+b', delete=False)
         file_path = file_obj.name
-        file_obj.write(b"Something\nSomething\nSomething\n")
         file_obj.close()
+        self.addCleanup(os.unlink, file_path)
 
-        for headers in {}, {"Content-Length": 30}:
-            with open(file_path, "rb") as f:
-                req = Request("http://example.com/", f, headers)
-                newreq = h.do_request_(req)
-                self.assertEqual(int(newreq.get_header('Content-length')), 30)
+        with open(file_path, "rb") as f:
+            req = Request("http://example.com/", f, {})
+            newreq = h.do_request_(req)
+            te = newreq.get_header('Transfer-encoding')
+            self.assertEqual(te, "chunked")
+            self.assertFalse(newreq.has_header('Content-length'))
 
-        os.unlink(file_path)
+        with open(file_path, "rb") as f:
+            req = Request("http://example.com/", f, {"Content-Length": 30})
+            newreq = h.do_request_(req)
+            self.assertEqual(int(newreq.get_header('Content-length')), 30)
+            self.assertFalse(newreq.has_header("Transfer-encoding"))
 
     def test_http_body_fileobj(self):
-        # A file object - Content Length is calculated unless already set.
+        # A file object - chunked encoding is used
+        # unless Content Length is already set.
         # (Note that there are some subtle differences to a regular
         # file, that is why we are testing both cases.)
 
         h = urllib.request.AbstractHTTPHandler()
         o = h.parent = MockOpener()
-
         file_obj = io.BytesIO()
-        file_obj.write(b"Something\nSomething\nSomething\n")
 
-        for headers in {}, {"Content-Length": 30}:
-            file_obj.seek(0)
-            req = Request("http://example.com/", file_obj, headers)
-            newreq = h.do_request_(req)
-            self.assertEqual(int(newreq.get_header('Content-length')), 30)
+        req = Request("http://example.com/", file_obj, {})
+        newreq = h.do_request_(req)
+        self.assertEqual(newreq.get_header('Transfer-encoding'), 'chunked')
+        self.assertFalse(newreq.has_header('Content-length'))
+
+        headers = {"Content-Length": 30}
+        req = Request("http://example.com/", file_obj, headers)
+        newreq = h.do_request_(req)
+        self.assertEqual(int(newreq.get_header('Content-length')), 30)
+        self.assertFalse(newreq.has_header("Transfer-encoding"))
 
         file_obj.close()
 
@@ -959,9 +969,7 @@ class HandlerTests(unittest.TestCase):
         h = urllib.request.AbstractHTTPHandler()
         o = h.parent = MockOpener()
 
-        cmd = [sys.executable, "-c",
-               r"import sys; "
-               r"sys.stdout.buffer.write(b'Something\nSomething\nSomething\n')"]
+        cmd = [sys.executable, "-c", r"pass"]
         for headers in {}, {"Content-Length": 30}:
             with subprocess.Popen(cmd, stdout=subprocess.PIPE) as proc:
                 req = Request("http://example.com/", proc.stdout, headers)
@@ -983,8 +991,6 @@ class HandlerTests(unittest.TestCase):
 
         def iterable_body():
             yield b"one"
-            yield b"two"
-            yield b"three"
 
         for headers in {}, {"Content-Length": 11}:
             req = Request("http://example.com/", iterable_body(), headers)
@@ -996,6 +1002,14 @@ class HandlerTests(unittest.TestCase):
             else:
                 self.assertEqual(int(newreq.get_header('Content-length')), 11)
 
+    def test_http_body_empty_seq(self):
+        # Zero-length iterable body should be treated like any other iterable
+        h = urllib.request.AbstractHTTPHandler()
+        h.parent = MockOpener()
+        req = h.do_request_(Request("http://example.com/", ()))
+        self.assertEqual(req.get_header("Transfer-encoding"), "chunked")
+        self.assertFalse(req.has_header("Content-length"))
+
     def test_http_body_array(self):
         # array.array Iterable - Content Length is calculated