From b9d84af6dd13fa2247131a608516308b9cbc9d90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Berkay=20=C3=96zt=C3=BCrk?= Date: Sun, 15 Jan 2023 12:15:26 +0300 Subject: [PATCH 1/4] Fix leaking proxy env vars --- src/snowflake/connector/connection.py | 10 +++--- src/snowflake/connector/network.py | 7 ++++ src/snowflake/connector/proxy.py | 49 +++++++++------------------ test/integ/test_connection.py | 10 +++--- test/unit/test_proxies.py | 36 +++++++------------- 5 files changed, 46 insertions(+), 66 deletions(-) diff --git a/src/snowflake/connector/connection.py b/src/snowflake/connector/connection.py index eb8f85304..d01d09aeb 100644 --- a/src/snowflake/connector/connection.py +++ b/src/snowflake/connector/connection.py @@ -32,7 +32,7 @@ from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey -from . import errors, proxy +from . import errors from ._query_context_cache import QueryContextCache from .auth import ( FIRST_PARTY_AUTHENTICATORS, @@ -927,16 +927,16 @@ def __open_connection(self): use_numpy=self._numpy, support_negative_year=self._support_negative_year ) - proxy.set_proxies( - self.proxy_host, self.proxy_port, self.proxy_user, self.proxy_password - ) - self._rest = SnowflakeRestful( host=self.host, port=self.port, protocol=self._protocol, inject_client_pause=self._inject_client_pause, connection=self, + proxy_host=self.proxy_host, + proxy_port=self.proxy_port, + proxy_user=self.proxy_user, + proxy_password=self.proxy_password, ) logger.debug("REST API object was created: %s:%s", self.host, self.port) diff --git a/src/snowflake/connector/network.py b/src/snowflake/connector/network.py index 06268d0d4..327cd1a99 100644 --- a/src/snowflake/connector/network.py +++ b/src/snowflake/connector/network.py @@ -87,6 +87,7 @@ ServiceUnavailableError, TooManyRequests, ) +from .proxy import get_proxy_url from .sqlstate import ( SQLSTATE_CONNECTION_NOT_EXISTS, SQLSTATE_CONNECTION_REJECTED, @@ -363,9 +364,14 @@ def __init__( protocol: str = "http", inject_client_pause: int = 0, connection: SnowflakeConnection | None = None, + proxy_host: str | None = None, + proxy_port: str | None = None, + proxy_user: str | None = None, + proxy_password: str | None = None, ) -> None: self._host = host self._port = port + self._proxy = get_proxy_url(proxy_host, proxy_port, proxy_user, proxy_password) self._protocol = protocol self._inject_client_pause = inject_client_pause self._connection = connection @@ -1160,6 +1166,7 @@ def make_requests_session(self) -> Session: s.mount("http://", ProxySupportAdapter(max_retries=REQUESTS_RETRY)) s.mount("https://", ProxySupportAdapter(max_retries=REQUESTS_RETRY)) s._reuse_count = itertools.count() + s.proxies = {"http": self._proxy, "https": self._proxy} return s @contextlib.contextmanager diff --git a/src/snowflake/connector/proxy.py b/src/snowflake/connector/proxy.py index 1729bf413..03132c1cf 100644 --- a/src/snowflake/connector/proxy.py +++ b/src/snowflake/connector/proxy.py @@ -5,43 +5,26 @@ from __future__ import annotations -import os - -def set_proxies( +def get_proxy_url( proxy_host: str | None, proxy_port: str | None, proxy_user: str | None = None, proxy_password: str | None = None, -) -> dict[str, str] | None: - """Sets proxy dict for requests.""" - PREFIX_HTTP = "http://" - PREFIX_HTTPS = "https://" - proxies = None +) -> str | None: + http_prefix = "http://" + https_prefix = "https://" + if proxy_host and proxy_port: - if proxy_host.startswith(PREFIX_HTTP): - proxy_host = proxy_host[len(PREFIX_HTTP) :] - elif proxy_host.startswith(PREFIX_HTTPS): - proxy_host = proxy_host[len(PREFIX_HTTPS) :] - if proxy_user or proxy_password: - proxy_auth = "{proxy_user}:{proxy_password}@".format( - proxy_user=proxy_user if proxy_user is not None else "", - proxy_password=proxy_password if proxy_password is not None else "", - ) + if proxy_host.startswith(http_prefix): + host = proxy_host[len(http_prefix) :] + elif proxy_host.startswith(https_prefix): + host = proxy_host[len(https_prefix) :] else: - proxy_auth = "" - proxies = { - "http": "http://{proxy_auth}{proxy_host}:{proxy_port}".format( - proxy_host=proxy_host, - proxy_port=str(proxy_port), - proxy_auth=proxy_auth, - ), - "https": "http://{proxy_auth}{proxy_host}:{proxy_port}".format( - proxy_host=proxy_host, - proxy_port=str(proxy_port), - proxy_auth=proxy_auth, - ), - } - os.environ["HTTP_PROXY"] = proxies["http"] - os.environ["HTTPS_PROXY"] = proxies["https"] - return proxies + host = proxy_host + auth = ( + f"{proxy_user or ''}:{proxy_password or ''}@" + if proxy_user or proxy_password + else "" + ) + return f"{http_prefix}{auth}{host}:{proxy_port}" diff --git a/test/integ/test_connection.py b/test/integ/test_connection.py index 4891a6fc5..5936fe208 100644 --- a/test/integ/test_connection.py +++ b/test/integ/test_connection.py @@ -524,6 +524,8 @@ def test_invalid_account_timeout(): @pytest.mark.timeout(15) def test_invalid_proxy(db_parameters): + http_proxy = os.environ.get("HTTP_PROXY") + https_proxy = os.environ.get("HTTPS_PROXY") with pytest.raises(OperationalError): snowflake.connector.connect( protocol="http", @@ -532,13 +534,13 @@ def test_invalid_proxy(db_parameters): password=db_parameters["password"], host=db_parameters["host"], port=db_parameters["port"], - login_timeout=5, + login_timeout=0, proxy_host="localhost", proxy_port="3333", ) - # NOTE environment variable is set if the proxy parameter is specified. - del os.environ["HTTP_PROXY"] - del os.environ["HTTPS_PROXY"] + # Proxy environment variables should not change + assert os.environ.get("HTTP_PROXY") == http_proxy + assert os.environ.get("HTTPS_PROXY") == https_proxy @pytest.mark.timeout(15) diff --git a/test/unit/test_proxies.py b/test/unit/test_proxies.py index 55aff685e..c8ef625e8 100644 --- a/test/unit/test_proxies.py +++ b/test/unit/test_proxies.py @@ -14,30 +14,18 @@ import snowflake.connector from snowflake.connector.errors import OperationalError - -def test_set_proxies(): - from snowflake.connector.proxy import set_proxies - - assert set_proxies("proxyhost", "8080") == { - "http": "http://proxyhost:8080", - "https": "http://proxyhost:8080", - } - assert set_proxies("http://proxyhost", "8080") == { - "http": "http://proxyhost:8080", - "https": "http://proxyhost:8080", - } - assert set_proxies("http://proxyhost", "8080", "testuser", "testpass") == { - "http": "http://testuser:testpass@proxyhost:8080", - "https": "http://testuser:testpass@proxyhost:8080", - } - assert set_proxies("proxyhost", "8080", "testuser", "testpass") == { - "http": "http://testuser:testpass@proxyhost:8080", - "https": "http://testuser:testpass@proxyhost:8080", - } - - # NOTE environment variable is set if the proxy parameter is specified. - del os.environ["HTTP_PROXY"] - del os.environ["HTTPS_PROXY"] +def test_get_proxy_url(): + from snowflake.connector.proxy import get_proxy_url + + assert get_proxy_url("host", "port", "user", "password") == ( + "http://user:password@host:port" + ) + assert get_proxy_url("host", "port") == "http://host:port" + + assert get_proxy_url("http://host", "port") == "http://host:port" + assert get_proxy_url("https://host", "port", "user", "password") == ( + "http://user:password@host:port" + ) @pytest.mark.skipolddriver From 6b54477a18400125d7d32c6a0fe3785d022b3e8c Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Tue, 20 Aug 2024 10:08:09 -0700 Subject: [PATCH 2/4] linting fixes --- src/snowflake/connector/proxy.py | 27 ++++++++++++++------------- test/unit/test_proxies.py | 1 + 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/snowflake/connector/proxy.py b/src/snowflake/connector/proxy.py index 03132c1cf..44add0882 100644 --- a/src/snowflake/connector/proxy.py +++ b/src/snowflake/connector/proxy.py @@ -15,16 +15,17 @@ def get_proxy_url( http_prefix = "http://" https_prefix = "https://" - if proxy_host and proxy_port: - if proxy_host.startswith(http_prefix): - host = proxy_host[len(http_prefix) :] - elif proxy_host.startswith(https_prefix): - host = proxy_host[len(https_prefix) :] - else: - host = proxy_host - auth = ( - f"{proxy_user or ''}:{proxy_password or ''}@" - if proxy_user or proxy_password - else "" - ) - return f"{http_prefix}{auth}{host}:{proxy_port}" + if not proxy_host or not proxy_port: + return None + if proxy_host.startswith(http_prefix): + host = proxy_host[len(http_prefix) :] + elif proxy_host.startswith(https_prefix): + host = proxy_host[len(https_prefix) :] + else: + host = proxy_host + auth = ( + f"{proxy_user or ''}:{proxy_password or ''}@" + if proxy_user or proxy_password + else "" + ) + return f"{http_prefix}{auth}{host}:{proxy_port}" diff --git a/test/unit/test_proxies.py b/test/unit/test_proxies.py index c8ef625e8..7edbb01a1 100644 --- a/test/unit/test_proxies.py +++ b/test/unit/test_proxies.py @@ -14,6 +14,7 @@ import snowflake.connector from snowflake.connector.errors import OperationalError + def test_get_proxy_url(): from snowflake.connector.proxy import get_proxy_url From dadcb5918d1540b7ef75a4b7a1402fa0cf9f8fff Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Tue, 20 Aug 2024 10:10:32 -0700 Subject: [PATCH 3/4] add changelog --- DESCRIPTION.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/DESCRIPTION.md b/DESCRIPTION.md index 46f2966ea..2aaede994 100644 --- a/DESCRIPTION.md +++ b/DESCRIPTION.md @@ -8,6 +8,9 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne # Release Notes +- v3.12.2(TBD) + - Fixed a bug where proxy settings were leaked through environmental variables + - v3.12.1(August 20,2024) - Fixed a bug that logged the session token when renewing a session. - Fixed a bug where disabling client telemetry did not work. From d71ab887593f489b2aaf7b8431f06535e60a97d8 Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Tue, 20 Aug 2024 10:13:10 -0700 Subject: [PATCH 4/4] add skipolddriver marker --- test/unit/test_proxies.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/test_proxies.py b/test/unit/test_proxies.py index 7edbb01a1..bd6bdf3c3 100644 --- a/test/unit/test_proxies.py +++ b/test/unit/test_proxies.py @@ -15,6 +15,7 @@ from snowflake.connector.errors import OperationalError +@pytest.mark.skipolddriver def test_get_proxy_url(): from snowflake.connector.proxy import get_proxy_url