Merge lp:~juliank/update-manager/lp1771914 into lp:update-manager

[Julian Andres Klode]

This implements support for https proxies to the extend we support http proxies. Unfortunately this breaks the API of the return value to return both http and https proxies in a dict indexed by method, but just returning http would be wrong too; and there do not seem to be any users of the return value, at least not in my /usr/lib.

[Brian Murray (brian-murray)]

ubuntu-release-upgrader calls init_proxy() but does not use the return value so I agree that breaking the API seems okay. I've one comment which appears in line otherwise this looks good to me.

review: Approve

[lp:~juliank/update-manager/lp1771914]

2826. By Julian Andres Klode on 2018-06-27

Remove an empty line in test_proxy

=== modified file 'UpdateManager/Core/utils.py'
--- UpdateManager/Core/utils.py	2018-05-30 19:11:04 +0000
+++ UpdateManager/Core/utils.py	2018-06-27 12:15:37 +0000
@@ -296,10 +296,10 @@
       if present
     """
     SYNAPTIC_CONF_FILE = "/root/.synaptic/synaptic.conf"
-    proxy = None
+    proxies = {}
     # generic apt config wins
     if apt_pkg.config.find("Acquire::http::Proxy") != '':
-        proxy = apt_pkg.config.find("Acquire::http::Proxy")
+        proxies["http"] = apt_pkg.config.find("Acquire::http::Proxy")
     # then synaptic
     elif os.path.exists(SYNAPTIC_CONF_FILE):
         cnf = apt_pkg.Configuration()
@@ -309,18 +309,26 @@
             proxy_host = cnf.find("Synaptic::httpProxy")
             proxy_port = str(cnf.find_i("Synaptic::httpProxyPort"))
             if proxy_host and proxy_port:
-                proxy = "http://%s:%s/" % (proxy_host, proxy_port)
+                proxies["http"] = "http://%s:%s/" % (proxy_host, proxy_port)
+    if apt_pkg.config.find("Acquire::https::Proxy") != '':
+        proxies["https"] = apt_pkg.config.find("Acquire::https::Proxy")
+    elif "http" in proxies:
+        proxies["https"] = proxies["http"]
     # if we have a proxy, set it
-    if proxy:
+    if proxies:
         # basic verification
-        if not re.match("http://\w+", proxy):
-            print("proxy '%s' looks invalid" % proxy, file=sys.stderr)
-            return
-        proxy_support = ProxyHandler({"http": proxy})
+        for proxy in proxies.values():
+            if not re.match("https?://\w+", proxy):
+                print("proxy '%s' looks invalid" % proxy, file=sys.stderr)
+                return
+        proxy_support = ProxyHandler(proxies)
         opener = build_opener(proxy_support)
         install_opener(opener)
-        os.putenv("http_proxy", proxy)
-    return proxy
+        if "http" in proxies:
+            os.putenv("http_proxy", proxies["http"])
+        if "https" in proxies:
+            os.putenv("https_proxy", proxies["https"])
+    return proxies
 
 
 def on_battery():

=== modified file 'debian/changelog'
--- debian/changelog	2018-06-08 06:35:07 +0000
+++ debian/changelog	2018-06-27 12:15:37 +0000
@@ -1,3 +1,10 @@
+update-manager (1:18.10.3) UNRELEASED; urgency=medium
+
+  * Add support for HTTPS proxies; this breaks UpdateManager.Core.utils.init_proxy()
+    API - the return value is now a dict, rather than a string (LP: #1771914).
+
+ -- Julian Andres Klode <[e-mail]>  Mon, 25 Jun 2018 11:14:40 +0200
+
 update-manager (1:18.10.2) cosmic; urgency=medium
 
   * Fix my spelling mistake in the previous upload. Oops.

=== modified file 'tests/test_proxy.py'
--- tests/test_proxy.py	2017-08-07 19:42:06 +0000
+++ tests/test_proxy.py	2018-06-27 12:15:37 +0000
@@ -13,26 +13,57 @@
 
 class TestInitProxy(unittest.TestCase):
     proxy = "http://10.0.2.2:3128"
+    https_proxy = "https://10.0.2.2:3128"
 
     def setUp(self):
         try:
             del os.environ["http_proxy"]
+            del os.environ["https_proxy"]
         except KeyError:
             pass
-        apt_pkg.config.set("Acquire::http::proxy", self.proxy)
+        apt_pkg.config.clear("Acquire::http::proxy")
+        apt_pkg.config.clear("Acquire::https::proxy")
 
     def tearDown(self):
-        try:
-            del os.environ["http_proxy"]
-        except KeyError:
-            pass
-        apt_pkg.config.clear("Acquire::http::proxy")
-
-    def testinitproxy(self):
-        from gi.repository import Gio
-        settings = Gio.Settings.new("com.ubuntu.update-manager")
-        detected_proxy = init_proxy(settings)
-        self.assertEqual(detected_proxy, self.proxy)
+        self.setUp()
+
+    def testinitproxyMixed(self):
+        apt_pkg.config.set("Acquire::http::proxy", self.proxy)
+        apt_pkg.config.set("Acquire::https::proxy", self.https_proxy)
+        from gi.repository import Gio
+        settings = Gio.Settings.new("com.ubuntu.update-manager")
+        detected_proxy = init_proxy(settings)
+        self.assertEqual(detected_proxy, {"http": self.proxy,
+                                          "https": self.https_proxy})
+
+    def testinitproxyHttpOnly(self):
+        apt_pkg.config.set("Acquire::http::proxy", self.proxy)
+        from gi.repository import Gio
+        settings = Gio.Settings.new("com.ubuntu.update-manager")
+        detected_proxy = init_proxy(settings)
+        self.assertEqual(detected_proxy, {"http": self.proxy,
+                                          "https": self.proxy})
+
+    def testinitproxyHttpOnlyWithHttpsUri(self):
+        apt_pkg.config.set("Acquire::http::proxy", self.https_proxy)
+        from gi.repository import Gio
+        settings = Gio.Settings.new("com.ubuntu.update-manager")
+        detected_proxy = init_proxy(settings)
+        self.assertEqual(detected_proxy, {"http": self.https_proxy,
+                                          "https": self.https_proxy})
+
+    def testinitproxyHttpsOnly(self):
+        apt_pkg.config.set("Acquire::https::proxy", self.https_proxy)
+        from gi.repository import Gio
+        settings = Gio.Settings.new("com.ubuntu.update-manager")
+        detected_proxy = init_proxy(settings)
+        self.assertEqual(detected_proxy, {"https": self.https_proxy})
+
+    def testinitproxyNoProxy(self):
+        from gi.repository import Gio
+        settings = Gio.Settings.new("com.ubuntu.update-manager")
+        detected_proxy = init_proxy(settings)
+        self.assertEqual(detected_proxy, {})
 
 
 if __name__ == '__main__':