Skip to content

Commit

Permalink
instrumentation/urllib3: fix urllib3 2.0.1+ crash with many args
Browse files Browse the repository at this point in the history
In c1dd69e we changed the logic of
update_headers taking into account a new body arg before the one for
headers.
The problem is that HTTPConnectionPool.urlopen did not change at all,
only HTTPConnectionPool.request did so the old login update_headers was
fine.

Fix #1928
  • Loading branch information
xrmx committed Mar 12, 2024
1 parent bab896d commit 28a5d72
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
7 changes: 1 addition & 6 deletions elasticapm/instrumentation/packages/urllib3.py
Expand Up @@ -61,12 +61,7 @@ def update_headers(args, kwargs, instance, transaction, trace_parent):
:param trace_parent: the TraceParent object
:return: an (args, kwargs) tuple
"""
from urllib3._version import __version__ as urllib3_version

if urllib3_version.startswith("2") and len(args) >= 5 and args[4]:
headers = args[4].copy()
args = tuple(itertools.chain((args[:4]), (headers,), args[5:]))
elif len(args) >= 4 and args[3]:
if len(args) >= 4 and args[3]:
headers = args[3].copy()
args = tuple(itertools.chain((args[:3]), (headers,), args[4:]))
elif "headers" in kwargs and kwargs["headers"]:
Expand Down
17 changes: 17 additions & 0 deletions tests/instrumentation/urllib3_tests.py
Expand Up @@ -294,3 +294,20 @@ def test_instance_headers_are_respected(
assert "kwargs" in request_headers
if instance_headers and not (header_arg or header_kwarg):
assert "instance" in request_headers


def test_connection_pool_urlopen_does_not_crash_with_many_args(instrument, elasticapm_client, waiting_httpserver):
"""Mimics ConnectionPool.urlopen error path with broken connection, see #1928"""
waiting_httpserver.serve_content("")
url = waiting_httpserver.url + "/hello_world"
parsed_url = urllib.parse.urlparse(url)
pool = urllib3.HTTPConnectionPool(
parsed_url.hostname,
parsed_url.port,
maxsize=1,
block=True,
)
retry = urllib3.util.Retry(10)
elasticapm_client.begin_transaction("transaction")
r = pool.urlopen("GET", url, None, {"args": "true"}, retry, False, False)
assert r.status == 200

0 comments on commit 28a5d72

Please sign in to comment.