Skip to content

resource_url_from_server_url preserves explicit default ports #3297

Open

Description

@otiscuilei

Initial checks

  • I tested the current main branch (6e30452).
  • I searched existing issues and pull requests for this behavior.

Description

resource_url_from_server_url() lowercases the scheme/authority and removes the fragment, but it preserves an explicitly specified default port. This makes an RFC 8707 resource URL such as https://example.com:443/mcp compare unequal to the equivalent https://example.com/mcp value used by Protected Resource Metadata.

RFC 3986 section 6.2.3 treats an explicit default port as equivalent to an omitted port. Pydantic also normalizes the metadata URL by dropping the default port. The SDK currently canonicalizes only one side, so check_resource_allowed() can reject an otherwise equivalent resource and abort OAuth resource validation.

Expected: HTTP port 80 and HTTPS port 443 are removed during canonicalization; non-default ports remain unchanged.

Example code

from mcp.shared.auth_utils import check_resource_allowed, resource_url_from_server_url

canonical = resource_url_from_server_url("https://example.com:443/mcp")
print(canonical)  # current: https://example.com:443/mcp
print(check_resource_allowed(canonical, "https://example.com/mcp"))  # current: False

Environment

  • Python 3.12
  • MCP Python SDK main at 6e30452

I used AI assistance while investigating this issue and reviewed and reproduced the behavior locally.

Activity

  1. added
    P3Nice to haves, rare edge cases
    on Aug 12, 2026
  2. Ben-Home commented on Aug 14, 2026

    @Ben-Home
  3. added
    v1Affects the v1.x maintenance line
    v2Affects the v2 line (2.x on main)
    on Aug 18, 2026
  4. tao943 commented on Aug 20, 2026

    @tao943

    I reproduced the behavior and reviewed the two existing implementations in #3299 and #3310, so I will not open a third duplicate PR.

    One scope question remains before any additional work: this issue is labeled v1 and v2, but the contribution guide limits v1.x to security and critical bug fixes while this issue is P3. Should the intended fix target main only?

    Whichever existing implementation is selected, I suggest keeping the regression matrix focused on:

    • stripping only HTTP :80 and HTTPS :443;
    • preserving non-default and malformed ports;
    • preserving userinfo, bracketed IPv6, path, and query components;
    • retaining the existing fragment-removal behavior.

    #3299 is narrower and already covers IPv6 and malformed authorities; #3310 additionally canonicalizes both inputs in check_resource_allowed(), which may be useful but is a broader semantic change than the reported helper bug.

    AI-assisted investigation disclosed; I reviewed the issue, contribution policy, both diffs, and their CI results and remain accountable for this analysis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Nice to haves, rare edge casesbugSomething isn't workingv1Affects the v1.x maintenance linev2Affects the v2 line (2.x on main)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions