Skip to content

Fix CORS allow-all middleware - #27

Open
samdark wants to merge 2 commits into
masterfrom
fix-cors-allow-all
Open

Fix CORS allow-all middleware#27
samdark wants to merge 2 commits into
masterfrom
fix-cors-allow-all

Conversation

@samdark

@samdark samdark commented Aug 6, 2026

Copy link
Copy Markdown
Member

Fixes #24.

  • reflect the request origin when credentials are enabled instead of combining credentials with wildcard values
  • echo requested preflight headers and explicitly expose downstream response headers
  • remove the unrelated Allow header
  • optionally short-circuit valid preflight requests with a PSR-17 response factory while keeping the zero-argument constructor backward compatible
  • document the optional response factory and cover both compatibility paths

Checks:

  • PHPUnit: 111 tests, 206 assertions
  • Psalm
  • PHP CS Fixer
  • Rector
  • Infection: 100% covered MSI for CorsAllowAllMiddleware

Copilot AI lite review requested due to automatic review settings August 6, 2026 22:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes CorsAllowAllMiddleware to produce CORS headers that comply with the Fetch/CORS protocol (especially when credentials are involved) and adds optional preflight short-circuiting via a PSR-17 response factory, with accompanying docs, tests, and changelog entry.

Changes:

  • Reflect request origin when credentials are enabled and avoid invalid Allow-Origin: * + Allow-Credentials: true combinations.
  • Add optional preflight (OPTIONS + Access-Control-Request-Method) short-circuiting with a PSR-17 ResponseFactoryInterface.
  • Update documentation, changelog, and tests for both constructor compatibility paths.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/CorsAllowAllMiddleware.php Updates CORS header logic and introduces optional preflight short-circuiting via PSR-17 response factory.
tests/CorsAllowAllMiddlewareTest.php Adds tests for credentialed requests, preflight behavior, and constructor compatibility.
docs/guide/en/cors-allow-all-middleware.md Documents the optional response factory for preflight short-circuiting.
CHANGELOG.md Records the bugfix and preflight-handling change under the upcoming release.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/CorsAllowAllMiddleware.php
Comment thread tests/CorsAllowAllMiddlewareTest.php
Comment thread src/CorsAllowAllMiddleware.php Outdated
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (05d8cc7) to head (ef22283).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##              master       #27   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity       100       112   +12     
===========================================
  Files             23        23           
  Lines            253       274   +21     
===========================================
+ Hits             253       274   +21     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samdark
samdark requested review from a team and a lite review from Copilot August 7, 2026 08:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/CorsAllowAllMiddleware.php:52

  • $exposedHeaders is computed for every request, but it’s only used when a single Origin is present. For the common $origin === '' path (no/invalid Origin), this loop does unnecessary work before returning wildcard headers.
        $exposedHeaders = [];
        /** @var array<string, string[]> $headers */
        $headers = $response->getHeaders();
        foreach ($headers as $name => $_) {
            if (strtolower($name) !== 'set-cookie') {

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CorsAllowAllMiddleware: invalid CORS headers and missing preflight handling

3 participants