Skip to content

fix(streamable-http): match Origin's omitted default port against explicit allowlist entries - #1282

Open
yinnho wants to merge 2 commits into
modelcontextprotocol:mainfrom
yinnho:fix/origin-default-port-matching
Open

yinnho wants to merge 2 commits into
modelcontextprotocol:mainfrom
yinnho:fix/origin-default-port-matching

Conversation

@yinnho

@yinnho yinnho commented Sep 17, 2026

Copy link
Copy Markdown

Fixes #1268.

Problem

origin_is_allowed compares the configured entry's port against the incoming origin's port as raw Option<u16>:

Rust a_port.is_none() || a_port == o_port

Browsers omit the port from the serialized Origin header when it equals the scheme default (RFC 6454 §6.2), so a portless incoming origin arrives as None. An explicitly configured https://example.com:443 therefore never matches a browser-sent https://example.com → 403, even though both denote the same effective port. This bites exactly the deployments that do the more careful thing and pin the port explicitly.

Fix

Resolve the incoming origin's effective port before comparing (RFC 6454 §4 — omitted port = scheme default: 443 for https/wss, 80 for http/ws):

Rust a_port.is_none() || a_port == &effective_origin_port(*o_port, o_scheme)

The comparison stays per-scheme: an https://…:443 entry still rejects http://… (whose implicit port is 80).

Deliberately unchanged: an entry with an omitted port keeps its any-port wildcard (configures scheme+host, any port). #1191 / #1192 left that behavior in place and downstream deployments (e.g. spiceai's runtime, which lists origins portless behind TLS terminators) rely on it. The allowed_origins rustdoc now documents both behaviors so the distinction is discoverable.

Behavior matrix

allowlist entry incoming Origin before after
https://example.com:443 https://example.com 403 ❌ 200 ✅
https://example.com:443 https://example.com:443 200 200
https://example.com:443 https://example.com:8443 403 403
https://example.com:443 http://example.com 403 403 (per-scheme resolution)
https://example.com (no port) https://example.com:8443 200 200 (wildcard preserved)
http://example.com:80 http://example.com 403 ❌ 200 ✅

Tests

Six new cases in origin_validation (test_custom_headers.rs) covering the matrix above. Verified red→green: the key test fails on the unfixed predicate and passes with the fix; all 35 tests in the file pass. cargo fmt --check and clippy clean on the touched files.

🤖 Generated with Claude Code

…licit allowlist entries

Browsers omit the port from the serialized Origin header when it equals
the scheme default (RFC 6454 §6.2), so a deployment that configures
allowed_origins = ["https://example.com:443"] never matches the browser-sent
"https://example.com" — the raw Option<u16> comparison sees Some(443) vs
None and rejects with 403.

Resolve the incoming origin's effective port (443 for https/wss, 80 for
http/ws) before comparing, so an explicitly configured port accepts both
spellings of the same effective port while still rejecting genuinely
different ports (8443 etc). The omitted-port entry form keeps its
any-port wildcard semantics, which deployments behind multiple TLS
terminators rely on; the rustdoc now states both behaviors.

Fixes modelcontextprotocol#1268

Co-Authored-By: Claude <noreply@anthropic.com>
@yinnho
yinnho requested a review from a team as a code owner September 17, 2026 13:25
@github-actions github-actions Bot added T-test Testing related changes T-core Core library changes T-transport Transport layer changes labels Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-test Testing related changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explicit default-port Origin entries reject browser-serialized origins

1 participant