Skip to content

[Server] Extract CORS handling into CorsMiddleware#277

Open
chr-hertel wants to merge 2 commits intomainfrom
refactor/cors-middleware
Open

[Server] Extract CORS handling into CorsMiddleware#277
chr-hertel wants to merge 2 commits intomainfrom
refactor/cors-middleware

Conversation

@chr-hertel
Copy link
Copy Markdown
Member

@chr-hertel chr-hertel commented Apr 11, 2026

Summary

  • Extracts inline CORS handling from StreamableHttpTransport into a dedicated CorsMiddleware (PSR-15)
  • CorsMiddleware is automatically prepended to the middleware chain, handling OPTIONS preflight and applying CORS headers to all responses
  • Changes default Access-Control-Allow-Origin from * (allow all) to not set (block cross-origin), with explicit opt-in via allowedOrigins

Breaking Changes

  • The $corsHeaders constructor parameter on StreamableHttpTransport is replaced by ?CorsMiddleware $corsMiddleware
  • Default CORS behavior no longer sets Access-Control-Allow-Origin: * — use new CorsMiddleware(allowedOrigins: ['*']) to restore

@chr-hertel chr-hertel added this to the 0.5.0 milestone Apr 11, 2026
@chr-hertel chr-hertel changed the title Extract CORS handling into CorsMiddleware [Server] Extract CORS handling into CorsMiddleware Apr 11, 2026
@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Apr 11, 2026
@chr-hertel chr-hertel force-pushed the refactor/cors-middleware branch 5 times, most recently from 2812af8 to 2ed3d9c Compare April 11, 2026 14:25
@chr-hertel chr-hertel marked this pull request as ready for review April 11, 2026 14:27
@chr-hertel chr-hertel added the breaking change Breaking the Backwards Compatibility Promise label Apr 11, 2026
@CodeWithKyrian
Copy link
Copy Markdown
Contributor

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:

  • accept CorsMiddleware as part of the middleware list
  • detect if it’s already present
  • ensure it’s in the correct position (e.g. prepend or reorder if needed)
  • optionally inject a default if none is provided

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?

@chr-hertel
Copy link
Copy Markdown
Member Author

@CodeWithKyrian Thanks, yeah, true - so similar to the approach in #260, right?

@CodeWithKyrian
Copy link
Copy Markdown
Contributor

Yes exactly!

@chr-hertel chr-hertel force-pushed the refactor/cors-middleware branch from 2ed3d9c to d4bb9f4 Compare April 11, 2026 16:05
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@chr-hertel chr-hertel force-pushed the refactor/cors-middleware branch from d4bb9f4 to e69f03c Compare April 11, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Breaking the Backwards Compatibility Promise Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hardcoded Wildcard CORS (Access-Control-Allow-Origin: *)

2 participants