Issue #26585: Eliminate _quote_html() and use html.escape(quote=False)

Patch by Xiang Zhang.

Issue #26585: Eliminate _quote_html() and use html.escape(quote=False)
Patch by Xiang Zhang.
6c6ec01f · Martin Panter · 79c868b9 · 6c6ec01f · 6c6ec01f · 6c6ec01f
Commit 6c6ec01f authored Apr 11, 2016 by Martin Panter
Hide whitespace changes
Inline Side-by-side

Showing with 40 additions and 9 deletions

Lib/http/server.py Lib/http/server.py +8 -8

Lib/test/test_httpservers.py Lib/test/test_httpservers.py +29 -1

Misc/NEWS Misc/NEWS +3 -0

No files found.
--- a/Lib/http/server.py
+++ b/Lib/http/server.py
@@ -127,9 +127,6 @@ DEFAULT_ERROR_MESSAGE = """\

 DEFAULT_ERROR_CONTENT_TYPE = "text/html;charset=utf-8"

-def _quote_html(html):
-    return html.replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")
-
 class HTTPServer(socketserver.TCPServer):

    allow_reuse_address = 1    # Seems to make sense in testing environment
@@ -449,9 +446,12 @@ class BaseHTTPRequestHandler(socketserver.StreamRequestHandler):
        if explain is None:
            explain = longmsg
        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': _quote_html(explain)})
+        # 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)
@@ -710,7 +710,7 @@ class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
                                               errors='surrogatepass')
        except UnicodeDecodeError:
            displaypath = urllib.parse.unquote(path)
-        displaypath = html.escape(displaypath)
+        displaypath = html.escape(displaypath, quote=False)
        enc = sys.getfilesystemencoding()
        title = 'Directory listing for %s' % displaypath
        r.append('<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" '
@@ -734,7 +734,7 @@ class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
            r.append('<li><a href="%s">%s</a></li>'
                    % (urllib.parse.quote(linkname,
                                          errors='surrogatepass'),
-                       html.escape(displayname)))
+                       html.escape(displayname, quote=False)))
        r.append('</ul>\n<hr>\n</body>\n</html>\n')
        encoded = '\n'.join(r).encode(enc, 'surrogateescape')
        f = io.BytesIO()

--- a/Lib/test/test_httpservers.py
+++ b/Lib/test/test_httpservers.py
@@ -344,7 +344,7 @@ class SimpleHTTPServerTestCase(BaseTestCase):
        quotedname = urllib.parse.quote(filename, errors='surrogatepass')
        self.assertIn(('href="%s"' % quotedname)
                      .encode(enc, 'surrogateescape'), body)
-        self.assertIn(('>%s<' % html.escape(filename))
+        self.assertIn(('>%s<' % html.escape(filename, quote=False))
                      .encode(enc, 'surrogateescape'), body)
        response = self.request(self.base_url + '/' + quotedname)
        self.check_status_and_reason(response, HTTPStatus.OK,
@@ -422,6 +422,27 @@ class SimpleHTTPServerTestCase(BaseTestCase):
        self.assertEqual(response.getheader("Location"),
                         self.tempdir_name + "/?hi=1")

+    def test_html_escape_filename(self):
+        filename = '<test&>.txt'
+        fullpath = os.path.join(self.tempdir, filename)
+
+        try:
+            open(fullpath, 'w').close()
+        except OSError:
+            raise unittest.SkipTest('Can not create file %s on current file '
+                                    'system' % filename)
+
+        try:
+            response = self.request(self.base_url + '/')
+            body = self.check_status_and_reason(response, HTTPStatus.OK)
+            enc = response.headers.get_content_charset()
+        finally:
+            os.unlink(fullpath)  # avoid affecting test_undecodable_filename
+
+        self.assertIsNotNone(enc)
+        html_text = '>%s<' % html.escape(filename, quote=False)
+        self.assertIn(html_text.encode(enc), body)
+

 cgi_file1 = """\
 #!%s
@@ -883,6 +904,13 @@ class BaseHTTPRequestHandlerTestCase(unittest.TestCase):
        self.assertFalse(self.handler.get_called)
        self.assertEqual(self.handler.requestline, 'GET / HTTP/1.1')

+    def test_html_escape_on_error(self):
+        result = self.send_typical_request(
+            b'<script>alert("hello")</script> / HTTP/1.1')
+        result = b''.join(result)
+        text = '<script>alert("hello")</script>'
+        self.assertIn(html.escape(text, quote=False).encode('ascii'), result)
+
    def test_close_connection(self):
        # handle_one_request() should be repeatedly called until
        # it sets close_connection

--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -240,6 +240,9 @@ Core and Builtins
 Library
 -------

+- Issue #26585: Eliminate http.server._quote_html() and use
+  html.escape(quote=False).  Patch by Xiang Zhang.
+
 - Issue #26685: Raise OSError if closing a socket fails.

 - Issue #16329: Add .webm to mimetypes.types_map.  Patch by Giampaolo Rodola'.