Skip to content

Commit 5bca690

Browse files
SNOW-2865839: fixed no_proxy issues while performing PUT (#2684)
Co-authored-by: Przemek Denkiewicz <[email protected]>
1 parent 7cadc70 commit 5bca690

File tree

10 files changed

+517
-9
lines changed

10 files changed

+517
-9
lines changed

DESCRIPTION.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
1010
- v4.1.1(TBD)
1111
- Relaxed pandas dependency requirements for Python below 3.12.
1212
- Changed CRL cache cleanup background task to daemon to avoid blocking main thread.
13+
- Fixed NO_PROXY issues with PUT operations
14+
1315
- v4.1.0(November 18,2025)
1416
- Added the `SNOWFLAKE_AUTH_FORCE_SERVER` environment variable to force the use of the local-listening server when using the `externalbrowser` auth method.
1517
- This allows headless environments (like Docker or Airflow) running locally to auth via a browser URL.

setup.cfg

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ boto =
8585
development =
8686
Cython
8787
coverage
88+
mitmproxy>=6.0.0; python_version >= '3.10'
8889
more-itertools
8990
numpy<=2.2.4
9091
pendulum!=2.1.1

src/snowflake/connector/session_manager.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -483,26 +483,32 @@ def make_session(self, *, url: str | None = None) -> Session:
483483
self._mount_adapters(session)
484484
return session
485485

486+
@staticmethod
487+
def _normalize_url(url: str | bytes | None) -> str:
488+
"""Normalize URL to string format (handles bytes from storage client)."""
489+
return url.decode("utf-8") if isinstance(url, bytes) else url
490+
486491
@contextlib.contextmanager
487492
@_propagate_session_manager_to_ocsp
488493
def use_session(
489-
self, url: str | bytes, use_pooling: bool | None = None
494+
self, url: str | bytes | None, use_pooling: bool | None = None
490495
) -> Generator[Session, Any, None]:
496+
"""Yield a session for the given URL (used for proxy handling and pooling).
497+
The 'url' is an obligatory parameter due to the need for correct proxy handling (i.e. bypassing caused by no_proxy settings).
491498
"""
492-
'url' is an obligatory parameter due to the need for correct proxy handling (i.e. bypassing caused by no_proxy settings).
493-
"""
499+
url_str = self._normalize_url(url)
494500
use_pooling = use_pooling if use_pooling is not None else self.use_pooling
495501
if not use_pooling:
496-
session = self.make_session(url=url)
502+
session = self.make_session(url=url_str)
497503
try:
498504
yield session
499505
finally:
500506
session.close()
501507
else:
502-
yield from self._yield_session_from_pool(url)
508+
yield from self._yield_session_from_pool(url_str)
503509

504510
def _yield_session_from_pool(
505-
self, url: str | bytes
511+
self, url: str | None
506512
) -> Generator[SessionT, Any, None]:
507513
hostname = self._get_pooling_key_from_url(url)
508514
pool = self._sessions_map[hostname]
@@ -635,8 +641,8 @@ def get_manager(
635641
config: HttpConfig | None = None, **http_config_kwargs
636642
) -> SessionManager:
637643
has_param_proxies = (
638-
hasattr(config, "proxy_host") or "proxies" in http_config_kwargs
639-
)
644+
config and config.proxy_host is not None
645+
) or "proxies" in http_config_kwargs
640646
if has_param_proxies:
641647
return ProxySessionManager(config, **http_config_kwargs)
642648
else:

test/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from logging import getLogger
77
from pathlib import Path
88
from test.test_utils.cross_module_fixtures.http_fixtures import * # NOQA
9+
from test.test_utils.cross_module_fixtures.mitm_fixtures import * # NOQA
910
from test.test_utils.cross_module_fixtures.wiremock_fixtures import * # NOQA
1011
from typing import Generator
1112

test/integ/test_proxies.py

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
#!/usr/bin/env python
2+
"""Integration test for proxy with real Snowflake connection.
3+
4+
Requirements:
5+
mitmproxy is installed automatically via the [development] extras in setup.cfg
6+
(Python 3.10+ only - mitmproxy has dependency conflicts on Python 3.9)
7+
8+
Important:
9+
When connecting through mitmproxy, you MUST set disable_ocsp_checks=True
10+
because mitmproxy performs MITM with self-signed certificates that cannot
11+
be validated via OCSP.
12+
"""
13+
from __future__ import annotations
14+
15+
import sys
16+
17+
import pytest
18+
19+
from snowflake.connector.compat import IS_WINDOWS
20+
21+
try:
22+
from snowflake.connector.util_text import random_string
23+
except ImportError:
24+
from ..randomize import random_string
25+
26+
27+
@pytest.mark.skipolddriver
28+
@pytest.mark.skipif(
29+
sys.version_info < (3, 10),
30+
reason="mitmproxy not installed for Python 3.9 due to dependency conflicts",
31+
)
32+
def test_put_with_https_proxy(conn_cnx, tmp_path, mitm_proxy, monkeypatch):
33+
test_file = tmp_path / "test_data.csv"
34+
test_file.write_text("col1,col2\n1,2\n3,4\n")
35+
36+
# Explicitly configure environment to use the proxy
37+
mitm_proxy.set_env_vars(monkeypatch)
38+
39+
# Connect to REAL Snowflake through mitmproxy
40+
# Must disable OCSP checks since mitmproxy presents self-signed certs for MITM
41+
with conn_cnx(
42+
disable_ocsp_checks=True,
43+
login_timeout=60, # Increase timeout for Windows proxy connection
44+
network_timeout=60, # Increase socket read timeout for proxy connections
45+
) as conn:
46+
with conn.cursor() as cur:
47+
stage_name = random_string(5, "test_proxy_")
48+
cur.execute(f"CREATE TEMPORARY STAGE {stage_name}")
49+
50+
# Use str().replace() for cross-platform file URI compatibility (like other tests)
51+
filename = str(test_file).replace("\\", "/")
52+
put_result = cur.execute(
53+
f"PUT 'file://{filename}' @{stage_name}"
54+
).fetchall()
55+
56+
assert len(put_result) > 0
57+
assert put_result[0][6] == "UPLOADED"
58+
59+
ls_result = cur.execute(f"LIST @{stage_name}").fetchall()
60+
assert len(ls_result) > 0
61+
62+
63+
@pytest.mark.skipolddriver
64+
@pytest.mark.skipif(
65+
sys.version_info < (3, 10) or IS_WINDOWS,
66+
reason="mitmproxy not installed for Python 3.9 due to dependency conflicts",
67+
)
68+
def test_put_with_https_proxy_and_no_proxy_regression(
69+
conn_cnx, tmp_path, mitm_proxy, monkeypatch
70+
):
71+
"""SNOW-2865839: PUT fails with TypeError when HTTPS_PROXY and NO_PROXY are set.
72+
73+
From bug report:
74+
"HTTPS_PROXY=http://localhost:8080 NO_PROXY='google.com' python test.py"
75+
causes TypeError during PUT operations.
76+
77+
Bug flow:
78+
1. HTTPS_PROXY set (mitmproxy)
79+
2. NO_PROXY set with ANY value (e.g., "google.com")
80+
3. Execute PUT operation
81+
4. storage_client passes bytes URL to use_session()
82+
5. Without fix: TypeError: inet_aton() argument 1 must be str, not bytes
83+
6. With fix: PUT succeeds
84+
"""
85+
test_file = tmp_path / "test_data.csv"
86+
test_file.write_text("col1,col2\n1,2\n3,4\n")
87+
88+
# Configure environment to use mitmproxy
89+
mitm_proxy.set_env_vars(monkeypatch)
90+
91+
# Set NO_PROXY with arbitrary value (from bug report)
92+
monkeypatch.setenv("NO_PROXY", "google.com")
93+
94+
with conn_cnx(
95+
disable_ocsp_checks=True,
96+
login_timeout=60, # Increase timeout for Windows proxy connection
97+
network_timeout=60, # Increase socket read timeout for proxy connections for Windows
98+
) as conn:
99+
with conn.cursor() as cur:
100+
stage_name = random_string(5, "test_no_proxy_")
101+
cur.execute(f"CREATE TEMPORARY STAGE {stage_name}")
102+
103+
# This is where the bug occurs - storage_client passes bytes URL
104+
# Use str().replace() for cross-platform file URI compatibility (like other tests)
105+
filename = str(test_file).replace("\\", "/")
106+
put_result = cur.execute(
107+
f"PUT 'file://{filename}' @{stage_name}"
108+
).fetchall()
109+
110+
assert len(put_result) > 0
111+
assert put_result[0][6] == "UPLOADED"
112+
113+
ls_result = cur.execute(f"LIST @{stage_name}").fetchall()
114+
assert len(ls_result) > 0
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
"""Mitmproxy fixtures for testing."""
2+
3+
from typing import Any, Generator, Union
4+
5+
import pytest
6+
7+
from ..mitm import MitmClient
8+
9+
10+
@pytest.fixture(scope="session")
11+
def mitm_proxy() -> Generator[Union[MitmClient, Any], Any, None]:
12+
"""Start mitmproxy for transparent HTTPS proxying in tests.
13+
14+
This fixture (session-scoped):
15+
- Starts mitmdump once for all tests
16+
- Waits for CA certificate generation
17+
- Returns MitmClient instance
18+
- Cleans up after all tests complete
19+
20+
The fixture does NOT automatically configure proxy settings.
21+
Tests should explicitly use the proxy via:
22+
1. Environment variables: mitm_proxy.set_env_vars(monkeypatch)
23+
2. Connection parameters: conn_cnx(proxy_host=mitm_proxy.host, ...)
24+
25+
Yields:
26+
MitmClient: Running mitmproxy client instance
27+
28+
Fails:
29+
When RuntimeError: If mitmproxy is not installed or fails to start
30+
"""
31+
try:
32+
with MitmClient() as client:
33+
yield client
34+
except (RuntimeError, TimeoutError) as e:
35+
pytest.fail(f"Failed to start mitmproxy: {e}")

test/test_utils/mitm/__init__.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
"""mitmproxy test utilities."""
2+
3+
from .mitm_client import MitmClient
4+
5+
__all__ = ["MitmClient"]

0 commit comments

Comments
 (0)