From a553e64bbd8c09e2ac150ca97d59ba9e944724c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Sat, 29 Feb 2020 16:50:59 +0200 Subject: [PATCH] Much cleanup, 12 failing... --- sanic/app.py | 71 +++++---------- sanic/errorpages.py | 8 +- sanic/http.py | 217 ++++++++++++++++++++++---------------------- sanic/server.py | 24 ++--- 4 files changed, 146 insertions(+), 174 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index beabcdfc..c804c693 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -930,6 +930,26 @@ class Sanic: """ pass + async def handle_exception(self, request, exception): + try: + response = self.error_handler.response(request, exception) + if isawaitable(response): + response = await response + except Exception as e: + if isinstance(e, SanicException): + response = self.error_handler.default(request, e) + elif self.debug: + response = HTTPResponse( + f"Error while handling error: {e}\nStack: {format_exc()}", + status=500, + ) + else: + response = HTTPResponse( + "An error occurred while handling an error", status=500 + ) + return response + + async def handle_request(self, request): """Take a request from the HTTP Server and return a response object to be sent back The HTTP Server only expects a response object, so @@ -1003,27 +1023,7 @@ class Sanic: # -------------------------------------------- # # Response Generation Failed # -------------------------------------------- # - - try: - response = self.error_handler.response(request, e) - if isawaitable(response): - response = await response - except Exception as e: - if isinstance(e, SanicException): - response = self.error_handler.default( - request=request, exception=e - ) - elif self.debug: - response = HTTPResponse( - "Error while handling error: {}\nStack: {}".format( - e, format_exc() - ), - status=500, - ) - else: - response = HTTPResponse( - "An error occurred while handling an error", status=500 - ) + response = await self.handle_exception(request, e) finally: # -------------------------------------------- # # Response Middleware @@ -1048,39 +1048,14 @@ class Sanic: try: # pass the response to the correct callback - if response is None: - pass - elif isinstance(response, StreamingHTTPResponse): + if isinstance(response, StreamingHTTPResponse): await response.stream(request) elif isinstance(response, HTTPResponse): await request.respond(response).send(end_stream=True) else: raise ServerError(f"Invalid response type {response} (need HTTPResponse)") except Exception as e: - # -------------------------------------------- # - # Response Generation Failed - # -------------------------------------------- # - - try: - response = self.error_handler.response(request, e) - if isawaitable(response): - response = await response - except Exception as e: - if isinstance(e, SanicException): - response = self.error_handler.default( - request=request, exception=e - ) - elif self.debug: - response = HTTPResponse( - "Error while handling error: {}\nStack: {}".format( - e, format_exc() - ), - status=500, - ) - else: - response = HTTPResponse( - "An error occurred while handling an error", status=500 - ) + response = await self.handle_exception(request, e) # pass the response to the correct callback if response is None: diff --git a/sanic/errorpages.py b/sanic/errorpages.py index 2d17cbde..f0c1a961 100644 --- a/sanic/errorpages.py +++ b/sanic/errorpages.py @@ -71,10 +71,14 @@ def _render_traceback_html(request, exception): exc_value = exc_value.__cause__ traceback_html = TRACEBACK_BORDER.join(reversed(exceptions)) - appname = escape(request.app.name) + if request is not None: + appname = escape(request.app.name) + path = escape(request.path) + else: + appname = "" + path = "unknown" name = escape(exception.__class__.__name__) value = escape(exception) - path = escape(request.path) return ( f"

Traceback of {appname} (most recent call last):

