From 65c7460ac4d92b0863fa50799a49706af5f0aece Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Mon, 7 Jan 2019 23:59:23 +0000 Subject: [PATCH] [3.5] plain text http responses to errors (#3483) (cherry picked from commit b42a23b4) Co-authored-by: Samuel Colvin --- CHANGES/3483.feature | 1 + aiohttp/test_utils.py | 2 +- aiohttp/web_protocol.py | 35 ++++++++++++++-------- tests/test_web_protocol.py | 2 +- tests/test_web_server.py | 60 ++++++++++++++++++++++++++++++++++++-- 5 files changed, 83 insertions(+), 17 deletions(-) create mode 100644 CHANGES/3483.feature diff --git a/CHANGES/3483.feature b/CHANGES/3483.feature new file mode 100644 index 00000000000..082f66b2803 --- /dev/null +++ b/CHANGES/3483.feature @@ -0,0 +1 @@ +Internal Server Errors in plain text if the browser does not support HTML. diff --git a/aiohttp/test_utils.py b/aiohttp/test_utils.py index 177441a42d7..d1db1a76c8d 100644 --- a/aiohttp/test_utils.py +++ b/aiohttp/test_utils.py @@ -219,7 +219,7 @@ async def _make_runner(self, debug: bool=True, **kwargs: Any) -> ServerRunner: srv = Server( - self._handler, loop=self._loop, debug=True, **kwargs) + self._handler, loop=self._loop, debug=debug, **kwargs) return ServerRunner(srv, debug=debug, **kwargs) diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index fc72a982b06..c736a78ef8c 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -5,6 +5,7 @@ from collections import deque from contextlib import suppress from html import escape as html_escape +from http import HTTPStatus from logging import Logger from typing import ( TYPE_CHECKING, @@ -523,24 +524,34 @@ def handle_error(self, information. It always closes current connection.""" self.log_exception("Error handling request", exc_info=exc) - if status == 500: - msg = "

500 Internal Server Error

" + ct = 'text/plain' + if status == HTTPStatus.INTERNAL_SERVER_ERROR: + title = '{0.value} {0.phrase}'.format( + HTTPStatus.INTERNAL_SERVER_ERROR + ) + msg = HTTPStatus.INTERNAL_SERVER_ERROR.description + tb = None if self.debug: with suppress(Exception): tb = traceback.format_exc() + + if 'text/html' in request.headers.get('Accept', ''): + if tb: tb = html_escape(tb) - msg += '

Traceback:

\n
'
-                    msg += tb
-                    msg += '
' + msg = '

Traceback:

\n
{}
'.format(tb) + message = ( + "" + "{title}" + "\n

{title}

" + "\n{msg}\n\n" + ).format(title=title, msg=msg) + ct = 'text/html' else: - msg += "Server got itself in trouble" - msg = ("500 Internal Server Error" - "" + msg + "") - resp = Response(status=status, text=msg, content_type='text/html') - else: - resp = Response(status=status, text=message, - content_type='text/html') + if tb: + msg = tb + message = title + '\n\n' + msg + resp = Response(status=status, text=message, content_type=ct) resp.force_close() # some data already got sent, connection is broken diff --git a/tests/test_web_protocol.py b/tests/test_web_protocol.py index 829d1ec4fae..49d0dd7e982 100644 --- a/tests/test_web_protocol.py +++ b/tests/test_web_protocol.py @@ -299,7 +299,7 @@ async def test_handle_error__utf( await asyncio.sleep(0) assert b'HTTP/1.0 500 Internal Server Error' in buf - assert b'Content-Type: text/html; charset=utf-8' in buf + assert b'Content-Type: text/plain; charset=utf-8' in buf pattern = escape("RuntimeError: что-то пошло не так") assert pattern.encode('utf-8') in buf assert not srv._keepalive diff --git a/tests/test_web_server.py b/tests/test_web_server.py index 7fe714b578e..57ffa110557 100644 --- a/tests/test_web_server.py +++ b/tests/test_web_server.py @@ -26,13 +26,15 @@ async def handler(request): raise exc logger = mock.Mock() - server = await aiohttp_raw_server(handler, logger=logger) + server = await aiohttp_raw_server(handler, logger=logger, debug=False) cli = await aiohttp_client(server) resp = await cli.get('/path/to') assert resp.status == 500 + assert resp.headers['Content-Type'].startswith('text/plain') txt = await resp.text() - assert "

500 Internal Server Error

" in txt + assert txt.startswith('500 Internal Server Error') + assert 'Traceback' not in txt logger.exception.assert_called_with( "Error handling request", @@ -102,10 +104,62 @@ async def handler(request): cli = await aiohttp_client(server) resp = await cli.get('/path/to') assert resp.status == 500 + assert resp.headers['Content-Type'].startswith('text/plain') txt = await resp.text() - assert "

Traceback:

" in txt + assert 'Traceback (most recent call last):\n' in txt logger.exception.assert_called_with( "Error handling request", exc_info=exc) + + +async def test_raw_server_html_exception(aiohttp_raw_server, aiohttp_client): + exc = RuntimeError("custom runtime error") + + async def handler(request): + raise exc + + logger = mock.Mock() + server = await aiohttp_raw_server(handler, logger=logger, debug=False) + cli = await aiohttp_client(server) + resp = await cli.get('/path/to', headers={'Accept': 'text/html'}) + assert resp.status == 500 + assert resp.headers['Content-Type'].startswith('text/html') + + txt = await resp.text() + assert txt == ( + '500 Internal Server Error\n' + '

500 Internal Server Error

\n' + 'Server got itself in trouble\n' + '\n' + ) + + logger.exception.assert_called_with( + "Error handling request", exc_info=exc) + + +async def test_raw_server_html_exception_debug(aiohttp_raw_server, + aiohttp_client): + exc = RuntimeError("custom runtime error") + + async def handler(request): + raise exc + + logger = mock.Mock() + server = await aiohttp_raw_server(handler, logger=logger, debug=True) + cli = await aiohttp_client(server) + resp = await cli.get('/path/to', headers={'Accept': 'text/html'}) + assert resp.status == 500 + assert resp.headers['Content-Type'].startswith('text/html') + + txt = await resp.text() + assert txt.startswith( + '500 Internal Server Error\n' + '

500 Internal Server Error

\n' + '

Traceback:

\n' + '
Traceback (most recent call last):\n'
+    )
+
+    logger.exception.assert_called_with(
+        "Error handling request", exc_info=exc)