Skip to content

Commit 4729ea7

Browse files
author
teddiesloco
committed
fix: catch malformed port ValueError and reach 100% coverage
- _canonical_netloc now catches ValueError from parsed.port for malformed explicit ports (e.g. out-of-range), falling back to the original netloc instead of letting check_resource_allowed() crash - add test coverage for the userinfo-in-netloc and IPv6-literal branches that were previously untested (CI requires 100% coverage) - add regression test for the malformed-port fallback
1 parent e8b2999 commit 4729ea7

2 files changed

Lines changed: 27 additions & 1 deletion

File tree

src/mcp/shared/auth_utils.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ def _canonical_netloc(parsed: SplitResult) -> str:
1010
"""Normalize netloc by lowercasing and stripping explicit default ports (RFC 3986 §6.2.3)."""
1111
scheme = parsed.scheme.lower()
1212
netloc = parsed.netloc.lower()
13-
port = parsed.port
13+
try:
14+
port = parsed.port
15+
except ValueError:
16+
# Malformed explicit port (e.g. non-numeric) - not canonicalizable, fall back as-is
17+
return netloc
1418

1519
if (scheme == "http" and port == 80) or (scheme == "https" and port == 443):
1620
# Strip default port while preserving userinfo and IPv6 brackets

tests/shared/test_auth_utils.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,23 @@ def test_resource_url_from_server_url_preserves_port():
3838
assert resource_url_from_server_url("https://example.com:443") == "https://example.com"
3939

4040

41+
def test_resource_url_from_server_url_strips_default_port_with_userinfo():
42+
"""Default port stripping must preserve userinfo in the netloc."""
43+
assert resource_url_from_server_url("https://user:pass@example.com:443/mcp") == "https://user:pass@example.com/mcp"
44+
assert resource_url_from_server_url("http://user@example.com:80/api") == "http://user@example.com/api"
45+
46+
47+
def test_resource_url_from_server_url_strips_default_port_with_ipv6():
48+
"""Default port stripping must preserve IPv6 literal brackets."""
49+
assert resource_url_from_server_url("https://[::1]:443/path") == "https://[::1]/path"
50+
assert resource_url_from_server_url("http://[2001:db8::1]:80/api") == "http://[2001:db8::1]/api"
51+
52+
53+
def test_resource_url_from_server_url_malformed_port_falls_back():
54+
"""A malformed explicit port must not raise; it should fall back to the original netloc."""
55+
assert resource_url_from_server_url("https://example.com:99999999/path") == "https://example.com:99999999/path"
56+
57+
4158
def test_resource_url_from_server_url_lowercase_scheme_and_host():
4259
"""Scheme and host should be lowercase for canonical form."""
4360
assert resource_url_from_server_url("HTTPS://EXAMPLE.COM/path") == "https://example.com/path"
@@ -125,6 +142,11 @@ def test_check_resource_allowed_case_insensitive_origin():
125142
assert check_resource_allowed("https://Example.Com:8080/api", "https://example.com:8080/api") is True
126143

127144

145+
def test_check_resource_allowed_malformed_port_does_not_raise():
146+
"""A malformed explicit port must not raise; check_resource_allowed should return a bool."""
147+
assert check_resource_allowed("https://example.com:99999999/path", "https://example.com/path") is False
148+
149+
128150
def test_check_resource_allowed_empty_paths():
129151
"""Empty paths should be handled correctly."""
130152
assert check_resource_allowed("https://example.com", "https://example.com") is True

0 commit comments

Comments
 (0)