Repository: websocket-client/websocket-client

Issue:
Environment variable HTTP_PROXY is used for HTTPS connections
The problem occurred in an environment where a proxy server is to be used for HTTP connections but not for HTTPS connections.
In this case `http_proxy` is set in the environment but `https_proxy` is not.
The problematic code is here:

https://github.com/websocket-client/websocket-client/blob/bd506ad2e14749e1d31c07a1a6fca5644adb0ec4/websocket/_url.py#L161-L163

In my opinion, only `https_proxy` should be used for secure connections.


Discussion:
engn33r: I agree with the update. Although the user is expected to set `is_secure` to true when `https_proxy` is set, the updated code makes it more clear and foolproof that `is_secure` controls whether `http_proxy` or `https_proxy` is chosen. This avoids the case of falling back to `http_proxy` if `https_proxy` is not set when the user specifically sets `is_secure` to true.

Thanks for adding test coverage :+1:

Patch:
diff --git a/websocket/_url.py b/websocket/_url.py
index 259ce64..2141b02 100644
--- a/websocket/_url.py
+++ b/websocket/_url.py
@@ -137,7 +137,7 @@ def get_proxy_info(
         Websocket server name.
     is_secure: bool
         Is the connection secure? (wss) looks for "https_proxy" in env
-        before falling back to "http_proxy"
+        instead of "http_proxy"
     proxy_host: str
         http proxy host name.
     proxy_port: str or int
@@ -158,15 +158,11 @@ def get_proxy_info(
         auth = proxy_auth
         return proxy_host, port, auth
 
-    env_keys = ["http_proxy"]
-    if is_secure:
-        env_keys.insert(0, "https_proxy")
-
-    for key in env_keys:
-        value = os.environ.get(key, os.environ.get(key.upper(), "")).replace(" ", "")
-        if value:
-            proxy = urlparse(value)
-            auth = (unquote(proxy.username), unquote(proxy.password)) if proxy.username else None
-            return proxy.hostname, proxy.port, auth
+    env_key = "https_proxy" if is_secure else "http_proxy"
+    value = os.environ.get(env_key, os.environ.get(env_key.upper(), "")).replace(" ", "")
+    if value:
+        proxy = urlparse(value)
+        auth = (unquote(proxy.username), unquote(proxy.password)) if proxy.username else None
+        return proxy.hostname, proxy.port, auth
 
     return None, 0, None


Test patch:
diff --git a/websocket/tests/test_url.py b/websocket/tests/test_url.py
index 6a210d5..a74dd76 100644
--- a/websocket/tests/test_url.py
+++ b/websocket/tests/test_url.py
@@ -254,6 +254,24 @@ def testProxyFromEnv(self):
         os.environ["https_proxy"] = "http://localhost2:3128/"
         self.assertEqual(get_proxy_info("echo.websocket.events", True), ("localhost2", 3128, None))
 
+        os.environ["http_proxy"] = ""
+        os.environ["https_proxy"] = "http://localhost2/"
+        self.assertEqual(get_proxy_info("echo.websocket.events", True), ("localhost2", None, None))
+        self.assertEqual(get_proxy_info("echo.websocket.events", False), (None, 0, None))
+        os.environ["http_proxy"] = ""
+        os.environ["https_proxy"] = "http://localhost2:3128/"
+        self.assertEqual(get_proxy_info("echo.websocket.events", True), ("localhost2", 3128, None))
+        self.assertEqual(get_proxy_info("echo.websocket.events", False), (None, 0, None))
+
+        os.environ["http_proxy"] = "http://localhost/"
+        os.environ["https_proxy"] = ""
+        self.assertEqual(get_proxy_info("echo.websocket.events", True), (None, 0, None))
+        self.assertEqual(get_proxy_info("echo.websocket.events", False), ("localhost", None, None))
+        os.environ["http_proxy"] = "http://localhost:3128/"
+        os.environ["https_proxy"] = ""
+        self.assertEqual(get_proxy_info("echo.websocket.events", True), (None, 0, None))
+        self.assertEqual(get_proxy_info("echo.websocket.events", False), ("localhost", 3128, None))
+
         os.environ["http_proxy"] = "http://a:b@localhost/"
         self.assertEqual(get_proxy_info("echo.websocket.events", False), ("localhost", None, ("a", "b")))
         os.environ["http_proxy"] = "http://a:b@localhost:3128/"