Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions sentry_sdk/integrations/stdlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,19 @@ def _install_httplib() -> None:
def putrequest(
self: "HTTPConnection", method: str, url: str, *args: "Any", **kwargs: "Any"
) -> "Any":
host = self.host
port = self.port
# proxy info is in _tunnel_host/_tunnel_port
tunnel_host = getattr(self, "_tunnel_host", None)
host = tunnel_host or self.host

default_port = self.default_port
tunnel_port = getattr(self, "_tunnel_port", None)
if tunnel_port:
port = tunnel_port
# need to override default_port for correct url recording
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest changing the url recording logic instead.

Otherwise, looks good to me!

if tunnel_port == 443:
default_port = 443
else:
port = self.port
Comment on lines +77 to +84

This comment was marked as outdated.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proxy port used when tunnel port is unspecified

Low Severity

When _tunnel_host is set but _tunnel_port is None (because set_tunnel was called without specifying a port), the code falls back to self.port which is the proxy port, not the tunnel destination port. This causes incorrect URL recording in the span. For example, if using HTTPConnection("proxy", 8080).set_tunnel("api.example.com") (no port), the recorded URL would incorrectly include :8080. The else branch assumes no tunnel is in use, but _tunnel_host being set indicates tunnel mode where the default port (80/443) is more appropriate than the proxy port.

Fix in Cursor Fix in Web


client = sentry_sdk.get_client()
if client.get_integration(StdlibIntegration) is None or is_sentry_url(
Expand Down Expand Up @@ -104,6 +114,11 @@ def putrequest(
span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)

if tunnel_host:
# for proxies, these point to the proxy host/port
span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, self.host)
span.set_data(SPANDATA.NETWORK_PEER_PORT, self.port)

rv = real_putrequest(self, method, url, *args, **kwargs)

if should_propagate_trace(client, real_url):
Expand Down
49 changes: 48 additions & 1 deletion tests/integrations/stdlib/test_httplib.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import os
import datetime
import socket
from http.client import HTTPConnection, HTTPSConnection
from http.server import BaseHTTPRequestHandler, HTTPServer
from socket import SocketIO
from threading import Thread
from urllib.error import HTTPError
from urllib.request import urlopen
from unittest import mock
Expand All @@ -12,11 +15,35 @@
from sentry_sdk.consts import MATCH_ALL, SPANDATA
from sentry_sdk.integrations.stdlib import StdlibIntegration

from tests.conftest import ApproxDict, create_mock_http_server
from tests.conftest import ApproxDict, create_mock_http_server, get_free_port

PORT = create_mock_http_server()


class MockProxyRequestHandler(BaseHTTPRequestHandler):
def do_CONNECT(self):
self.send_response(200, "Connection Established")
self.end_headers()

self.rfile.readline()

response = b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"
self.wfile.write(response)
self.wfile.flush()


def create_mock_proxy_server():
proxy_port = get_free_port()
proxy_server = HTTPServer(("localhost", proxy_port), MockProxyRequestHandler)
proxy_thread = Thread(target=proxy_server.serve_forever)
proxy_thread.daemon = True
proxy_thread.start()
return proxy_port


PROXY_PORT = create_mock_proxy_server()


def test_crumb_capture(sentry_init, capture_events):
sentry_init(integrations=[StdlibIntegration()])
events = capture_events()
Expand Down Expand Up @@ -642,3 +669,23 @@ def test_http_timeout(monkeypatch, sentry_init, capture_envelopes):
span = transaction["spans"][0]
assert span["op"] == "http.client"
assert span["description"] == f"GET http://localhost:{PORT}/bla" # noqa: E231


def test_proxy_https_tunnel(sentry_init, capture_events):
sentry_init(traces_sample_rate=1.0)
events = capture_events()

with start_transaction(name="test_transaction"):
conn = HTTPConnection("localhost", PROXY_PORT)
conn.set_tunnel("api.example.com", 443)
conn.request("GET", "/foo")
conn.getresponse()

(event,) = events
(span,) = event["spans"]

assert span["description"] == "GET https://api.example.com/foo"
assert span["data"]["url"] == "https://api.example.com/foo"
assert span["data"][SPANDATA.HTTP_METHOD] == "GET"
assert span["data"][SPANDATA.NETWORK_PEER_ADDRESS] == "localhost"
assert span["data"][SPANDATA.NETWORK_PEER_PORT] == PROXY_PORT
Loading