Skip to content

Commit 88f5e91

Browse files
Oberon00Oberon00
committed
http.host & url: Fix and test edge cases.
1 parent 09d224f commit 88f5e91

File tree

2 files changed

+78
-33
lines changed

2 files changed

+78
-33
lines changed

‎ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py

+21-14
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,17 @@ def _add_request_attributes(span, environ):
4747
span.set_attribute("component", "http")
4848
span.set_attribute("http.method", environ["REQUEST_METHOD"])
4949

50-
host = environ.get("HTTP_HOST") or environ["SERVER_NAME"]
51-
span.set_attribute("http.host", host) # NOTE: Nonstandard.
50+
host = environ.get("HTTP_HOST")
51+
if not host:
52+
host = environ["SERVER_NAME"]
53+
if environ["wsgi.url_scheme"] == "https":
54+
if environ.get("SERVER_PORT", "443") != "443":
55+
host += ":" + environ["SERVER_PORT"]
56+
elif environ.get("SERVER_PORT", "80") != "80":
57+
host += ":" + environ["SERVER_PORT"]
58+
59+
# NOTE: Nonstandard, may (not) include port
60+
span.set_attribute("http.host", host)
5261

5362
url = environ.get("REQUEST_URI") or environ.get("RAW_URI")
5463

@@ -60,18 +69,16 @@ def _add_request_attributes(span, environ):
6069
except Exception: # pylint:disable=broad-except
6170
url = wsgiref_util.request_uri(environ)
6271
else:
63-
if not urlparts.netloc:
64-
scheme = environ["wsgi.url_scheme"]
65-
portstr = ""
66-
if scheme == "https":
67-
if environ.get("SERVER_PORT", "443") != "443":
68-
portstr = ":" + environ["SERVER_PORT"]
69-
elif environ.get("SERVER_PORT", "80") != "80":
70-
portstr = ":" + environ["SERVER_PORT"]
71-
72-
url = (
73-
scheme + "://" + (host or "localhost") + portstr + url
74-
)
72+
if url.startswith("//"): # Scheme-relative URL
73+
url = url[2:]
74+
if not urlparts.netloc and urlparts.scheme: # E.g., "http:///?"
75+
scheme, path = url.split("://", 1)
76+
url = scheme + "://" + host + path
77+
elif not urlparts.netloc or not urlparts.scheme:
78+
scheme = environ["wsgi.url_scheme"] + "://"
79+
if not urlparts.netloc:
80+
url = host + url
81+
url = scheme + url
7582
else:
7683
url = wsgiref_util.request_uri(environ)
7784

‎ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py

+57-19
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import unittest
1818
import unittest.mock as mock
1919
import wsgiref.util as wsgiref_util
20+
from urllib.parse import urlparse
2021

2122
from opentelemetry import trace as trace_api
2223
from opentelemetry.ext.wsgi import OpenTelemetryMiddleware
@@ -191,35 +192,72 @@ def test_request_attributes(self):
191192
self.assertEqual(self.span.set_attribute.call_count, len(expected))
192193
self.span.set_attribute.assert_has_calls(expected, any_order=True)
193194

194-
def test_request_attributes_with_partial_raw_uri(self):
195-
self.environ["RAW_URI"] = "/#top"
195+
def validate_url(self, expected_url):
196196
OpenTelemetryMiddleware._add_request_attributes( # noqa pylint: disable=protected-access
197197
self.span, self.environ
198198
)
199-
self.span.set_attribute.assert_any_call(
200-
"http.url", "http://127.0.0.1/#top"
201-
)
199+
attrs = {args[0][0]: args[0][1] for args in self.span.set_attribute.call_args_list}
200+
self.assertIn("http.url", attrs)
201+
self.assertEqual(attrs["http.url"], expected_url)
202+
self.assertIn("http.host", attrs)
203+
self.assertEqual(attrs["http.host"], urlparse(attrs["http.url"]).netloc)
204+
205+
206+
def test_request_attributes_with_partial_raw_uri(self):
207+
self.environ["RAW_URI"] = "/#top"
208+
self.validate_url("http://127.0.0.1/#top")
202209

203210
def test_request_attributes_with_partial_raw_uri_and_nonstandard_port(
204211
self
205212
):
206-
self.environ["RAW_URI"] = "/#top"
213+
self.environ["RAW_URI"] = "/?"
214+
del self.environ["HTTP_HOST"]
207215
self.environ["SERVER_PORT"] = "8080"
208-
OpenTelemetryMiddleware._add_request_attributes( # noqa pylint: disable=protected-access
209-
self.span, self.environ
210-
)
211-
self.span.set_attribute.assert_any_call(
212-
"http.url", "http://127.0.0.1:8080/#top"
213-
)
216+
self.validate_url("http://127.0.0.1:8080/?")
217+
218+
def test_request_attributes_with_nonstandard_port_and_no_host(
219+
self
220+
):
221+
del self.environ["HTTP_HOST"]
222+
self.environ["SERVER_PORT"] = "8080"
223+
self.validate_url("http://127.0.0.1:8080/")
224+
225+
def test_request_attributes_with_nonstandard_port(
226+
self
227+
):
228+
self.environ["HTTP_HOST"] += ":8080"
229+
self.validate_url("http://127.0.0.1:8080/")
230+
231+
def test_request_attributes_with_scheme_relative_raw_uri(
232+
self
233+
):
234+
self.environ["RAW_URI"] = "//127.0.0.1/?"
235+
self.validate_url("http://127.0.0.1/?")
236+
237+
def test_request_attributes_with_netlocless_raw_uri(
238+
self
239+
):
240+
self.environ["RAW_URI"] = "http:///?"
241+
self.validate_url("http://127.0.0.1/?")
242+
243+
def test_request_attributes_with_pathless_raw_uri(
244+
self
245+
):
246+
self.environ["RAW_URI"] = "http://hello"
247+
self.environ["HTTP_HOST"] = "hello"
248+
self.validate_url("http://hello")
249+
250+
def test_request_attributes_with_strange_raw_uri(
251+
self
252+
):
253+
self.environ["RAW_URI"] = "http://?"
254+
self.validate_url("http://127.0.0.1?")
255+
214256

215257
def test_request_attributes_with_full_request_uri(self):
216-
self.environ["REQUEST_URI"] = "http://foobar.com:8080/?foo=bar#top"
217-
OpenTelemetryMiddleware._add_request_attributes( # noqa pylint: disable=protected-access
218-
self.span, self.environ
219-
)
220-
self.span.set_attribute.assert_any_call(
221-
"http.url", "http://foobar.com:8080/?foo=bar#top"
222-
)
258+
self.environ["HTTP_HOST"] = "127.0.0.1:8080"
259+
self.environ["REQUEST_URI"] = "http://127.0.0.1:8080/?foo=bar#top"
260+
self.validate_url("http://127.0.0.1:8080/?foo=bar#top")
223261

224262
def test_response_attributes(self):
225263
OpenTelemetryMiddleware._add_response_attributes( # noqa pylint: disable=protected-access

0 commit comments

Comments
 (0)