Merge pull request #4198 from wRAR/deprecate-noconnect

Deprecate the HTTPS proxy noconnect mode.

scrapy/core/downloader/handlers/http11.py

@@ -16,6 +16,7 @@ from twisted.web.http import _DataLoss, PotentialDataLoss
 from twisted.web.client import Agent, ResponseDone, HTTPConnectionPool, ResponseFailed, URI
 from twisted.internet.endpoints import TCP4ClientEndpoint
 
+from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.http import Headers
 from scrapy.responsetypes import responsetypes
 from scrapy.core.downloader.webclient import _parse
@@ -285,6 +286,12 @@ class ScrapyAgent(object):
             scheme = _parse(request.url)[0]
             proxyHost = to_unicode(proxyHost)
             omitConnectTunnel = b'noconnect' in proxyParams
+            if omitConnectTunnel:
+                warnings.warn("Using HTTPS proxies in the noconnect mode is deprecated. "
+                              "If you use Crawlera, it doesn't require this mode anymore, "
+                              "so you should update scrapy-crawlera to 1.3.0+ "
+                              "and remove '?noconnect' from the Crawlera URL.",
+                              ScrapyDeprecationWarning)
             if scheme == b'https' and not omitConnectTunnel:
                 proxyAuth = request.headers.get(b'Proxy-Authorization', None)
                 proxyConf = (proxyHost, proxyPort, proxyAuth)

tests/test_downloader_handlers.py

@@ -33,7 +33,7 @@ from scrapy.responsetypes import responsetypes
 from scrapy.settings import Settings
 from scrapy.utils.test import get_crawler, skip_if_no_boto
 from scrapy.utils.python import to_bytes
-from scrapy.exceptions import NotConfigured
+from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning
 
 from tests.mockserver import MockServer, ssl_context_factory, Echo
 from tests.spiders import SingleRequestSpider
@@ -695,7 +695,9 @@ class HttpProxyTestCase(unittest.TestCase):
 
         http_proxy = '%s?noconnect' % self.getURL('')
         request = Request('https://example.com', meta={'proxy': http_proxy})
-        return self.download_request(request, Spider('foo')).addCallback(_test)
+        with self.assertWarnsRegex(ScrapyDeprecationWarning,
+                                   r'Using HTTPS proxies in the noconnect mode is deprecated'):
+            return self.download_request(request, Spider('foo')).addCallback(_test)
 
     def test_download_without_proxy(self):
         def _test(response):
@@ -710,6 +712,9 @@ class HttpProxyTestCase(unittest.TestCase):
 class Http10ProxyTestCase(HttpProxyTestCase):
     download_handler_cls = HTTP10DownloadHandler
 
+    def test_download_with_proxy_https_noconnect(self):
+        raise unittest.SkipTest('noconnect is not supported in HTTP10DownloadHandler')
+
 
 class Http11ProxyTestCase(HttpProxyTestCase):
     download_handler_cls = HTTP11DownloadHandler

tests/test_proxy_connect.py

@@ -108,32 +108,6 @@ class ProxyConnectTestCase(TestCase):
         echo = json.loads(crawler.spider.meta['responses'][0].text)
         self.assertTrue('Proxy-Authorization' not in echo['headers'])
 
-    # The noconnect mode isn't supported by the current mitmproxy, it returns
-    # "Invalid request scheme: https" as it doesn't seem to support full URLs in GET at all,
-    # and it's not clear what behavior is intended by Scrapy and by mitmproxy here.
-    # https://github.com/mitmproxy/mitmproxy/issues/848 may be related.
-    # The Scrapy noconnect mode was required, at least in the past, to work with Crawlera,
-    # and https://github.com/scrapy-plugins/scrapy-crawlera/pull/44 seems to be related.
-
-    @pytest.mark.xfail(reason='mitmproxy gives an error for noconnect requests')
-    @defer.inlineCallbacks
-    def test_https_noconnect(self):
-        proxy = os.environ['https_proxy']
-        os.environ['https_proxy'] = proxy + '?noconnect'
-        crawler = get_crawler(SimpleSpider)
-        with LogCapture() as l:
-            yield crawler.crawl(self.mockserver.url("/status?n=200", is_secure=True))
-        self._assert_got_response_code(200, l)
-
-    @pytest.mark.xfail(reason='mitmproxy gives an error for noconnect requests')
-    @defer.inlineCallbacks
-    def test_https_noconnect_auth_error(self):
-        os.environ['https_proxy'] = _wrong_credentials(os.environ['https_proxy']) + '?noconnect'
-        crawler = get_crawler(SimpleSpider)
-        with LogCapture() as l:
-            yield crawler.crawl(self.mockserver.url("/status?n=200", is_secure=True))
-        self._assert_got_response_code(407, l)
-
     def _assert_got_response_code(self, code, log):
         print(log)
         self.assertEqual(str(log).count('Crawled (%d)' % code), 1)