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

Vary: Origin logic is incorrect #1251

Open
ehmicky opened this issue Oct 18, 2024 · 0 comments
Open

Vary: Origin logic is incorrect #1251

ehmicky opened this issue Oct 18, 2024 · 0 comments
Labels

Comments

@ehmicky
Copy link

ehmicky commented Oct 18, 2024

Describe the bug
By default, @middy/http-cors sets the Vary: Origin header, which is good.

However, it only does so based on whether the Access-Control-Allow-Origin request header has been set and is not *. This condition does not quite capture whether the Origin request header has been used to compute the response.

let vary = options.vary
if (
headers['Access-Control-Allow-Origin'] &&
headers['Access-Control-Allow-Origin'] !== '*' &&
!vary
) {
vary = 'Origin'
}
if (vary && !existingHeaders.includes('Vary')) {
headers.Vary = vary
}

Expected behaviour
The Vary: Origin response header should be set if and only if the CORS response (preflight or not) might change depending on the Origin request header. That's because the Origin will become part of the cache key used by any reverse proxy. See the standard and the RFC. This includes when then Origin request header is missing (i.e. non-CORS request), as mentioned in the standard and as explained in this blog post.

In the case of @middy/http-cors, the Origin request header is only used to compute (or not) Access-Control-Allow-Origin. This means:

  • If the getOrigin option is set, then Vary: Origin should be set since we don't know whether the Origin request header is used by the custom function or not (and it probably is).
  • Else, if either the origin option is * or the origins option is an array containing *, then:
    • If the credentials option is false (its default value): Vary: Origin should not be set since Allow-Control-Allow-Origin will always be *.
    • Else, Vary: Origin should be set since Allow-Control-Allow-Origin will reflect the Origin request header.
  • Else, if both the origin option is not * and the origins option is an empty array (the default value), then Vary: Origin should not be set since the origin option will be used regardless of the Origin request header.

Additional context

Note: if the Vary response header is already set, this middleware will keep it as is, instead of concatenating its value with commas. See the code used by Express for this.

@ehmicky ehmicky added the bug label Oct 18, 2024
@ehmicky ehmicky changed the title Vary: Origin should be set even without Access-Control-Allow-Origin Vary: Origin logic is incorrect Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

1 participant