Fix bugs with client proxy: target path, host with port (aio-libs#1413)

aiohttp/client_reqrep.py

@@ -432,9 +432,19 @@ def write_bytes(self, request, reader):
         self._writer = None
 
     def send(self, writer, reader):
-        path = self.url.raw_path
-        if self.url.raw_query_string:
-            path += '?' + self.url.raw_query_string
+        # Specify request target:
+        # - CONNECT request must send authority form URI
+        # - not CONNECT proxy must send absolute form URI
+        # - most common is origin form URI
+        if self.method == hdrs.METH_CONNECT:
+            path = '{}:{}'.format(self.url.host, self.url.port)
+        elif self.proxy and not self.ssl:
+            path = str(self.url)
+        else:
+            path = self.url.raw_path
+            if self.url.raw_query_string:
+                path += '?' + self.url.raw_query_string
+
         request = aiohttp.Request(writer, self.method, path,
                                   self.version)
 

aiohttp/connector.py

@@ -634,7 +634,7 @@ def _create_direct_connection(self, req):
     def _create_proxy_connection(self, req):
         proxy_req = ClientRequest(
             hdrs.METH_GET, req.proxy,
-            headers={hdrs.HOST: req.host},
+            headers={hdrs.HOST: req.headers[hdrs.HOST]},
             auth=req.proxy_auth,
             loop=self._loop)
         try:
@@ -644,8 +644,6 @@ def _create_proxy_connection(self, req):
         except OSError as exc:
             raise ProxyConnectionError(*exc.args) from exc
 
-        if not req.ssl:
-            req.path = str(req.url)
         if hdrs.AUTHORIZATION in proxy_req.headers:
             auth = proxy_req.headers[hdrs.AUTHORIZATION]
             del proxy_req.headers[hdrs.AUTHORIZATION]
@@ -665,7 +663,7 @@ def _create_proxy_connection(self, req):
             # to do this we must wrap raw socket into secure one
             # asyncio handles this perfectly
             proxy_req.method = hdrs.METH_CONNECT
-            proxy_req.path = '{}:{}'.format(req.host, req.port)
+            proxy_req.url = req.url
             key = (req.host, req.port, req.ssl)
             conn = Connection(self, key, proxy_req,
                               transport, proto, self._loop)

tests/test_proxy.py

@@ -40,7 +40,7 @@ def test_connect(self, ClientRequestMock):
         tr, proto = mock.Mock(), mock.Mock()
         self.loop.create_connection = make_mocked_coro((tr, proto))
         conn = self.loop.run_until_complete(connector.connect(req))
-        self.assertEqual(req.path, 'http://www.python.org')
+        self.assertEqual(req.url, URL('http://www.python.org'))
         self.assertIs(conn._transport, tr)
         self.assertIs(conn._protocol, proto)
 
@@ -105,7 +105,7 @@ def test_auth(self, ClientRequestMock):
         self.assertNotIn('PROXY-AUTHORIZATION', req.headers)
         conn = self.loop.run_until_complete(connector.connect(req))
 
-        self.assertEqual(req.path, 'http://www.python.org')
+        self.assertEqual(req.url, URL('http://www.python.org'))
         self.assertNotIn('AUTHORIZATION', req.headers)
         self.assertIn('PROXY-AUTHORIZATION', req.headers)
         self.assertNotIn('AUTHORIZATION', proxy_req.headers)
@@ -148,7 +148,7 @@ def test_auth_from_url(self, ClientRequestMock):
         self.assertNotIn('PROXY-AUTHORIZATION', req.headers)
         conn = self.loop.run_until_complete(connector.connect(req))
 
-        self.assertEqual(req.path, 'http://www.python.org')
+        self.assertEqual(req.url, URL('http://www.python.org'))
         self.assertNotIn('AUTHORIZATION', req.headers)
         self.assertIn('PROXY-AUTHORIZATION', req.headers)
         self.assertNotIn('AUTHORIZATION', proxy_req.headers)
@@ -212,7 +212,7 @@ def test_https_connect(self, ClientRequestMock):
 
         self.assertEqual(req.url.path, '/')
         self.assertEqual(proxy_req.method, 'CONNECT')
-        self.assertEqual(proxy_req.path, 'www.python.org:443')
+        self.assertEqual(proxy_req.url, URL('https://www.python.org'))
         tr.pause_reading.assert_called_once_with()
         tr.get_extra_info.assert_called_with('socket', default=None)
 
@@ -341,7 +341,7 @@ def test_request_port(self, ClientRequestMock):
             loop=self.loop,
         )
         self.loop.run_until_complete(connector._create_connection(req))
-        self.assertEqual(req.path, 'http://localhost:1234/path')
+        self.assertEqual(req.url, URL('http://localhost:1234/path'))
 
     def test_proxy_auth_property(self):
         req = aiohttp.ClientRequest(
@@ -393,7 +393,7 @@ def test_https_connect_pass_ssl_context(self, ClientRequestMock):
 
         self.assertEqual(req.url.path, '/')
         self.assertEqual(proxy_req.method, 'CONNECT')
-        self.assertEqual(proxy_req.path, 'www.python.org:443')
+        self.assertEqual(proxy_req.url, URL('https://www.python.org'))
         tr.pause_reading.assert_called_once_with()
         tr.get_extra_info.assert_called_with('socket', default=None)
 
@@ -494,7 +494,7 @@ def test_connect(self, ClientRequestMock):
         tr, proto = mock.Mock(), mock.Mock()
         self.loop.create_connection = make_mocked_coro((tr, proto))
         conn = self.loop.run_until_complete(connector.connect(req))
-        self.assertEqual(req.path, 'http://www.python.org')
+        self.assertEqual(req.url, URL('http://www.python.org'))
         self.assertIs(conn._transport, tr)
         self.assertIs(conn._protocol, proto)