[3.5] plain text http responses to errors (#3483) (#3499)

(cherry picked from commit b42a23b) Co-authored-by: Samuel Colvin <samcolvin@gmail.com>
aio-libs · Jan 8, 2019 · 396ec1f · 396ec1f
1 parent 42ff0b2
commit 396ec1f
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 17 deletions.
diff --git a/CHANGES/3483.feature 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
@@ -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
@@ -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 = "<h1>500 Internal Server Error</h1>"
+        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 += '<br><h2>Traceback:</h2>\n<pre>'
-                    msg += tb
-                    msg += '</pre>'
+                    msg = '<h2>Traceback:</h2>\n<pre>{}</pre>'.format(tb)
+                message = (
+                    "<html><head>"
+                    "<title>{title}</title>"
+                    "</head><body>\n<h1>{title}</h1>"
+                    "\n{msg}\n</body></html>\n"
+                ).format(title=title, msg=msg)
+                ct = 'text/html'
             else:
-                msg += "Server got itself in trouble"
-                msg = ("<html><head><title>500 Internal Server Error</title>"
-                       "</head><body>" + msg + "</body></html>")
-            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
@@ -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
@@ -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 "<h1>500 Internal Server Error</h1>" 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 "<h2>Traceback:</h2>" 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 == (
+        '<html><head><title>500 Internal Server Error</title></head><body>\n'
+        '<h1>500 Internal Server Error</h1>\n'
+        'Server got itself in trouble\n'
+        '</body></html>\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(
+        '<html><head><title>500 Internal Server Error</title></head><body>\n'
+        '<h1>500 Internal Server Error</h1>\n'
+        '<h2>Traceback:</h2>\n'
+        '<pre>Traceback (most recent call last):\n'
+    )
+
+    logger.exception.assert_called_with(
+        "Error handling request", exc_info=exc)