diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index fe6c41cd70..fc071f0df6 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -156,10 +156,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) + 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 ec4b77e8ff..5afd1cb6d2 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 @@ -52,25 +53,106 @@ 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_fallback(monkeypatch): + os.environ["HTTP_PROXY"] = "http://localhost/123" + client = Client("https://foo@sentry.io/123") + 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"