Embedders - fix proxies, default on http, tests

Author: PrimozGodec

Orange/misc/tests/test_embedder_utils.py

@@ -0,0 +1,65 @@
+import os
+import unittest
+
+from Orange.misc.utils.embedder_utils import get_proxies
+
+
+class TestProxies(unittest.TestCase):
+    def setUp(self) -> None:
+        self.previous_http = os.environ.get("http_proxy")
+        self.previous_https = os.environ.get("https_proxy")
+        os.environ.pop("http_proxy", None)
+        os.environ.pop("https_proxy", None)
+
+    def tearDown(self) -> None:
+        os.environ.pop("http_proxy", None)
+        os.environ.pop("https_proxy", None)
+        if self.previous_http is not None:
+            os.environ["http_proxy"] = self.previous_http
+        if self.previous_https is not None:
+            os.environ["https_proxy"] = self.previous_https
+
+    def test_add_scheme(self):
+        os.environ["http_proxy"] = "test1.com"
+        os.environ["https_proxy"] = "test2.com"
+        res = get_proxies()
+        self.assertEqual("http://test1.com", res["http://"])
+        self.assertEqual("http://test2.com", res["https://"])
+
+        os.environ["http_proxy"] = "test1.com/path"
+        os.environ["https_proxy"] = "test2.com/path"
+        res = get_proxies()
+        self.assertEqual("http://test1.com/path", res["http://"])
+        self.assertEqual("http://test2.com/path", res["https://"])
+
+        os.environ["http_proxy"] = "https://test1.com:123"
+        os.environ["https_proxy"] = "https://test2.com:124"
+        res = get_proxies()
+        self.assertEqual("https://test1.com:123", res["http://"])
+        self.assertEqual("https://test2.com:124", res["https://"])
+
+    def test_both_urls(self):
+        os.environ["http_proxy"] = "http://test1.com:123"
+        os.environ["https_proxy"] = "https://test2.com:124"
+        res = get_proxies()
+        self.assertEqual("http://test1.com:123", res["http://"])
+        self.assertEqual("https://test2.com:124", res["https://"])
+        self.assertNotIn("all://", res)
+
+    def test_http_only(self):
+        os.environ["http_proxy"] = "http://test1.com:123"
+        res = get_proxies()
+        self.assertEqual("http://test1.com:123", res["all://"])
+        self.assertNotIn("http://", res)
+        self.assertNotIn("https://", res)
+
+    def test_https_only(self):
+        os.environ["https_proxy"] = "https://test1.com:123"
+        res = get_proxies()
+        self.assertEqual("https://test1.com:123", res["all://"])
+        self.assertNotIn("http://", res)
+        self.assertNotIn("https://", res)
+
+
+if __name__ == "__main__":
+    unittest.main()

Orange/misc/utils/embedder_utils.py

@@ -1,6 +1,7 @@
 import logging
 import hashlib
 import pickle
+import re
 from os import environ
 from os.path import join, isfile
 from typing import Optional, Dict
@@ -78,22 +79,25 @@ def add(self, cache_key, value):
 
 def get_proxies() -> Optional[Dict[str, str]]:
     """
-    Return dict with proxy addresses if they exists.
+    Return dict with proxy addresses if they exist.
 
     Returns
     -------
     proxy_dict
         Dictionary with format {proxy type: proxy address} or None if
         they not set.
     """
-    def add_protocol(url: Optional[str], prot: str) -> Optional[str]:
-        if url and not url.startswith(prot):
-            return f"{prot}://{url}"
-        return url
-    http_proxy = add_protocol(environ.get("http_proxy"), "http")
-    https_proxy = add_protocol(environ.get("https_proxy"), "https")
+    def add_scheme(url: Optional[str]) -> Optional[str]:
+        if url is not None and not re.search(r"^[A-Za-z0-9+.\-]+://", url):
+            # if no scheme default to http - as other libraries do (e.g. requests)
+            return f"http://{url}"
+        else:
+            return url
+
+    http_proxy = add_scheme(environ.get("http_proxy"))
+    https_proxy = add_scheme(environ.get("https_proxy"))
     if http_proxy and https_proxy:  # both proxy addresses defined
-        return {"http://": https_proxy, "https://": https_proxy}
+        return {"http://": http_proxy, "https://": https_proxy}
     elif any([https_proxy, http_proxy]):  # one of the proxies defined
         return {"all://": http_proxy or https_proxy}
     return None  # proxies not defined