Issue #25738: Don’t send message body for 205 Reset Content

Patch by Susumu Koshiba.

diff --git a/Doc/library/basehttpserver.rst b/Doc/library/basehttpserver.rst
index 01776af50b9fa1a8e64f224552be16d24ab95c14..0c663fec3096c54af508fd13a23b5d707aa7d21d 100644 (file)
--- a/Doc/library/basehttpserver.rst
+++ b/Doc/library/basehttpserver.rst
@@ -197,7 +197,10 @@ to a handler.  Code to create and run the server looks like this::
       Sends and logs a complete error reply to the client. The numeric *code*
       specifies the HTTP error code, with *message* as optional, more specific text. A
       complete set of headers is sent, followed by text composed using the
-      :attr:`error_message_format` class variable.
+      :attr:`error_message_format` class variable. 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``.
 
 
    .. method:: send_response(code[, message])

diff --git a/Lib/BaseHTTPServer.py b/Lib/BaseHTTPServer.py
index deaf2f960b83c76b38b0c494db91202c70886833..3df3323a97eae69d0284df9280d01a2dd45a2add 100644 (file)
--- a/Lib/BaseHTTPServer.py
+++ b/Lib/BaseHTTPServer.py
@@ -362,14 +362,25 @@ class BaseHTTPRequestHandler(SocketServer.StreamRequestHandler):
             message = short
         explain = long
         self.log_error("code %d, message %s", code, message)
-        # using _quote_html to prevent Cross Site Scripting attacks (see bug #1100201)
-        content = (self.error_message_format %
-                   {'code': code, 'message': _quote_html(message), 'explain': explain})
         self.send_response(code, message)
-        self.send_header("Content-Type", self.error_content_type)
         self.send_header('Connection', 'close')
+
+        # 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)
+        content = None
+        if code >= 200 and code not in (204, 205, 304):
+            # HTML encode to prevent Cross Site Scripting attacks
+            # (see bug #1100201)
+            content = (self.error_message_format % {
+                'code': code,
+                'message': _quote_html(message),
+                'explain': explain
+            })
+            self.send_header("Content-Type", self.error_content_type)
         self.end_headers()
-        if self.command != 'HEAD' and code >= 200 and code not in (204, 304):
+
+        if self.command != 'HEAD' and content:
             self.wfile.write(content)
 
     error_message_format = DEFAULT_ERROR_MESSAGE

diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py
index 672c187a42836d23ced26ffe2dd13bc929639e87..1b6339d235d8cf726b1942ed73d4a710c7ae17c7 100644 (file)
--- a/Lib/test/test_httpservers.py
+++ b/Lib/test/test_httpservers.py
@@ -178,6 +178,12 @@ class BaseHTTPServerTestCase(BaseTestCase):
             self.send_header('Connection', 'close')
             self.end_headers()
 
+        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 = httplib.HTTPConnection('localhost', self.PORT)
@@ -276,6 +282,38 @@ class BaseHTTPServerTestCase(BaseTestCase):
         res = self.con.getresponse()
         self.assertEqual(res.status, 999)
 
+    def test_send_error(self):
+        allow_transfer_encoding_codes = (205, 304)
+        for code in (101, 102, 204, 205, 304):
+            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 = (205, 304)
+        for code in (101, 200, 204, 205, 304):
+            self.con.request('HEAD', '/{}'.format(code))
+            res = self.con.getresponse()
+            self.assertEqual(code, res.status)
+            if code == 200:
+                self.assertEqual(None, res.getheader('Content-Length'))
+                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 SimpleHTTPServerTestCase(BaseTestCase):
     class request_handler(NoLogRequestHandler, SimpleHTTPRequestHandler):

diff --git a/Misc/ACKS b/Misc/ACKS
index 57618b7bb09b1d07038f3abf51e5f2251badad2b..ee3a465e34aa14f35ea67fd6414b4a61626b0e75 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -741,6 +741,7 @@ Peter A. Koren
 Марк Коренберг
 Vlad Korolev
 Anna Koroliuk
+Susumu Koshiba
 Joseph Koshy
 Daniel Kozan
 Jerzy Kozera

diff --git a/Misc/NEWS b/Misc/NEWS
index d555bf5f89ccaaa2e9d5b185f8cfd68d547a800e..6359e7cb98f0bcce15655fd8378da4e77bdb10ec 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -92,6 +92,11 @@ Core and Builtins
 Library
 -------
 
+- Issue #25738: Stop BaseHTTPServer.BaseHTTPRequestHandler.send_error() from
+  sending a message body for 205 Reset Content.  Also, don't send the
+  Content-Type header field in responses that don't have a body.  Based on
+  patch by Susumu Koshiba.
+
 - Issue #21313: Fix the "platform" module to tolerate when sys.version
   contains truncated build information.