Merge lp:~cjwatson/launchpad/cveimport-requests into lp:launchpad

[Colin Watson]

Convert update-cve to urlfetch with explicit proxy configuration.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'constraints.txt'
--- constraints.txt	2018-05-31 10:23:03 +0000
+++ constraints.txt	2018-06-05 01:51:40 +0000
@@ -336,6 +336,7 @@
 PyYAML==3.10
 rabbitfixture==0.3.6
 requests==2.7.0
+requests-file==1.4.3
 requests-toolbelt==0.6.2
 responses==0.9.0
 scandir==1.7

=== modified file 'lib/lp/bugs/scripts/cveimport.py'
--- lib/lp/bugs/scripts/cveimport.py	2016-08-16 15:33:02 +0000
+++ lib/lp/bugs/scripts/cveimport.py	2018-06-05 01:51:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """A set of functions related to the ability to parse the XML CVE database,
@@ -10,9 +10,9 @@
 import gzip
 import StringIO
 import time
-import urllib2
 import xml.etree.cElementTree as cElementTree
 
+import requests
 from zope.component import getUtility
 from zope.event import notify
 from zope.interface import implementer
@@ -31,6 +31,10 @@
     LaunchpadCronScript,
     LaunchpadScriptFailure,
     )
+from lp.services.timeout import (
+    override_timeout,
+    urlfetch,
+    )
 
 
 CVEDB_NS = '{http://cve.mitre.org/cve/downloads/1.0}'
@@ -209,14 +213,22 @@
         elif self.options.cveurl is not None:
             self.logger.info("Downloading CVE database from %s..." %
                              self.options.cveurl)
+            proxies = {}
+            if config.launchpad.http_proxy:
+                proxies['http'] = config.launchpad.http_proxy
+                proxies['https'] = config.launchpad.http_proxy
             try:
-                url = urllib2.urlopen(self.options.cveurl)
-            except (urllib2.HTTPError, urllib2.URLError):
+                with override_timeout(config.cveupdater.timeout):
+                    # Command-line options are trusted, so allow file://
+                    # URLs to ease testing.
+                    response = urlfetch(
+                        self.options.cveurl, proxies=proxies, allow_file=True)
+            except requests.RequestException:
                 raise LaunchpadScriptFailure(
                     'Unable to connect for CVE database %s'
                     % self.options.cveurl)
 
-            cve_db_gz = url.read()
+            cve_db_gz = response.content
             self.logger.info("%d bytes downloaded." % len(cve_db_gz))
             cve_db = gzip.GzipFile(
                 fileobj=StringIO.StringIO(cve_db_gz)).read()

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2018-05-31 12:47:31 +0000
+++ lib/lp/services/config/schema-lazr.conf	2018-06-05 01:51:40 +0000
@@ -490,6 +490,9 @@
 # datatype: string
 cve_db_url: https://cve.mitre.org/data/downloads/allitems.xml.gz
 
+# datatype: integer
+timeout: 30
+
 
 [database]
 # Connection strings, format as per the PQconnectdb connection string as

=== modified file 'lib/lp/services/tests/test_timeout.py'
--- lib/lp/services/tests/test_timeout.py	2018-06-05 01:31:47 +0000
+++ lib/lp/services/tests/test_timeout.py	2018-06-05 01:51:40 +0000
@@ -15,14 +15,22 @@
 import threading
 import xmlrpclib
 
-from fixtures import MonkeyPatch
+from fixtures import (
+    MonkeyPatch,
+    TempDir,
+    )
 from requests import Response
 from requests.exceptions import (
     ConnectionError,
     InvalidSchema,
     )
-from testtools.matchers import MatchesStructure
+from testtools.matchers import (
+    ContainsDict,
+    Equals,
+    MatchesStructure,
+    )
 
+from lp.services.osutils import write_file
 from lp.services.timeout import (
     default_timeout,
     get_default_timeout_function,
@@ -368,6 +376,17 @@
         self.assertEqual(
             "No connection adapters were found for '%s'" % url, str(e))
 
+    def test_urlfetch_does_not_support_file_urls_by_default(self):
+        """urlfetch() does not support file urls by default."""
+        set_default_timeout_function(lambda: 1)
+        self.addCleanup(set_default_timeout_function, None)
+        test_path = self.useFixture(TempDir()).join('file')
+        write_file(test_path, '')
+        url = 'file://' + test_path
+        e = self.assertRaises(InvalidSchema, urlfetch, url)
+        self.assertEqual(
+            "No connection adapters were found for '%s'" % url, str(e))
+
     def test_urlfetch_no_proxy_by_default(self):
         """urlfetch does not use a proxy by default."""
         self.pushConfig('launchpad', http_proxy='http://proxy.example:3128/')
@@ -399,6 +418,18 @@
             {scheme: proxy for scheme in ('http', 'https')},
             fake_send.calls[0][1]['proxies'])
 
+    def test_urlfetch_supports_file_urls_if_allow_file(self):
+        """urlfetch() supports file urls if explicitly asked to do so."""
+        set_default_timeout_function(lambda: 1)
+        self.addCleanup(set_default_timeout_function, None)
+        test_path = self.useFixture(TempDir()).join('file')
+        write_file(test_path, 'Success.')
+        url = 'file://' + test_path
+        self.assertThat(urlfetch(url, allow_file=True), MatchesStructure(
+            status_code=Equals(200),
+            headers=ContainsDict({'Content-Length': Equals(8)}),
+            content=Equals('Success.')))
+
     def test_xmlrpc_transport(self):
         """ Another use case for timeouts is communicating with external
         systems using XMLRPC.  In order to allow timeouts using XMLRPC we

=== modified file 'lib/lp/services/timeout.py'
--- lib/lp/services/timeout.py	2018-06-05 01:31:47 +0000
+++ lib/lp/services/timeout.py	2018-06-05 01:51:40 +0000
@@ -40,6 +40,7 @@
     )
 from requests.packages.urllib3.exceptions import ClosedPoolError
 from requests.packages.urllib3.poolmanager import PoolManager
+from requests_file import FileAdapter
 from six import reraise
 
 from lp.services.config import config
@@ -318,25 +319,38 @@
 class URLFetcher:
     """Object fetching remote URLs with a time out."""
 
-    @staticmethod
-    def _makeSession(trust_env=None):
-        session = Session()
+    def __init__(self):
+        self.session = None
+
+    @with_timeout(cleanup='cleanup')
+    def fetch(self, url, trust_env=None, use_proxy=False, allow_file=False,
+              **request_kwargs):
+        """Fetch the URL using a custom HTTP handler supporting timeout.
+
+        :param url: The URL to fetch.
+        :param trust_env: If not None, set the session's trust_env to this
+            to determine whether it fetches proxy configuration from the
+            environment.
+        :param use_proxy: If True, use Launchpad's configured proxy.
+        :param allow_file: If True, allow file:// URLs.  (Be careful to only
+            pass this if the URL is trusted.)
+        :param request_kwargs: Additional keyword arguments passed on to
+            `Session.request`.
+        """
+        self.session = Session()
         if trust_env is not None:
-            session.trust_env = trust_env
+            self.session.trust_env = trust_env
         # Mount our custom adapters.
-        session.mount("https://", CleanableHTTPAdapter())
-        session.mount("http://", CleanableHTTPAdapter())
-        return session
+        self.session.mount("https://", CleanableHTTPAdapter())
+        self.session.mount("http://", CleanableHTTPAdapter())
+        if allow_file:
+            self.session.mount("file://", FileAdapter())
 
-    @with_timeout(cleanup='cleanup')
-    def fetch(self, url, trust_env=None, use_proxy=False, **request_kwargs):
-        """Fetch the URL using a custom HTTP handler supporting timeout."""
         request_kwargs.setdefault("method", "GET")
         if use_proxy and config.launchpad.http_proxy:
             request_kwargs.setdefault("proxies", {})
             request_kwargs["proxies"]["http"] = config.launchpad.http_proxy
             request_kwargs["proxies"]["https"] = config.launchpad.http_proxy
-        self.session = self._makeSession(trust_env=trust_env)
         response = self.session.request(url=url, **request_kwargs)
         response.raise_for_status()
         # Make sure the content has been consumed before returning.
@@ -345,7 +359,9 @@
 
     def cleanup(self):
         """Reset the connection when the operation timed out."""
-        self.session.close()
+        if self.session is not None:
+            self.session.close()
+        self.session = None
 
 
 def urlfetch(url, trust_env=None, **request_kwargs):

=== modified file 'setup.py'
--- setup.py	2018-05-31 10:23:03 +0000
+++ setup.py	2018-06-05 01:51:40 +0000
@@ -207,6 +207,7 @@
         'PyYAML',
         'rabbitfixture',
         'requests',
+        'requests-file',
         'requests-toolbelt',
         'responses',
         'scandir',