Merge lp:~mandel/ubuntu-sso-client/parse-empty into lp:ubuntu-sso-client

[Manuel de la Peña]

- Fixed the issue where we use an empty proxy url when the proxy was not set in gsettings (LP: #969280).

- Fixed the issue where we use an empty proxy url when the proxy was not set in gsettings (LP: #969280).

[Eric Casteleijn (thisfred)]

In general I prefer to use exceptions rather than signal values like None, but I assume there are other reasons to not to it that way. +1

review: Approve

[dobey (dobey)]

review: Approve

=== modified file 'ubuntu_sso/utils/webclient/gsettings.py'
--- ubuntu_sso/utils/webclient/gsettings.py	2012-03-07 15:12:39 +0000
+++ ubuntu_sso/utils/webclient/gsettings.py	2012-04-02 11:47:26 +0000
@@ -34,6 +34,11 @@
 def parse_manual_proxy_settings(scheme, gsettings):
     """Parse the settings for a given scheme."""
     host, username, pwd = parse_proxy_host(gsettings[scheme + ".host"])
+    # if the user did not set a proxy for a type (http/https/ftp) we should
+    # return None to ensure that it is not used
+    if host == '':
+        return None
+
     settings = {
         "host": host,
         "port": gsettings[scheme + ".port"],
@@ -76,7 +81,9 @@
     elif mode == "manual":
         settings = {}
         for scheme in ["http", "https"]:
-            settings[scheme] = parse_manual_proxy_settings(scheme, gsettings)
+            scheme_settings = parse_manual_proxy_settings(scheme, gsettings)
+            if scheme_settings is not None:
+                settings[scheme] = scheme_settings
     else:
         # If mode is automatic the PAC javascript should be interpreted
         # on each request. That is out of scope so it's ignored for now

=== modified file 'ubuntu_sso/utils/webclient/tests/test_gsettings.py'
--- ubuntu_sso/utils/webclient/tests/test_gsettings.py	2012-03-09 12:04:31 +0000
+++ ubuntu_sso/utils/webclient/tests/test_gsettings.py	2012-04-02 11:47:26 +0000
@@ -169,6 +169,29 @@
         ps = gsettings.get_proxy_settings()
         self.assertEqual(ps["http"], http_expected)
 
+    def _assert_parser_empty_url(self, scheme):
+        """Assert the parsing of an empty url."""
+        template_values = dict(BASE_GSETTINGS_VALUES)
+        template_values.update({
+            "mode": "manual",
+            scheme + "_host": '',
+            scheme + "_port": 0,
+            "http_use_auth": "false",
+        })
+        fake_output = TEMPLATE_GSETTINGS_OUTPUT.format(**template_values)
+        self.patch(gsettings.subprocess, "check_output",
+                   lambda _: fake_output)
+        ps = gsettings.get_proxy_settings()
+        self.assertNotIn(scheme, ps)
+
+    def test_gsettings_parser_empty_http_url(self):
+        """Test when there is no http proxy set."""
+        self._assert_parser_empty_url('http')
+
+    def test_gsettings_parser_empty_https_url(self):
+        """Test when there is no https proxy set."""
+        self._assert_parser_empty_url('https')
+
 
 class ParseProxyHostTestCase(TestCase):
     """Test the parsing of the domain."""