diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index 1b7928e0f3..c344b67610 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -668,12 +668,32 @@ jobs: make ca-certificates python3 -m proxy --version + check: # This job does nothing and is only used for the branch protection + if: always() + + needs: + - analyze + - test + - lint + - dashboard + - brew + - developer + + runs-on: Ubuntu-latest + + steps: + - name: Decide whether the needed jobs succeeded or failed + uses: re-actors/alls-green@release/v1 + with: + jobs: ${{ toJSON(needs) }} + docker: runs-on: Ubuntu-latest permissions: packages: write + if: success() needs: - - build + - check - pre-setup # transitive, for accessing settings name: 🐳 containerize strategy: @@ -786,26 +806,6 @@ jobs: }}' -t $CONTAINER_TAG . - check: # This job does nothing and is only used for the branch protection - if: always() - - needs: - - analyze - - test - - lint - - docker - - dashboard - - brew - - developer - - runs-on: Ubuntu-latest - - steps: - - name: Decide whether the needed jobs succeeded or failed - uses: re-actors/alls-green@release/v1 - with: - jobs: ${{ toJSON(needs) }} - publish-pypi: name: Publish 🐍📦 ${{ needs.pre-setup.outputs.git-tag }} to PyPI needs: diff --git a/proxy/core/acceptor/threadless.py b/proxy/core/acceptor/threadless.py index f36f9c0a98..cfd2f12086 100644 --- a/proxy/core/acceptor/threadless.py +++ b/proxy/core/acceptor/threadless.py @@ -180,13 +180,15 @@ async def _update_work_events(self, work_id: int) -> None: # else: # logger.info( # 'fd#{0} by work#{1} not modified'.format(fileno, work_id)) + # Can throw ValueError: Invalid file descriptor: -1 + # + # A guard within Work classes may not help here due to + # asynchronous nature. Hence, threadless will handle + # ValueError exceptions raised by selector.register + # for invalid fd. + # + # TODO: Also remove offending work from pool to avoid spin loop. elif fileno != -1: - # Can throw ValueError: Invalid file descriptor: -1 - # - # A guard within Work classes may not help here due to - # asynchronous nature. Hence, threadless will handle - # ValueError exceptions raised by selector.register - # for invalid fd. self.selector.register( fileno, events=mask, data=work_id, diff --git a/proxy/http/exception/base.py b/proxy/http/exception/base.py index 37817c9265..d39a640392 100644 --- a/proxy/http/exception/base.py +++ b/proxy/http/exception/base.py @@ -12,9 +12,10 @@ http """ -from typing import Optional +from typing import Optional, TYPE_CHECKING -from ..parser import HttpParser +if TYPE_CHECKING: + from ..parser import HttpParser class HttpProtocolException(Exception): @@ -25,5 +26,5 @@ class HttpProtocolException(Exception): ``response()`` method to optionally return custom response to client. """ - def response(self, request: HttpParser) -> Optional[memoryview]: + 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 1b3b1fd742..6d095481fe 100644 --- a/proxy/http/exception/http_request_rejected.py +++ b/proxy/http/exception/http_request_rejected.py @@ -8,12 +8,15 @@ :copyright: (c) 2013-present by Abhinav Singh and contributors. :license: BSD, see LICENSE for more details. """ -from typing import Optional, Dict +from typing import Optional, Dict, TYPE_CHECKING from .base import HttpProtocolException -from ..parser import HttpParser + from ...common.utils import build_http_response +if TYPE_CHECKING: + from ..parser import HttpParser + class HttpRequestRejected(HttpProtocolException): """Generic exception that can be used to reject the client requests. @@ -33,7 +36,7 @@ def __init__( self.headers: Optional[Dict[bytes, bytes]] = headers self.body: Optional[bytes] = body - def response(self, _request: HttpParser) -> Optional[memoryview]: + def response(self, _request: 'HttpParser') -> Optional[memoryview]: if self.status_code: return memoryview( build_http_response( diff --git a/proxy/http/exception/proxy_auth_failed.py b/proxy/http/exception/proxy_auth_failed.py index 6c9649880b..ecb9a2994d 100644 --- a/proxy/http/exception/proxy_auth_failed.py +++ b/proxy/http/exception/proxy_auth_failed.py @@ -13,14 +13,18 @@ auth http """ +from typing import TYPE_CHECKING + from .base import HttpProtocolException from ..codes import httpStatusCodes -from ..parser import HttpParser from ...common.constants import PROXY_AGENT_HEADER_VALUE, PROXY_AGENT_HEADER_KEY from ...common.utils import build_http_response +if TYPE_CHECKING: + from ..parser import HttpParser + class ProxyAuthenticationFailed(HttpProtocolException): """Exception raised when HTTP Proxy auth is enabled and @@ -39,5 +43,5 @@ class ProxyAuthenticationFailed(HttpProtocolException): ), ) - def response(self, _request: HttpParser) -> memoryview: + 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 3c90557e8e..c091bf50be 100644 --- a/proxy/http/exception/proxy_conn_failed.py +++ b/proxy/http/exception/proxy_conn_failed.py @@ -12,14 +12,18 @@ conn """ +from typing import TYPE_CHECKING + from .base import HttpProtocolException from ..codes import httpStatusCodes -from ..parser import HttpParser from ...common.constants import PROXY_AGENT_HEADER_VALUE, PROXY_AGENT_HEADER_KEY from ...common.utils import build_http_response +if TYPE_CHECKING: + from ..parser import HttpParser + class ProxyConnectionFailed(HttpProtocolException): """Exception raised when ``HttpProxyPlugin`` is unable to establish connection to upstream server.""" @@ -41,5 +45,5 @@ def __init__(self, host: str, port: int, reason: str): self.port: int = port self.reason: str = reason - def response(self, _request: HttpParser) -> memoryview: + def response(self, _request: 'HttpParser') -> memoryview: return self.RESPONSE_PKT diff --git a/proxy/http/handler.py b/proxy/http/handler.py index 4a03aa5915..c033e8b022 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.debug('HttpProtocolException raised') + logger.exception('HttpProtocolException raised', exc_info=e) response: Optional[memoryview] = e.response(self.request) if response: self.work.queue(response) diff --git a/proxy/http/parser/parser.py b/proxy/http/parser/parser.py index c446b34b15..5ed707246a 100644 --- a/proxy/http/parser/parser.py +++ b/proxy/http/parser/parser.py @@ -22,6 +22,7 @@ from ..url import Url from ..methods import httpMethods +from ..exception import HttpProtocolException from .protocol import ProxyProtocol from .chunk import ChunkParser, chunkParserStates @@ -363,10 +364,7 @@ def _process_line(self, raw: bytes) -> Tuple[bool, bytes]: break # To avoid a possible attack vector, we raise exception # if parser receives an invalid request line. - # - # TODO: Better to use raise HttpProtocolException, - # but we should solve circular import problem first. - raise ValueError('Invalid request line') + raise HttpProtocolException('Invalid request line %r' % raw) parts = line.split(WHITESPACE, 2) self.version = parts[0] self.code = parts[1] diff --git a/proxy/http/parser/protocol.py b/proxy/http/parser/protocol.py index d749fdc08a..c44192f0e5 100644 --- a/proxy/http/parser/protocol.py +++ b/proxy/http/parser/protocol.py @@ -10,6 +10,8 @@ """ from typing import Optional, Tuple +from ..exception import HttpProtocolException + from ...common.constants import WHITESPACE PROXY_PROTOCOL_V2_SIGNATURE = b'\x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A' @@ -43,4 +45,6 @@ def parse(self, raw: bytes) -> None: self.version = 2 raise NotImplementedError() else: - raise ValueError('Neither a v1 or v2 proxy protocol packet') + raise HttpProtocolException( + 'Neither a v1 or v2 proxy protocol packet', + ) diff --git a/tests/http/test_http_parser.py b/tests/http/test_http_parser.py index 774bf9f3d2..5f771bd708 100644 --- a/tests/http/test_http_parser.py +++ b/tests/http/test_http_parser.py @@ -15,6 +15,7 @@ from proxy.common.utils import find_http_line, bytes_ from proxy.http import httpStatusCodes, httpMethods +from proxy.http.exception import HttpProtocolException from proxy.http.parser import HttpParser, httpParserTypes, httpParserStates @@ -24,10 +25,10 @@ def setUp(self) -> None: self.parser = HttpParser(httpParserTypes.REQUEST_PARSER) def test_issue_127(self) -> None: - with self.assertRaises(ValueError): + with self.assertRaises(HttpProtocolException): self.parser.parse(CRLF) - with self.assertRaises(ValueError): + with self.assertRaises(HttpProtocolException): raw = b'qwqrqw!@!#@!#ad adfad\r\n' while True: self.parser.parse(raw) diff --git a/tests/http/test_proxy_protocol.py b/tests/http/test_proxy_protocol.py index b6701abfb7..f8b609b1b9 100644 --- a/tests/http/test_proxy_protocol.py +++ b/tests/http/test_proxy_protocol.py @@ -11,6 +11,7 @@ import unittest from proxy.http.parser import ProxyProtocol, PROXY_PROTOCOL_V2_SIGNATURE +from proxy.http.exception import HttpProtocolException class TestProxyProtocol(unittest.TestCase): @@ -81,6 +82,6 @@ def test_v2_not_implemented(self) -> None: self.assertEqual(self.protocol.version, 2) def test_unknown_value_error(self) -> None: - with self.assertRaises(ValueError): + with self.assertRaises(HttpProtocolException): self.protocol.parse(PROXY_PROTOCOL_V2_SIGNATURE[:10]) self.assertEqual(self.protocol.version, None)