Raise HttpProtocolException instead of ValueError

.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:

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,

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

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(

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

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

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)

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]

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',
+            )

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)

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)