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

Patch by Xiang Zhang.
This commit is contained in:
Martin Panter 2016-04-11 00:40:08 +00:00
parent 50ab1a3694
commit da3bb38452
3 changed files with 40 additions and 9 deletions

View File

@ -127,9 +127,6 @@ DEFAULT_ERROR_MESSAGE = """\
DEFAULT_ERROR_CONTENT_TYPE = "text/html;charset=utf-8" 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): class HTTPServer(socketserver.TCPServer):
allow_reuse_address = 1 # Seems to make sense in testing environment allow_reuse_address = 1 # Seems to make sense in testing environment
@ -449,9 +446,12 @@ class BaseHTTPRequestHandler(socketserver.StreamRequestHandler):
if explain is None: if explain is None:
explain = longmsg explain = longmsg
self.log_error("code %d, message %s", code, message) self.log_error("code %d, message %s", code, message)
# using _quote_html to prevent Cross Site Scripting attacks (see bug #1100201) # HTML encode to prevent Cross Site Scripting attacks (see bug #1100201)
content = (self.error_message_format % content = (self.error_message_format % {
{'code': code, 'message': _quote_html(message), 'explain': _quote_html(explain)}) 'code': code,
'message': html.escape(message, quote=False),
'explain': html.escape(explain, quote=False)
})
body = content.encode('UTF-8', 'replace') body = content.encode('UTF-8', 'replace')
self.send_response(code, message) self.send_response(code, message)
self.send_header("Content-Type", self.error_content_type) self.send_header("Content-Type", self.error_content_type)
@ -710,7 +710,7 @@ class SimpleHTTPRequestHandler(BaseHTTPRequestHandler):
errors='surrogatepass') errors='surrogatepass')
except UnicodeDecodeError: except UnicodeDecodeError:
displaypath = urllib.parse.unquote(path) displaypath = urllib.parse.unquote(path)
displaypath = html.escape(displaypath) displaypath = html.escape(displaypath, quote=False)
enc = sys.getfilesystemencoding() enc = sys.getfilesystemencoding()
title = 'Directory listing for %s' % displaypath title = 'Directory listing for %s' % displaypath
r.append('<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" ' 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>' r.append('<li><a href="%s">%s</a></li>'
% (urllib.parse.quote(linkname, % (urllib.parse.quote(linkname,
errors='surrogatepass'), errors='surrogatepass'),
html.escape(displayname))) html.escape(displayname, quote=False)))
r.append('</ul>\n<hr>\n</body>\n</html>\n') r.append('</ul>\n<hr>\n</body>\n</html>\n')
encoded = '\n'.join(r).encode(enc, 'surrogateescape') encoded = '\n'.join(r).encode(enc, 'surrogateescape')
f = io.BytesIO() f = io.BytesIO()

View File

@ -344,7 +344,7 @@ class SimpleHTTPServerTestCase(BaseTestCase):
quotedname = urllib.parse.quote(filename, errors='surrogatepass') quotedname = urllib.parse.quote(filename, errors='surrogatepass')
self.assertIn(('href="%s"' % quotedname) self.assertIn(('href="%s"' % quotedname)
.encode(enc, 'surrogateescape'), body) .encode(enc, 'surrogateescape'), body)
self.assertIn(('>%s<' % html.escape(filename)) self.assertIn(('>%s<' % html.escape(filename, quote=False))
.encode(enc, 'surrogateescape'), body) .encode(enc, 'surrogateescape'), body)
response = self.request(self.base_url + '/' + quotedname) response = self.request(self.base_url + '/' + quotedname)
self.check_status_and_reason(response, HTTPStatus.OK, self.check_status_and_reason(response, HTTPStatus.OK,
@ -422,6 +422,27 @@ class SimpleHTTPServerTestCase(BaseTestCase):
self.assertEqual(response.getheader("Location"), self.assertEqual(response.getheader("Location"),
self.tempdir_name + "/?hi=1") 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 = """\ cgi_file1 = """\
#!%s #!%s
@ -883,6 +904,13 @@ class BaseHTTPRequestHandlerTestCase(unittest.TestCase):
self.assertFalse(self.handler.get_called) self.assertFalse(self.handler.get_called)
self.assertEqual(self.handler.requestline, 'GET / HTTP/1.1') 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): def test_close_connection(self):
# handle_one_request() should be repeatedly called until # handle_one_request() should be repeatedly called until
# it sets close_connection # it sets close_connection

View File

@ -240,6 +240,9 @@ Core and Builtins
Library 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 #26685: Raise OSError if closing a socket fails.
- Issue #16329: Add .webm to mimetypes.types_map. Patch by Giampaolo Rodola'. - Issue #16329: Add .webm to mimetypes.types_map. Patch by Giampaolo Rodola'.