Repository: abhinavsingh/proxy.py
Pull request #360 (2020-06-06 12:39:26)

urlsplit compliance for https CONNECT


Patch:
diff --git a/proxy/http/parser.py b/proxy/http/parser.py
index bdd2683..79d9a04 100644
--- a/proxy/http/parser.py
+++ b/proxy/http/parser.py
@@ -105,19 +105,21 @@ class HttpParser:
             self.del_header(key.lower())
 
     def set_url(self, url: bytes) -> None:
+        # Work around with urlsplit semantics.
+        #
+        # For CONNECT requests, request line contains
+        # upstream_host:upstream_port which is not complaint
+        # with urlsplit, which expects a fully qualified url.
+        if self.method == b'CONNECT':
+            url = b'https://' + url
         self.url = urlparse.urlsplit(url)
         self.set_line_attributes()
 
     def set_line_attributes(self) -> None:
         if self.type == httpParserTypes.REQUEST_PARSER:
             if self.method == httpMethods.CONNECT and self.url:
-                if self.url.scheme == b'':
-                    u = urlparse.urlsplit(b'//' + self.url.path)
-                    self.host, self.port = u.hostname, u.port
-                else:
-                    self.host = self.url.scheme
-                    self.port = 443 if self.url.path == b'' else \
-                        int(self.url.path)
+                self.host = self.url.hostname
+                self.port = 443 if self.url.port is None else self.url.port
             elif self.url:
                 self.host, self.port = self.url.hostname, self.url.port \
                     if self.url.port else DEFAULT_HTTP_PORT
@@ -125,7 +127,7 @@ class HttpParser:
                 raise KeyError(
                     'Invalid request. Method: %r, Url: %r' %
                     (self.method, self.url))
-            self.path = self.build_url()
+            self.path = self.build_path()
 
     def is_chunked_encoded(self) -> bool:
         return b'transfer-encoding' in self.headers and \
@@ -222,7 +224,7 @@ class HttpParser:
         value = COLON.join(parts[1:]).strip()
         self.add_headers([(key, value)])
 
-    def build_url(self) -> bytes:
+    def build_path(self) -> bytes:
         if not self.url:
             return b'/None'
         url = self.url.path
diff --git a/proxy/http/proxy/server.py b/proxy/http/proxy/server.py
index 7662414..db03de9 100644
--- a/proxy/http/proxy/server.py
+++ b/proxy/http/proxy/server.py
@@ -278,8 +278,8 @@ class HttpProxyPlugin(HttpProtocolHandlerPlugin):
                     logger.error(
                         'BrokenPipeError when wrapping client')
                     return True
-                except OSError:
-                    logger.error('OSError when wrapping client')
+                except OSError as e:
+                    logger.exception('OSError when wrapping client', exc_info=e)
                     return True
                 # Update all plugin connection reference
                 for plugin in self.plugins.values():
@@ -398,8 +398,8 @@ class HttpProxyPlugin(HttpProtocolHandlerPlugin):
         self.client._conn = ssl.wrap_socket(
             self.client.connection,
             server_side=True,
-            keyfile=self.flags.ca_signing_key_file,
-            certfile=generated_cert)
+            certfile=generated_cert,
+            keyfile=self.flags.ca_signing_key_file)
         self.client.connection.setblocking(False)
         logger.debug(
             'TLS interception using %s', generated_cert)


Test patch:
diff --git a/requirements-testing.txt b/requirements-testing.txt
index cc6dd2a..2815720 100644
--- a/requirements-testing.txt
+++ b/requirements-testing.txt
@@ -10,3 +10,4 @@ codecov==2.1.4
 tox==3.15.1
 mccabe==0.6.1
 pylint==2.5.2
+rope==0.17.0
diff --git a/tests/http/test_http_parser.py b/tests/http/test_http_parser.py
index 896b778..c98e638 100644
--- a/tests/http/test_http_parser.py
+++ b/tests/http/test_http_parser.py
@@ -132,7 +132,7 @@ class TestHttpParser(unittest.TestCase):
                      b'example.com')
         self.parser.parse(pkt)
         self.assertEqual(self.parser.total_size, len(pkt))
-        self.assertEqual(self.parser.build_url(), b'/path/dir/?a=b&c=d#p=q')
+        self.assertEqual(self.parser.build_path(), b'/path/dir/?a=b&c=d#p=q')
         self.assertEqual(self.parser.method, b'GET')
         assert self.parser.url
         self.assertEqual(self.parser.url.hostname, b'example.com')
@@ -149,7 +149,7 @@ class TestHttpParser(unittest.TestCase):
             self.parser.build())
 
     def test_build_url_none(self) -> None:
-        self.assertEqual(self.parser.build_url(), b'/None')
+        self.assertEqual(self.parser.build_path(), b'/None')
 
     def test_line_rcvd_to_rcving_headers_state_change(self) -> None:
         pkt = b'GET http://localhost HTTP/1.1'