fix: ScrapyProxyH2Agent

- add required test cases

BREAKING CHANGES
Presently the tests (in test_downloader_handlers.py)
1. test_download_without_proxy
2. test_download_with_proxy_https_timeout

collide with each other when run together. However, if both of the tests
are ran individually then both pass.

scrapy/core/downloader/handlers/http2.py

@@ -6,7 +6,7 @@ from urllib.parse import urldefrag
 from twisted.internet.base import ReactorBase
 from twisted.internet.defer import Deferred
 from twisted.internet.error import TimeoutError
-from twisted.web.client import URI
+from twisted.web.client import URI, BrowserLikePolicyForHTTPS
 
 from scrapy.core.downloader.contextfactory import load_context_factory_from_settings
 from scrapy.core.downloader.webclient import _parse
@@ -45,19 +45,24 @@ class ScrapyProxyH2Agent(H2Agent):
     def __init__(
         self, reactor: ReactorBase,
         proxy_uri: URI, pool: H2ConnectionPool,
+        context_factory=BrowserLikePolicyForHTTPS(),
         connect_timeout: Optional[float] = None, bind_address: Optional[bytes] = None
     ) -> None:
         super(ScrapyProxyH2Agent, self).__init__(
             reactor=reactor,
             pool=pool,
+            context_factory=context_factory,
             connect_timeout=connect_timeout,
             bind_address=bind_address
         )
         self._proxy_uri = proxy_uri
 
-    @staticmethod
-    def get_key(uri: URI) -> Tuple:
-        return "http-proxy", uri.host, uri.port
+    def get_endpoint(self, uri: URI):
+        return self.endpoint_factory.endpointForURI(self._proxy_uri)
+
+    def get_key(self, uri: URI) -> Tuple:
+        """We use the proxy uri instead of uri obtained from request url"""
+        return "http-proxy", self._proxy_uri.host, self._proxy_uri.port
 
 
 class ScrapyH2Agent:
@@ -100,6 +105,7 @@ class ScrapyH2Agent:
             else:
                 return self._ProxyAgent(
                     reactor=reactor,
+                    context_factory=self._context_factory,
                     proxy_uri=URI.fromBytes(bytes(proxy, encoding='ascii')),
                     connect_timeout=timeout,
                     bind_address=bind_address,

scrapy/core/http2/agent.py

@@ -118,22 +118,25 @@ class H2Agent:
         self._reactor = reactor
         self._pool = pool
         self._context_factory = AcceptableProtocolsContextFactory(context_factory, acceptable_protocols=[b'h2'])
-        self._endpoint_factory = _StandardEndpointFactory(
+        self.endpoint_factory = _StandardEndpointFactory(
             self._reactor, self._context_factory,
             connect_timeout, bind_address
         )
 
-    def _get_endpoint(self, uri: URI):
-        return self._endpoint_factory.endpointForURI(uri)
+    def get_endpoint(self, uri: URI):
+        return self.endpoint_factory.endpointForURI(uri)
 
-    @staticmethod
-    def get_key(uri: URI) -> Tuple:
+    def get_key(self, uri: URI) -> Tuple:
+        """
+        Arguments:
+            uri - URI obtained directly from request URL
+        """
         return uri.scheme, uri.host, uri.port
 
     def request(self, request: Request, spider: Spider) -> Deferred:
         uri = URI.fromBytes(bytes(request.url, encoding='utf-8'))
         try:
-            endpoint = self._get_endpoint(uri)
+            endpoint = self.get_endpoint(uri)
         except SchemeNotSupported:
             return defer.fail(Failure())
 

scrapy/core/http2/stream.py

@@ -185,14 +185,32 @@ class Stream:
         if url.query:
             path += '?' + url.query
 
+        # This pseudo-header field MUST NOT be empty for "http" or "https"
+        # URIs; "http" or "https" URIs that do not contain a path component
+        # MUST include a value of '/'. The exception to this rule is an
+        # OPTIONS request for an "http" or "https" URI that does not include
+        # a path component; these MUST include a ":path" pseudo-header field
+        # with a value of '*' (refer RFC 7540 - Section 8.1.2.3)
+        if not path:
+            if self._request.method == 'OPTIONS':
+                path = path or '*'
+            else:
+                path = path or '/'
+
         # Make sure pseudo-headers comes before all the other headers
         headers = [
             (':method', self._request.method),
             (':authority', url.netloc),
-            (':scheme', self._protocol.metadata['uri'].scheme),
-            (':path', path),
         ]
 
+        # The ":scheme" and ":path" pseudo-header fields MUST
+        # be omitted for CONNECT method (refer RFC 7540 - Section 8.3)
+        if self._request.method != 'CONNECT':
+            headers += [
+                (':scheme', self._protocol.metadata['uri'].scheme),
+                (':path', path),
+            ]
+
         for name, value in self._request.headers.items():
             headers.append((str(name, 'utf-8'), str(value[0], 'utf-8')))
 

tests/test_downloader_handlers.py

@@ -786,7 +786,10 @@ class HttpProxyTestCase(unittest.TestCase):
         def _test(response):
             self.assertEqual(response.status, 200)
             self.assertEqual(response.url, request.url)
-            self.assertEqual(response.body, b'http://example.com')
+            self.assertTrue(
+                response.body == b'http://example.com'  # HTTP/1.x
+                or response.body == b'/'  # HTTP/2
+            )
 
         http_proxy = self.getURL('')
         request = Request('http://example.com', meta={'proxy': http_proxy})
@@ -796,10 +799,13 @@ class HttpProxyTestCase(unittest.TestCase):
         def _test(response):
             self.assertEqual(response.status, 200)
             self.assertEqual(response.url, request.url)
-            self.assertEqual(response.body, b'https://example.com')
+            self.assertTrue(
+                response.body == b'http://example.com'  # HTTP/1.x
+                or response.body == b'/'  # HTTP/2
+            )
 
         http_proxy = '%s?noconnect' % self.getURL('')
-        request = Request('https://example.com', meta={'proxy': http_proxy})
+        request = Request('http://example.com', meta={'proxy': http_proxy})
         with self.assertWarnsRegex(ScrapyDeprecationWarning,
                                    r'Using HTTPS proxies in the noconnect mode is deprecated'):
             return self.download_request(request, Spider('foo')).addCallback(_test)
@@ -836,10 +842,30 @@ class Http11ProxyTestCase(HttpProxyTestCase):
         self.assertIn(domain, timeout.osError)
 
 
-# TODO:
-class Http2ProxyTestCase(Http11ProxyTestCase):
+class Https2ProxyTestCase(Http11ProxyTestCase):
+    # only used for HTTPS tests
+    keyfile = 'keys/localhost.key'
+    certfile = 'keys/localhost.crt'
+
+    scheme = 'https'
+    host = u'127.0.0.1'
+
     download_handler_cls = H2DownloadHandler
 
+    def setUp(self):
+        site = server.Site(UriResource(), timeout=None)
+        self.port = reactor.listenSSL(
+            0, site,
+            ssl_context_factory(self.keyfile, self.certfile),
+            interface=self.host
+        )
+        self.portno = self.port.getHost().port
+        self.download_handler = create_instance(self.download_handler_cls, None, get_crawler())
+        self.download_request = self.download_handler.download_request
+
+    def getURL(self, path):
+        return f"{self.scheme}://{self.host}:{self.portno}/{path}"
+
 
 class HttpDownloadHandlerMock: