From 7545b91cca32a551009959d704d659c74365b051 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 22 Dec 2021 09:02:07 +0530 Subject: [PATCH] Ensure message for every `HttpProtocolException` raised --- proxy/http/exception/base.py | 5 ++++- proxy/http/exception/http_request_rejected.py | 20 +++++++++++++------ proxy/http/exception/proxy_auth_failed.py | 5 ++++- proxy/http/exception/proxy_conn_failed.py | 5 +++-- proxy/http/handler.py | 2 +- proxy/http/proxy/server.py | 3 +-- proxy/http/server/web.py | 6 ++---- proxy/plugin/proxy_pool.py | 6 ++---- proxy/plugin/reverse_proxy.py | 3 +-- 9 files changed, 32 insertions(+), 23 deletions(-) diff --git a/proxy/http/exception/base.py b/proxy/http/exception/base.py index d39a640392..17a69bb689 100644 --- a/proxy/http/exception/base.py +++ b/proxy/http/exception/base.py @@ -12,7 +12,7 @@ http """ -from typing import Optional, TYPE_CHECKING +from typing import Any, Optional, TYPE_CHECKING if TYPE_CHECKING: from ..parser import HttpParser @@ -26,5 +26,8 @@ class HttpProtocolException(Exception): ``response()`` method to optionally return custom response to client. """ + def __init__(self, message: Optional[str] = None, **kwargs: Any) -> None: + super().__init__(message or 'Reason unknown') + def response(self, request: 'HttpParser') -> Optional[memoryview]: return None # pragma: no cover diff --git a/proxy/http/exception/http_request_rejected.py b/proxy/http/exception/http_request_rejected.py index 6d095481fe..baec9e997a 100644 --- a/proxy/http/exception/http_request_rejected.py +++ b/proxy/http/exception/http_request_rejected.py @@ -8,7 +8,7 @@ :copyright: (c) 2013-present by Abhinav Singh and contributors. :license: BSD, see LICENSE for more details. """ -from typing import Optional, Dict, TYPE_CHECKING +from typing import Any, Optional, Dict, TYPE_CHECKING from .base import HttpProtocolException @@ -25,16 +25,24 @@ class HttpRequestRejected(HttpProtocolException): HTTP status code can be returned.""" def __init__( - self, - status_code: Optional[int] = None, - reason: Optional[bytes] = None, - headers: Optional[Dict[bytes, bytes]] = None, - body: Optional[bytes] = None, + self, + status_code: Optional[int] = None, + reason: Optional[bytes] = None, + headers: Optional[Dict[bytes, bytes]] = None, + body: Optional[bytes] = None, + **kwargs: Any, ): self.status_code: Optional[int] = status_code self.reason: Optional[bytes] = reason self.headers: Optional[Dict[bytes, bytes]] = headers self.body: Optional[bytes] = body + klass_name = self.__class__.__name__ + super().__init__( + message='%s %r' % (klass_name, reason) + if reason + else klass_name, + **kwargs, + ) def response(self, _request: 'HttpParser') -> Optional[memoryview]: if self.status_code: diff --git a/proxy/http/exception/proxy_auth_failed.py b/proxy/http/exception/proxy_auth_failed.py index ecb9a2994d..60782e0c23 100644 --- a/proxy/http/exception/proxy_auth_failed.py +++ b/proxy/http/exception/proxy_auth_failed.py @@ -13,7 +13,7 @@ auth http """ -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from .base import HttpProtocolException @@ -43,5 +43,8 @@ class ProxyAuthenticationFailed(HttpProtocolException): ), ) + def __init__(self, **kwargs: Any) -> None: + super().__init__(self.__class__.__name__, **kwargs) + def response(self, _request: 'HttpParser') -> memoryview: return self.RESPONSE_PKT diff --git a/proxy/http/exception/proxy_conn_failed.py b/proxy/http/exception/proxy_conn_failed.py index c091bf50be..d0eae48192 100644 --- a/proxy/http/exception/proxy_conn_failed.py +++ b/proxy/http/exception/proxy_conn_failed.py @@ -12,7 +12,7 @@ conn """ -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from .base import HttpProtocolException @@ -40,10 +40,11 @@ class ProxyConnectionFailed(HttpProtocolException): ), ) - def __init__(self, host: str, port: int, reason: str): + def __init__(self, host: str, port: int, reason: str, **kwargs: Any): self.host: str = host self.port: int = port self.reason: str = reason + super().__init__('%s %s' % (self.__class__.__name__, reason), **kwargs) def response(self, _request: 'HttpParser') -> memoryview: return self.RESPONSE_PKT diff --git a/proxy/http/handler.py b/proxy/http/handler.py index c033e8b022..38429df7de 100644 --- a/proxy/http/handler.py +++ b/proxy/http/handler.py @@ -237,7 +237,7 @@ def handle_data(self, data: memoryview) -> Optional[bool]: break data = optional_data except HttpProtocolException as e: - logger.exception('HttpProtocolException raised', exc_info=e) + logger.info('HttpProtocolException: %s' % e) response: Optional[memoryview] = e.response(self.request) if response: self.work.queue(response) diff --git a/proxy/http/proxy/server.py b/proxy/http/proxy/server.py index 2fda84f8d5..462cf0fc83 100644 --- a/proxy/http/proxy/server.py +++ b/proxy/http/proxy/server.py @@ -657,8 +657,7 @@ def connect_upstream(self) -> None: text_(host), port, repr(e), ) from e else: - logger.exception('Both host and port must exist') - raise HttpProtocolException() + raise HttpProtocolException('Both host and port must exist') # # Interceptor related methods diff --git a/proxy/http/server/web.py b/proxy/http/server/web.py index c645267f57..520f170d53 100644 --- a/proxy/http/server/web.py +++ b/proxy/http/server/web.py @@ -263,10 +263,9 @@ def on_client_data(self, raw: memoryview) -> Optional[memoryview]: # TODO: Tear down if invalid protocol exception remaining = frame.parse(remaining) if frame.opcode == websocketOpcodes.CONNECTION_CLOSE: - logger.warning( + raise HttpProtocolException( 'Client sent connection close packet', ) - raise HttpProtocolException() else: assert self.route self.route.on_websocket_message(frame) @@ -287,10 +286,9 @@ def on_client_data(self, raw: memoryview) -> Optional[memoryview]: if self.pipeline_request.is_complete: self.route.handle_request(self.pipeline_request) if not self.pipeline_request.is_http_1_1_keep_alive: - logger.error( + raise HttpProtocolException( 'Pipelined request is not keep-alive, will tear down request...', ) - raise HttpProtocolException() self.pipeline_request = None return raw diff --git a/proxy/plugin/proxy_pool.py b/proxy/plugin/proxy_pool.py index bfe00aaafa..cfc8017820 100644 --- a/proxy/plugin/proxy_pool.py +++ b/proxy/plugin/proxy_pool.py @@ -110,12 +110,11 @@ def before_upstream_connection( try: self.upstream.connect() except TimeoutError: - logger.info( + raise HttpProtocolException( 'Timed out connecting to upstream proxy {0}:{1}'.format( *endpoint_tuple, ), ) - raise HttpProtocolException() except ConnectionRefusedError: # TODO(abhinavsingh): Try another choice, when all (or max configured) choices have # exhausted, retry for configured number of times before giving up. @@ -124,12 +123,11 @@ def before_upstream_connection( # A periodic health check must put them back in the pool. This can be achieved # using a datastructure without having to spawn separate thread/process for health # check. - logger.info( + raise HttpProtocolException( 'Connection refused by upstream proxy {0}:{1}'.format( *endpoint_tuple, ), ) - raise HttpProtocolException() logger.debug( 'Established connection to upstream proxy {0}:{1}'.format( *endpoint_tuple, diff --git a/proxy/plugin/reverse_proxy.py b/proxy/plugin/reverse_proxy.py index 0715d184f8..26ebbb6aae 100644 --- a/proxy/plugin/reverse_proxy.py +++ b/proxy/plugin/reverse_proxy.py @@ -101,12 +101,11 @@ def handle_request(self, request: HttpParser) -> None: ) self.upstream.queue(memoryview(request.build())) except ConnectionRefusedError: - logger.info( + raise HttpProtocolException( 'Connection refused by upstream server {0}:{1}'.format( text_(self.choice.hostname), port, ), ) - raise HttpProtocolException() def on_client_connection_close(self) -> None: if self.upstream and not self.upstream.closed: