Skip to content

Commit

Permalink
http.host & url: Fix and test edge cases.
Browse files Browse the repository at this point in the history
  • Loading branch information
Oberon00 committed Sep 18, 2019
1 parent 09d224f commit 8aee33c
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 32 deletions.
37 changes: 23 additions & 14 deletions ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,17 @@ def _add_request_attributes(span, environ):
span.set_attribute("component", "http")
span.set_attribute("http.method", environ["REQUEST_METHOD"])

host = environ.get("HTTP_HOST") or environ["SERVER_NAME"]
span.set_attribute("http.host", host) # NOTE: Nonstandard.
host = environ.get("HTTP_HOST")
if not host:
host = environ["SERVER_NAME"]
if environ["wsgi.url_scheme"] == "https":
if environ.get("SERVER_PORT", "443") != "443":
host += ":" + environ["SERVER_PORT"]
elif environ.get("SERVER_PORT", "80") != "80":
host += ":" + environ["SERVER_PORT"]

# NOTE: Nonstandard, may (not) include port
span.set_attribute("http.host", host)

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

Expand All @@ -60,18 +69,18 @@ def _add_request_attributes(span, environ):
except Exception: # pylint:disable=broad-except
url = wsgiref_util.request_uri(environ)
else:
if not urlparts.netloc:
scheme = environ["wsgi.url_scheme"]
portstr = ""
if scheme == "https":
if environ.get("SERVER_PORT", "443") != "443":
portstr = ":" + environ["SERVER_PORT"]
elif environ.get("SERVER_PORT", "80") != "80":
portstr = ":" + environ["SERVER_PORT"]

url = (
scheme + "://" + (host or "localhost") + portstr + url
)
if url.startswith("//"): # Scheme-relative URL
url = url[2:]
if (
not urlparts.netloc and urlparts.scheme
): # E.g., "http:///?"
scheme, path = url.split("://", 1)
url = scheme + "://" + host + path
elif not urlparts.netloc or not urlparts.scheme:
scheme = environ["wsgi.url_scheme"] + "://"
if not urlparts.netloc:
url = host + url
url = scheme + url
else:
url = wsgiref_util.request_uri(environ)

Expand Down
65 changes: 47 additions & 18 deletions ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import unittest
import unittest.mock as mock
import wsgiref.util as wsgiref_util
from urllib.parse import urlparse

from opentelemetry import trace as trace_api
from opentelemetry.ext.wsgi import OpenTelemetryMiddleware
Expand Down Expand Up @@ -191,35 +192,63 @@ def test_request_attributes(self):
self.assertEqual(self.span.set_attribute.call_count, len(expected))
self.span.set_attribute.assert_has_calls(expected, any_order=True)

def test_request_attributes_with_partial_raw_uri(self):
self.environ["RAW_URI"] = "/#top"
def validate_url(self, expected_url):
OpenTelemetryMiddleware._add_request_attributes( # noqa pylint: disable=protected-access
self.span, self.environ
)
self.span.set_attribute.assert_any_call(
"http.url", "http://127.0.0.1/#top"
attrs = {
args[0][0]: args[0][1]
for args in self.span.set_attribute.call_args_list
}
self.assertIn("http.url", attrs)
self.assertEqual(attrs["http.url"], expected_url)
self.assertIn("http.host", attrs)
self.assertEqual(
attrs["http.host"], urlparse(attrs["http.url"]).netloc
)

def test_request_attributes_with_partial_raw_uri(self):
self.environ["RAW_URI"] = "/#top"
self.validate_url("http://127.0.0.1/#top")

def test_request_attributes_with_partial_raw_uri_and_nonstandard_port(
self
):
self.environ["RAW_URI"] = "/#top"
self.environ["RAW_URI"] = "/?"
del self.environ["HTTP_HOST"]
self.environ["SERVER_PORT"] = "8080"
OpenTelemetryMiddleware._add_request_attributes( # noqa pylint: disable=protected-access
self.span, self.environ
)
self.span.set_attribute.assert_any_call(
"http.url", "http://127.0.0.1:8080/#top"
)
self.validate_url("http://127.0.0.1:8080/?")

def test_request_attributes_with_nonstandard_port_and_no_host(self):
del self.environ["HTTP_HOST"]
self.environ["SERVER_PORT"] = "8080"
self.validate_url("http://127.0.0.1:8080/")

def test_request_attributes_with_nonstandard_port(self):
self.environ["HTTP_HOST"] += ":8080"
self.validate_url("http://127.0.0.1:8080/")

def test_request_attributes_with_scheme_relative_raw_uri(self):
self.environ["RAW_URI"] = "//127.0.0.1/?"
self.validate_url("http://127.0.0.1/?")

def test_request_attributes_with_netlocless_raw_uri(self):
self.environ["RAW_URI"] = "http:///?"
self.validate_url("http://127.0.0.1/?")

def test_request_attributes_with_pathless_raw_uri(self):
self.environ["RAW_URI"] = "http://hello"
self.environ["HTTP_HOST"] = "hello"
self.validate_url("http://hello")

def test_request_attributes_with_strange_raw_uri(self):
self.environ["RAW_URI"] = "http://?"
self.validate_url("http://127.0.0.1?")

def test_request_attributes_with_full_request_uri(self):
self.environ["REQUEST_URI"] = "http://foobar.com:8080/?foo=bar#top"
OpenTelemetryMiddleware._add_request_attributes( # noqa pylint: disable=protected-access
self.span, self.environ
)
self.span.set_attribute.assert_any_call(
"http.url", "http://foobar.com:8080/?foo=bar#top"
)
self.environ["HTTP_HOST"] = "127.0.0.1:8080"
self.environ["REQUEST_URI"] = "http://127.0.0.1:8080/?foo=bar#top"
self.validate_url("http://127.0.0.1:8080/?foo=bar#top")

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

0 comments on commit 8aee33c

Please sign in to comment.