" f"{traceback_html}" diff --git a/sanic/http.py b/sanic/http.py index 04e3320e..6befc3fa 100644 --- a/sanic/http.py +++ b/sanic/http.py @@ -1,5 +1,7 @@ +from asyncio import CancelledError from enum import Enum +from sanic.compat import Header from sanic.exceptions import ( HeaderExpectationFailed, InvalidUsage, @@ -14,7 +16,6 @@ from sanic.helpers import has_message_body, remove_entity_headers from sanic.log import access_logger, logger from sanic.request import Request from sanic.response import HTTPResponse -from sanic.compat import Header class Stage(Enum): @@ -34,119 +35,121 @@ class Http: self.recv_buffer = protocol.recv_buffer self.protocol = protocol self.expecting_continue = False - # Note: connections are initially in request mode and do not obey - # keep-alive timeout like with some other servers. - self.stage = Stage.REQUEST + self.stage = Stage.IDLE self.keep_alive = True self.head_only = None + self.request = None + self.exception = None + + async def http1_receive_request(self): + buf = self.recv_buffer + pos = 0 + while len(buf) < self.protocol.request_max_size: + if buf: + pos = buf.find(b"\r\n\r\n", pos) + if pos >= 0: + break + pos = max(0, len(buf) - 3) + await self._receive_more() + if self.stage is Stage.IDLE: + self.stage = Stage.REQUEST + else: + raise PayloadTooLarge("Payload Too Large") + + self.protocol._total_request_size = pos + 4 + + try: + reqline, *headers = buf[:pos].decode().split("\r\n") + method, self.url, protocol = reqline.split(" ") + if protocol not in ("HTTP/1.0", "HTTP/1.1"): + raise Exception + self.head_only = method.upper() == "HEAD" + headers = Header( + (name.lower(), value.lstrip()) + for name, value in (h.split(":", 1) for h in headers) + ) + except: + raise InvalidUsage("Bad Request") + request = self.protocol.request_class( + url_bytes=self.url.encode(), + headers=headers, + version=protocol[-3:], + method=method, + transport=self.protocol.transport, + app=self.protocol.app, + ) + + # Prepare a request object from the header received + request.stream = self + self.protocol.state["requests_count"] += 1 + self.keep_alive = ( + protocol == "HTTP/1.1" + or headers.get("connection", "").lower() == "keep-alive" + ) + # Prepare for request body + body = headers.get("transfer-encoding") == "chunked" or int( + headers.get("content-length", 0) + ) + self.request_chunked = False + self.request_bytes_left = 0 + if body: + expect = headers.get("expect") + if expect: + if expect.lower() == "100-continue": + self.expecting_continue = True + else: + raise HeaderExpectationFailed( + f"Unknown Expect: {expect}") + request.stream = self + if body is True: + self.request_chunked = True + pos -= 2 # One CRLF stays in buffer + else: + self.request_bytes_left = body + # Remove header and its trailing CRLF + del buf[: pos + 4] + self.stage = Stage.HANDLER + self.request = request async def http1(self): """HTTP 1.1 connection handler""" - buf = self.recv_buffer - self.url = None - while self.keep_alive: - # Read request header - pos = 0 - while len(buf) < self.protocol.request_max_size: - if buf: - pos = buf.find(b"\r\n\r\n", pos) - if pos >= 0: - break - pos = max(0, len(buf) - 3) - await self._receive_more() + while self.stage is Stage.IDLE and self.keep_alive: + try: + # Receive request header and call handler + await self.http1_receive_request() + await self.protocol.request_handler(self.request) + if self.stage is Stage.HANDLER: + raise ServerError("Handler produced no response") + # Finish sending a response (if no error) + if self.stage is Stage.RESPONSE: + await self.send(end_stream=True) + # Consume any remaining request body + if self.request_bytes_left or self.request_chunked: + logger.error(f"{self.request} body not consumed.") + async for _ in self: + pass + except Exception as e: + self.exception = e + except BaseException: + # Exit after trying to finish a response + self.keep_alive = False + if self.exception is None: + self.exception = ServiceUnavailable(f"Cancelled") + if self.exception: + e, self.exception = self.exception, None + # Exception while idle? Probably best to close connection if self.stage is Stage.IDLE: - self.stage = Stage.REQUEST - else: - self.stage = Stage.HANDLER - raise PayloadTooLarge("Payload Too Large") + return + # Request failure? Try to respond but then disconnect + if self.stage is Stage.REQUEST: + self.keep_alive = False + self.stage = Stage.HANDLER + # Return an error page if possible + if self.stage is Stage.HANDLER: + app = self.protocol.app + response = await app.handle_exception(self.request, e) + await self.respond(response).send(end_stream=True) - self.protocol._total_request_size = pos + 4 - - try: - reqline, *headers = buf[:pos].decode().split("\r\n") - method, self.url, protocol = reqline.split(" ") - if protocol not in ("HTTP/1.0", "HTTP/1.1"): - raise Exception - self.head_only = method.upper() == "HEAD" - headers = Header( - (name.lower(), value.lstrip()) - for name, value in (h.split(":", 1) for h in headers) - ) - except: - self.stage = Stage.HANDLER - raise InvalidUsage("Bad Request") - - # Prepare a request object from the header received - request = self.protocol.request_class( - url_bytes=self.url.encode(), - headers=headers, - version=protocol[-3:], - method=method, - transport=self.protocol.transport, - app=self.protocol.app, - ) - request.stream = self - self.protocol.state["requests_count"] += 1 - self.keep_alive = ( - protocol == "HTTP/1.1" - or headers.get("connection", "").lower() == "keep-alive" - ) - # Prepare for request body - body = headers.get("transfer-encoding") == "chunked" or int( - headers.get("content-length", 0) - ) - self.request_chunked = False - self.request_bytes_left = 0 - self.stage = Stage.HANDLER - if body: - expect = headers.get("expect") - if expect: - if expect.lower() == "100-continue": - self.expecting_continue = True - else: - raise HeaderExpectationFailed(f"Unknown Expect: {expect}") - request.stream = self - if body is True: - self.request_chunked = True - pos -= 2 # One CRLF stays in buffer - else: - self.request_bytes_left = body - # Remove header and its trailing CRLF - del buf[: pos + 4] - - # Run handler - try: - await self.protocol.request_handler(request) - except Exception: - logger.exception("Uncaught from app/handler") - await self.write_error(ServerError("Internal Server Error")) - if self.stage is Stage.IDLE: - continue - - if self.stage is Stage.HANDLER: - await self.respond(HTTPResponse(status=204)).send(end_stream=True) - - # Finish sending a response (if no error) - elif self.stage is Stage.RESPONSE: - await self.send(end_stream=True) - - # Consume any remaining request body - if self.request_bytes_left or self.request_chunked: - logger.error( - f"Handler of {method} {url} did not consume request body." - ) - while await self.read(): - pass - - self.stage = Stage.IDLE - - async def write_error(self, e): - if self.stage is Stage.HANDLER: - try: - response = HTTPResponse(f"{e}", e.status_code, content_type="text/plain") - await self.respond(response).send(end_stream=True) - except: - logger.exception("Error sending error") # Request methods diff --git a/sanic/server.py b/sanic/server.py index 789ae67d..d11e14e0 100644 --- a/sanic/server.py +++ b/sanic/server.py @@ -4,6 +4,7 @@ import os import sys import traceback +from asyncio import CancelledError from collections import deque from functools import partial from inspect import isawaitable @@ -11,7 +12,8 @@ from multiprocessing import Process from signal import SIG_IGN, SIGINT, SIGTERM, Signals from signal import signal as signal_func from socket import SO_REUSEADDR, SOL_SOCKET, socket -from time import time, monotonic as current_time +from time import monotonic as current_time +from time import time from sanic.compat import Header from sanic.exceptions import ( @@ -145,20 +147,8 @@ class HttpProtocol(asyncio.Protocol): self._http = Http(self) self._time = current_time() self.check_timeouts() - try: - await self._http.http1() - except asyncio.CancelledError: - await self._http.write_error( - self._exception - or ServiceUnavailable("Request handler cancelled") - ) - except SanicException as e: - await self._http.write_error(e) - except BaseException as e: - logger.exception( - f"Uncaught exception while handling URL {self._http.url}" - ) - except asyncio.CancelledError: + await self._http.http1() + except CancelledError: pass except: logger.exception("protocol.connection_task uncaught") @@ -185,12 +175,12 @@ class HttpProtocol(asyncio.Protocol): if stage is Stage.IDLE and duration > self.keep_alive_timeout: logger.debug("KeepAlive Timeout. Closing connection.") elif stage is Stage.REQUEST and duration > self.request_timeout: - self._exception = RequestTimeout("Request Timeout") + self._http.exception = RequestTimeout("Request Timeout") elif ( stage in (Stage.REQUEST, Stage.FAILED) and duration > self.response_timeout ): - self._exception = ServiceUnavailable("Response Timeout") + self._http.exception = ServiceUnavailable("Response Timeout") else: self.loop.call_later(1.0, self.check_timeouts) return