Repository: abhinavsingh/proxy.py
Pull request #957 (2022-01-10 20:44:24)

[HttpProtocolHandler] Handle invalid request parsing exceptions
Log the bytes for later inspection.  Example:

<img width="768" alt="Screen Shot 2022-01-11 at 1 38 42 AM" src="https://user-images.githubusercontent.com/126065/148836779-dc4a2dd5-780c-46e0-894c-359b6131b759.png">


codecov[bot]: # [Codecov](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) Report
> Merging [#957](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) (0675588) into [develop](https://codecov.io/gh/abhinavsingh/proxy.py/commit/1dba7f053cff2845543738c820d5909b59d100ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) (1dba7f0) will **decrease** coverage by `0.18%`.
> The diff coverage is `77.77%`.

[![Impacted file tree graph](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/graphs/tree.svg?width=650&height=150&src=pr&token=Zh9J7b4la2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh)](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh)

```diff
@@             Coverage Diff             @@
##           develop     #957      +/-   ##
===========================================
- Coverage    86.19%   86.00%   -0.19%     
===========================================
  Files          145      145              
  Lines         6380     6394      +14     
  Branches       646      598      -48     
===========================================
  Hits          5499     5499              
- Misses         761      768       +7     
- Partials       120      127       +7     
```

| Flag | Coverage Δ | |
|---|---|---|
| pytest | `85.97% <77.77%> (-0.08%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) | Coverage Δ | |
|---|---|---|
| [proxy/http/inspector/inspect\_traffic.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvaHR0cC9pbnNwZWN0b3IvaW5zcGVjdF90cmFmZmljLnB5) | `50.00% <0.00%> (ø)` | |
| [proxy/http/handler.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvaHR0cC9oYW5kbGVyLnB5) | `71.24% <40.00%> (-2.12%)` | :arrow_down: |
| [proxy/http/url.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvaHR0cC91cmwucHk=) | `98.43% <100.00%> (+0.05%)` | :arrow_up: |
| [tests/http/test\_url.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-dGVzdHMvaHR0cC90ZXN0X3VybC5weQ==) | `100.00% <100.00%> (ø)` | |
| [tests/test\_set\_open\_file\_limit.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-dGVzdHMvdGVzdF9zZXRfb3Blbl9maWxlX2xpbWl0LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
| [proxy/common/utils.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvY29tbW9uL3V0aWxzLnB5) | `91.59% <0.00%> (-2.53%)` | :arrow_down: |
| [proxy/testing/test\_case.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvdGVzdGluZy90ZXN0X2Nhc2UucHk=) | `97.82% <0.00%> (-2.18%)` | :arrow_down: |
| [proxy/common/flag.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvY29tbW9uL2ZsYWcucHk=) | `91.24% <0.00%> (-1.46%)` | :arrow_down: |
| [proxy/proxy.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvcHJveHkucHk=) | `99.03% <0.00%> (-0.97%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh). Last update [1dba7f0...0675588](https://codecov.io/gh/abhinavsingh/proxy.py/pull/957?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh).


Patch:
diff --git a/proxy/http/handler.py b/proxy/http/handler.py
index 4512729..8193255 100644
--- a/proxy/http/handler.py
+++ b/proxy/http/handler.py
@@ -302,7 +302,13 @@ class HttpProtocolHandler(BaseTcpServerHandler):
         #
         # TODO(abhinavsingh): Remove .tobytes after parser is
         # memoryview compliant
-        self.request.parse(data.tobytes())
+        try:
+            self.request.parse(data.tobytes())
+        except Exception as exc:
+            logger.exception('Error parsing the request', exc_info=exc)
+            raise HttpProtocolException(
+                'Error when parsing request: %s' % data.tobytes().decode('utf-8'),
+            )
         if not self.request.is_complete:
             return False
         # Discover which HTTP handler plugin is capable of
diff --git a/proxy/http/inspector/inspect_traffic.py b/proxy/http/inspector/inspect_traffic.py
index 4c64697..456626d 100644
--- a/proxy/http/inspector/inspect_traffic.py
+++ b/proxy/http/inspector/inspect_traffic.py
@@ -39,20 +39,10 @@ class InspectTrafficPlugin(WebSocketTransportBasePlugin):
         if message['method'] == 'enable_inspection':
             # inspection can only be enabled if --enable-events is used
             if not self.flags.enable_events:
-                self.client.queue(
-                    memoryview(
-                        WebsocketFrame.text(
-                            bytes_(
-                                json.dumps(
-                                    {
-                                        'id': message['id'],
-                                        'response': 'not enabled',
-                                    },
-                                ),
-                            ),
-                        ),
-                    ),
-                )
+                self.reply({
+                    'id': message['id'],
+                    'response': 'not enabled',
+                })
             else:
                 self.subscriber.setup()
                 self.reply(
diff --git a/proxy/http/url.py b/proxy/http/url.py
index 9a5db36..fc06412 100644
--- a/proxy/http/url.py
+++ b/proxy/http/url.py
@@ -74,14 +74,19 @@ class Url:
 
         We use heuristics based approach for our URL parser.
         """
-        if raw[0] == 47:    # SLASH == 47
+        # SLASH == 47, check if URL starts with single slash but not double slash
+        is_single_slash = raw[0] == 47
+        is_double_slash = is_single_slash and len(raw) >= 2 and raw[1] == 47
+        if is_single_slash and not is_double_slash:
             return cls(remainder=raw)
         is_http = raw.startswith(HTTP_URL_PREFIX)
         is_https = raw.startswith(HTTPS_URL_PREFIX)
-        if is_http or is_https:
+        if is_http or is_https or is_double_slash:
             rest = raw[len(b'https://'):] \
                 if is_https \
-                else raw[len(b'http://'):]
+                else raw[len(b'http://'):] \
+                if is_http \
+                else raw[len(SLASH + SLASH):]
             parts = rest.split(SLASH, 1)
             username, password, host, port = Url._parse(parts[0])
             return cls(


Test patch:
diff --git a/tests/http/test_url.py b/tests/http/test_url.py
index a640f74..0cfb8c6 100644
--- a/tests/http/test_url.py
+++ b/tests/http/test_url.py
@@ -134,3 +134,12 @@ class TestUrl(unittest.TestCase):
         self.assertEqual(url.remainder, None)
         self.assertEqual(url.username, b'user')
         self.assertEqual(url.password, b'pass')
+
+    def test_no_scheme_suffix(self) -> None:
+        url = Url.from_bytes(b'//example-server.net/server?arg=87')
+        self.assertEqual(url.scheme, b'http')
+        self.assertEqual(url.hostname, b'example-server.net')
+        self.assertEqual(url.port, None)
+        self.assertEqual(url.remainder, b'/server?arg=87')
+        self.assertEqual(url.username, None)
+        self.assertEqual(url.password, None)