Issue #25738: Merge HTTP server from 3.5

vadmium · vadmium · commit 40de69ac588c · 2016-06-08T09:45:58.000Z
diff --git a/Doc/library/http.server.rst b/Doc/library/http.server.rst
@@ -191,7 +191,9 @@ of which this module provides three different variants:
       a complete set of headers, as the response body.  The :attr:`responses`
       attribute holds the default values for *message* and *explain* that
       will be used if no value is provided; for unknown codes the default value
-      for both is the string ``???``.
+      for both is the string ``???``. The body will be empty if the method is
+      HEAD or the response code is one of the following: ``1xx``,
+      ``204 No Content``, ``205 Reset Content``, ``304 Not Modified``.
 
       .. versionchanged:: 3.4
          The error response includes a Content-Length header.
diff --git a/Lib/http/server.py b/Lib/http/server.py
@@ -446,23 +446,30 @@ def send_error(self, code, message=None, explain=None):
         if explain is None:
             explain = longmsg
         self.log_error("code %d, message %s", code, message)
-        # HTML encode to prevent Cross Site Scripting attacks (see bug #1100201)
-        content = (self.error_message_format % {
-            'code': code,
-            'message': html.escape(message, quote=False),
-            'explain': html.escape(explain, quote=False)
-        })
-        body = content.encode('UTF-8', 'replace')
         self.send_response(code, message)
-        self.send_header("Content-Type", self.error_content_type)
         self.send_header('Connection', 'close')
-        self.send_header('Content-Length', int(len(body)))
+
+        # Message body is omitted for cases described in:
+        #  - RFC7230: 3.3. 1xx, 204(No Content), 304(Not Modified)
+        #  - RFC7231: 6.3.6. 205(Reset Content)
+        body = None
+        if (code >= 200 and
+            code not in (HTTPStatus.NO_CONTENT,
+                         HTTPStatus.RESET_CONTENT,
+                         HTTPStatus.NOT_MODIFIED)):
+            # HTML encode to prevent Cross Site Scripting attacks
+            # (see bug #1100201)
+            content = (self.error_message_format % {
+                'code': code,
+                'message': html.escape(message, quote=False),
+                'explain': html.escape(explain, quote=False)
+            })
+            body = content.encode('UTF-8', 'replace')
+            self.send_header("Content-Type", self.error_content_type)
+            self.send_header('Content-Length', int(len(body)))
         self.end_headers()
 
-        if (self.command != 'HEAD' and
-                code >= 200 and
-                code not in (
-                    HTTPStatus.NO_CONTENT, HTTPStatus.NOT_MODIFIED)):
+        if self.command != 'HEAD' and body:
             self.wfile.write(body)
 
     def send_response(self, code, message=None):
diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py
@@ -116,6 +116,12 @@ def do_LATINONEHEADER(self):
             body = self.headers['x-special-incoming'].encode('utf-8')
             self.wfile.write(body)
 
+        def do_SEND_ERROR(self):
+            self.send_error(int(self.path[1:]))
+
+        def do_HEAD(self):
+            self.send_error(int(self.path[1:]))
+
     def setUp(self):
         BaseTestCase.setUp(self)
         self.con = http.client.HTTPConnection(self.HOST, self.PORT)
@@ -237,6 +243,44 @@ def test_error_content_length(self):
         data = res.read()
         self.assertEqual(int(res.getheader('Content-Length')), len(data))
 
+    def test_send_error(self):
+        allow_transfer_encoding_codes = (HTTPStatus.NOT_MODIFIED,
+                                         HTTPStatus.RESET_CONTENT)
+        for code in (HTTPStatus.NO_CONTENT, HTTPStatus.NOT_MODIFIED,
+                     HTTPStatus.PROCESSING, HTTPStatus.RESET_CONTENT,
+                     HTTPStatus.SWITCHING_PROTOCOLS):
+            self.con.request('SEND_ERROR', '/{}'.format(code))
+            res = self.con.getresponse()
+            self.assertEqual(code, res.status)
+            self.assertEqual(None, res.getheader('Content-Length'))
+            self.assertEqual(None, res.getheader('Content-Type'))
+            if code not in allow_transfer_encoding_codes:
+                self.assertEqual(None, res.getheader('Transfer-Encoding'))
+
+            data = res.read()
+            self.assertEqual(b'', data)
+
+    def test_head_via_send_error(self):
+        allow_transfer_encoding_codes = (HTTPStatus.NOT_MODIFIED,
+                                         HTTPStatus.RESET_CONTENT)
+        for code in (HTTPStatus.OK, HTTPStatus.NO_CONTENT,
+                     HTTPStatus.NOT_MODIFIED, HTTPStatus.RESET_CONTENT,
+                     HTTPStatus.SWITCHING_PROTOCOLS):
+            self.con.request('HEAD', '/{}'.format(code))
+            res = self.con.getresponse()
+            self.assertEqual(code, res.status)
+            if code == HTTPStatus.OK:
+                self.assertTrue(int(res.getheader('Content-Length')) > 0)
+                self.assertIn('text/html', res.getheader('Content-Type'))
+            else:
+                self.assertEqual(None, res.getheader('Content-Length'))
+                self.assertEqual(None, res.getheader('Content-Type'))
+            if code not in allow_transfer_encoding_codes:
+                self.assertEqual(None, res.getheader('Transfer-Encoding'))
+
+            data = res.read()
+            self.assertEqual(b'', data)
+
 
 class RequestHandlerLoggingTestCase(BaseTestCase):
     class request_handler(BaseHTTPRequestHandler):
diff --git a/Misc/ACKS b/Misc/ACKS
@@ -787,6 +787,7 @@ Arkady Koplyarov
 Peter A. Koren
 Марк Коренберг
 Vlad Korolev
+Susumu Koshiba
 Joseph Koshy
 Daniel Kozan
 Jerzy Kozera
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -27,6 +27,11 @@ Core and Builtins
 Library
 -------
 
+- Issue #25738: Stop http.server.BaseHTTPRequestHandler.send_error() from
+  sending a message body for 205 Reset Content.  Also, don't send Content
+  header fields in responses that don't have a body.  Patch by Susumu
+  Koshiba.
+
 - Issue #21313: Fix the "platform" module to tolerate when sys.version
   contains truncated build information.
 
