Skip to content

fix(binding-http): include CORS headers for 404 responses#1496

Open
Pranav-0440 wants to merge 1 commit intoeclipse-thingweb:masterfrom
Pranav-0440:improve/cors-edge-cases
Open

fix(binding-http): include CORS headers for 404 responses#1496
Pranav-0440 wants to merge 1 commit intoeclipse-thingweb:masterfrom
Pranav-0440:improve/cors-edge-cases

Conversation

@Pranav-0440
Copy link

Closes #1495

Description

This PR ensures that 404 responses include Access-Control-Allow-Origin
when an Origin header is present in the request.

Previously, unknown routes returned 404 without CORS headers,
causing browsers to block cross-origin error responses.

Changes

  • Added a regression test to verify CORS behavior for 404 responses
  • Updated defaultRoute in http-server.ts to include:
    • Access-Control-Allow-Origin
    • Vary: Origin

Motivation

Extends the CORS coverage introduced in #1486 and improves
standards compliance by ensuring consistent CORS behavior
for all HTTP responses, including 404 errors.

Copilot AI review requested due to automatic review settings February 21, 2026 19:03
Copy link

Copilot AI left a comment

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 addresses issue #1495 by adding CORS headers to 404 responses returned by the defaultRoute handler. When an Origin header is present in a request to an unknown route, the server now reflects the origin in the Access-Control-Allow-Origin response header along with a Vary: Origin header.

Changes:

  • Updated defaultRoute in http-server.ts to include CORS headers (Access-Control-Allow-Origin and Vary: Origin) when an Origin header is present
  • Added two tests to http-server-cors-test.ts: one attempting to verify 404 CORS behavior and another testing preflight with custom headers

Reviewed changes

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

File Description
packages/binding-http/src/http-server.ts Added CORS header logic to defaultRoute for 404 responses
packages/binding-http/test/http-server-cors-test.ts Added two tests, one for 404 CORS (though implementation is flawed) and one for preflight with custom headers

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>
@Pranav-0440 Pranav-0440 force-pushed the improve/cors-edge-cases branch from e996fa1 to e5585ec Compare March 1, 2026 14:13
@egekorkan
Copy link
Member

Coming from me and @danielpeintner: @Pranav-0440 we are not able to review this many PRs (more than 10 across here and EdiTDor). Please focus on less and make sure you understand the codebase. We will prefer less but higher quality PRs over too many PRs for our decision process. Using AI tooling is fine but you still need to understand what you are changing

@Pranav-0440
Copy link
Author

Thanks @egekorkan for the feedback, that makes sense.
I’ll focus on fewer PRs and make sure I understand the codebase more thoroughly before opening new ones. I’ll prioritize improving the existing PRs rather than submitting many changes at once.

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.

binding-http: 404 responses do not include CORS headers

4 participants