[Server] Extract CORS handling into CorsMiddleware#277
Conversation
chr-hertel
changed the title
chr-hertel
force-pushed
the
refactor/cors-middleware
branch
5 times, most recently
from
2812af8 to
2ed3d9c
Compare
chr-hertel
requested review from
CodeWithKyrian,
Nyholm and
soyuka
as code owners
chr-hertel
added
the
breaking change
|
This is a solid refactor...makes a lot of sense. One thing I was wondering though: instead of introducing a dedicated $corsMiddleware parameter on the transport, would it be feasible to just work with the existing $middleware array? For example:
That way everything stays within a single middleware pipeline instead of splitting configuration paths. I get that this might complicate the setup a bit, so maybe that’s why you went this route...but I was just curious if you explored that approach at all? |
|
@CodeWithKyrian Thanks, yeah, true - so similar to the approach in #260, right? |
|
Yes exactly! |
chr-hertel
force-pushed
the
refactor/cors-middleware
branch
2 times, most recently
from
d4bb9f4 to
e69f03c
Compare
|
|
||
| if (!$hasCorsMiddleware) { | ||
| array_unshift($this->middleware, new CorsMiddleware()); | ||
| } |
There was a problem hiding this comment.
let's say I use the mcp bundle with the famous https://github.com/nelmio/NelmioCorsBundle how am I going to avoid conflicts?
There was a problem hiding this comment.
Haven't tested, but we'd need a way to opt-out completely you mean?
There was a problem hiding this comment.
yes can we just remove these checks? we add it by default in our server in here but lets not add it if not found as it prevents other libraries to do their own cors handling.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
chr-hertel
force-pushed
the
refactor/cors-middleware
branch
from
8d26c5c to
a19723e
Compare
Introduce three PSR-15 middleware for `StreamableHttpTransport` exposed through a public `StreamableHttpTransport::defaultMiddleware()` factory composed automatically when no middleware is passed. - `CorsMiddleware`: secure-by-default (no `Access-Control-Allow-Origin`), configurable allowlist, reflects matching origin with `Vary: Origin` to protect shared caches. - `DnsRebindingProtectionMiddleware`: validates `Origin`/`Host` against a hostname allowlist (localhost-only by default). - `ProtocolVersionMiddleware`: rejects requests carrying an unsupported `Mcp-Protocol-Version` header with `400 Bad Request`. The transport no longer applies CORS via an `instanceof + array_unshift` post-hook; the middleware parameter is nullable — `null` installs the secure defaults, `[]` disables them, and users compose by spreading `StreamableHttpTransport::defaultMiddleware()`. `SESSION_HEADER` and `PROTOCOL_VERSION_HEADER` are promoted to public constants so middleware can reuse them. BC breaks: - The `corsHeaders` constructor parameter is removed; the `middleware` parameter shifts one position. Positional callers passing the old `corsHeaders` argument must switch to named arguments or drop it. - Default `Access-Control-Allow-Origin` is no longer `*`. Addresses modelcontextprotocol#260 (DNS rebinding), modelcontextprotocol#277 (CORS extraction) and modelcontextprotocol#306 (protocol version validation).
3 participants
Summary
StreamableHttpTransportinto a dedicatedCorsMiddleware(PSR-15)CorsMiddlewareis automatically prepended to the middleware chain, handling OPTIONS preflight and applying CORS headers to all responsesAccess-Control-Allow-Originfrom*(allow all) to not set (block cross-origin), with explicit opt-in viaallowedOriginsBreaking Changes
$corsHeadersconstructor parameter onStreamableHttpTransportis replaced by?CorsMiddleware $corsMiddlewareAccess-Control-Allow-Origin: *— usenew CorsMiddleware(allowedOrigins: ['*'])to restore