Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null value of Access-Control-Allow-Origin #975

Open
ToyB0x opened this issue Nov 28, 2023 · 2 comments
Open

Null value of Access-Control-Allow-Origin #975

ToyB0x opened this issue Nov 28, 2023 · 2 comments

Comments

@ToyB0x
Copy link

ToyB0x commented Nov 28, 2023

Describe the bug

It seems that MDN states that null should not be used for the value of Access-Control-Allow-Origin, but looking at the implementation below, it seems that null is returned. (See also 7.4. Avoid returning Access-Control-Allow-Origin: "null")

} else {
// There is no origin found in the headers, so we should return null
headers['Access-Control-Allow-Origin'] = 'null';
}

To Reproduce Steps to reproduce the behavior:

Run the existing test below.

it('should return null if the sent origin does not match', () => {
const request = new Request('http://localhost:4002/graphql', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
origin: 'http://localhost:4002',
},
});
const headers = getCORSHeadersByRequestAndOptions(request, corsOptionsWithMultipleOrigins);
expect(headers?.['Access-Control-Allow-Origin']).toBe('null');
});
});

Expected behavior

A server can respond to requests from disallowed origins without including Access-Control-Allow-Origin header.
In this case, the browser blocks the request based on the Same-Origin Policy.

Environment:

  • @whatwg-node/server: 0.9.18
@ardatan
Copy link
Owner

ardatan commented Nov 28, 2023

The idea there to let the browser know the server doesn't accept that origin. Let us know if you have better idea for this behavior. And I also see it is not restricted to return null.
We really need e2e browser tests to confirm browser rejects the request if that header is not present.

@ToyB0x
Copy link
Author

ToyB0x commented Nov 28, 2023

Looking at the expressjs/cors code, it seems to be testing the absence of a CORS-related header.
Therefore, I don't think it's a bad idea to not return the CORS-related Header itself instead of null.

I agree that it is safer to add browser E2E testing.

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

No branches or pull requests

2 participants