Skip to content

fix(transport): match explicit default ports in Origin allowlist - #1270

Open
DaleSeo wants to merge 1 commit into
mainfrom
DaleSeo/explicit-default-port-origin-entries-reject-brow
Open

DaleSeo wants to merge 1 commit into
mainfrom
DaleSeo/explicit-default-port-origin-entries-reject-brow

Conversation

@DaleSeo

@DaleSeo DaleSeo commented Sep 14, 2026

Copy link
Copy Markdown
Member

Fixes #1268.

Motivation and Context

An allowed_origins entry with an explicit default port, like https://client.example:443, rejected the origin that browsers actually send, which is Origin: https://client.example. So the entry did not work in practice.

RFC 6454 treats an omitted port as the scheme's default port, and then removes that default port when serializing the origin. The matcher compared the missing port from the incoming request directly with the configured port. That meant a configured :443 could only match a client that explicitly included the port.

The fix resolves the effective port on the incoming origin before comparing it. That means 443 for https and 80 for http. The configured side stays unchanged.

How Has This Been Tested?

Add tests

Breaking Changes

None

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@github-actions github-actions Bot added T-test Testing related changes T-core Core library changes T-transport Transport layer changes labels Sep 14, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review September 14, 2026 22:58
@DaleSeo
DaleSeo requested a review from a team as a code owner September 14, 2026 22:58
@DaleSeo DaleSeo self-assigned this Sep 14, 2026
@DaleSeo
DaleSeo force-pushed the DaleSeo/explicit-default-port-origin-entries-reject-brow branch from 15b9894 to 4e2a6b6 Compare September 15, 2026 15:20
/// an `Origin` header must match per RFC 6454 `(scheme, host, port)`;
/// missing-`Origin` requests still pass. Entries must include a scheme;
/// missing-`Origin` requests still pass. An entry that omits the port
/// permits any port; an entry with an explicit port matches only that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it really permit any port, or if it's not listed should it permit only the default port for the scheme?

People can always configure it with a specific known port if they need one...

If we need the unconditional match, perhaps it would be better to introduce a syntax for specifying that like:

http://localhost:*

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

2 participants