From dd96979c8302079665815f089c5720caec23482f Mon Sep 17 00:00:00 2001 From: Marco Neumann Date: Wed, 27 Mar 2019 11:01:31 +0100 Subject: [PATCH 1/3] split and extend proxy tests --- tests/test_client.py | 49 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index 907c187230..494a84c018 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,6 +1,7 @@ # coding: utf-8 import json import logging +import os import pytest import subprocess import sys @@ -43,28 +44,62 @@ def test_transport_option(monkeypatch): assert str(Client(transport=transport).dsn) == dsn -def test_http_proxy(monkeypatch): - client = Client("https://foo@sentry.io/123", http_proxy="http://localhost/123") +def test_proxy_http_use(monkeypatch): + client = Client("http://foo@sentry.io/123", http_proxy="http://localhost/123") assert client.transport._pool.proxy.scheme == "http" + +def test_proxy_https_use(monkeypatch): + client = Client("https://foo@sentry.io/123", http_proxy="https://localhost/123") + assert client.transport._pool.proxy.scheme == "https" + + +def test_proxy_both_select_http(monkeypatch): client = Client( - "https://foo@sentry.io/123", + "http://foo@sentry.io/123", https_proxy="https://localhost/123", http_proxy="http://localhost/123", ) - assert client.transport._pool.proxy.scheme == "https" - - client = Client("http://foo@sentry.io/123", http_proxy="http://localhost/123") assert client.transport._pool.proxy.scheme == "http" + +def test_proxy_both_select_https(monkeypatch): client = Client( - "http://foo@sentry.io/123", + "https://foo@sentry.io/123", https_proxy="https://localhost/123", http_proxy="http://localhost/123", ) + assert client.transport._pool.proxy.scheme == "https" + + +def test_proxy_http_fallback_http(monkeypatch): + client = Client("https://foo@sentry.io/123", http_proxy="http://localhost/123") + assert client.transport._pool.proxy.scheme == "http" + + +def test_proxy_none_noenv(monkeypatch): + client = Client("http://foo@sentry.io/123") + assert client.transport._pool.proxy is None + + +def test_proxy_none_httpenv_select(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + client = Client("http://foo@sentry.io/123") assert client.transport._pool.proxy.scheme == "http" +def test_proxy_none_httpsenv_select(monkeypatch): + os.environ["HTTPS_PROXY"] = "https://localhost/123" + client = Client("https://foo@sentry.io/123") + assert client.transport._pool.proxy.scheme == "https" + + +def test_proxy_none_httpenv_no_fallback(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + client = Client("https://foo@sentry.io/123") + assert client.transport._pool.proxy is None + + def test_simple_transport(): events = [] with Hub(Client(transport=events.append)): From a133017d1b860b6d76a62de63f0a7d719a78a9a4 Mon Sep 17 00:00:00 2001 From: Marco Neumann Date: Wed, 27 Mar 2019 12:28:44 +0100 Subject: [PATCH 2/3] fix HTTP fallback for proxy env vars Fixes #308 --- sentry_sdk/transport.py | 4 ++++ tests/test_client.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index d4a711a35b..de88ee7290 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -157,6 +157,10 @@ def _make_pool( if not proxy: proxy = getproxies().get(parsed_dsn.scheme) + # maybe fallback to HTTP proxy + if parsed_dsn.scheme == "https" and not proxy: + proxy = getproxies().get("http") + opts = self._get_pool_options(ca_certs) if proxy: diff --git a/tests/test_client.py b/tests/test_client.py index 494a84c018..c914583d6e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -94,10 +94,10 @@ def test_proxy_none_httpsenv_select(monkeypatch): assert client.transport._pool.proxy.scheme == "https" -def test_proxy_none_httpenv_no_fallback(monkeypatch): +def test_proxy_none_httpenv_fallback(monkeypatch): os.environ["HTTP_PROXY"] = "http://localhost/123" client = Client("https://foo@sentry.io/123") - assert client.transport._pool.proxy is None + assert client.transport._pool.proxy.scheme == "http" def test_simple_transport(): From 22c7d7ac23fbaccc6389e8d14e86ee87964a2190 Mon Sep 17 00:00:00 2001 From: Marco Neumann Date: Wed, 24 Apr 2019 15:41:36 +0200 Subject: [PATCH 3/3] Fine-grained control over proxy selection. Fixes #178 --- sentry_sdk/transport.py | 17 ++++++++------- tests/test_client.py | 47 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index de88ee7290..48d076034b 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -152,14 +152,15 @@ def _make_pool( ca_certs, # type: Optional[Any] ): # type: (...) -> Union[PoolManager, ProxyManager] - # Use http_proxy if scheme is https and https_proxy is not set - proxy = parsed_dsn.scheme == "https" and https_proxy or http_proxy - if not proxy: - proxy = getproxies().get(parsed_dsn.scheme) - - # maybe fallback to HTTP proxy - if parsed_dsn.scheme == "https" and not proxy: - proxy = getproxies().get("http") + proxy = None + + # try HTTPS first + if parsed_dsn.scheme == "https" and (https_proxy != ""): + proxy = https_proxy or getproxies().get("https") + + # maybe fallback to HTTP proxy + if not proxy and (http_proxy != ""): + proxy = http_proxy or getproxies().get("http") opts = self._get_pool_options(ca_certs) diff --git a/tests/test_client.py b/tests/test_client.py index c914583d6e..665cdc9727 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -100,6 +100,53 @@ def test_proxy_none_httpenv_fallback(monkeypatch): assert client.transport._pool.proxy.scheme == "http" +def test_proxy_bothselect_bothen(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + os.environ["HTTPS_PROXY"] = "https://localhost/123" + client = Client("https://foo@sentry.io/123", http_proxy="", https_proxy="") + assert client.transport._pool.proxy is None + + +def test_proxy_bothavoid_bothenv(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + os.environ["HTTPS_PROXY"] = "https://localhost/123" + client = Client("https://foo@sentry.io/123", http_proxy=None, https_proxy=None) + assert client.transport._pool.proxy.scheme == "https" + + +def test_proxy_bothselect_httpenv(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + client = Client("https://foo@sentry.io/123", http_proxy=None, https_proxy=None) + assert client.transport._pool.proxy.scheme == "http" + + +def test_proxy_httpselect_bothenv(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + os.environ["HTTPS_PROXY"] = "https://localhost/123" + client = Client("https://foo@sentry.io/123", http_proxy=None, https_proxy="") + assert client.transport._pool.proxy.scheme == "http" + + +def test_proxy_httpsselect_bothenv(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + os.environ["HTTPS_PROXY"] = "https://localhost/123" + client = Client("https://foo@sentry.io/123", http_proxy="", https_proxy=None) + assert client.transport._pool.proxy.scheme == "https" + + +def test_proxy_httpselect_httpsenv(monkeypatch): + os.environ["HTTPS_PROXY"] = "https://localhost/123" + client = Client("https://foo@sentry.io/123", http_proxy=None, https_proxy="") + assert client.transport._pool.proxy is None + + +def test_proxy_httpsselect_bothenv_http(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + os.environ["HTTPS_PROXY"] = "https://localhost/123" + client = Client("http://foo@sentry.io/123", http_proxy=None, https_proxy=None) + assert client.transport._pool.proxy.scheme == "http" + + def test_simple_transport(): events = [] with Hub(Client(transport=events.append)):