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

Allow specifying multiple comma-separated values for CORS header #5626

Merged
merged 1 commit into from
May 8, 2024

Conversation

Martchus
Copy link
Contributor

@Martchus Martchus commented May 8, 2024

The Access-Control-Allow-Origin only allows specifying one concrete origin or the wildcard *. So in order to support multiple allowed origins we must split the header value server-side and return the header value that matches the requests origin (if allowed). This is also what is suggested in the last paragraph of
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin#examples.

This change also only appends the additional value of the Vary header (which is set in accordance with
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin#cors_and_caching) to avoid completely overriding it (e.g. preserving the Mojolicious default).

I tested this locally configuring multiple or just one allowed origins and verifying via the developer tools that the headers are returned correctly.

Related ticket: https://progress.opensuse.org/issues/159384

The `Access-Control-Allow-Origin` only allows specifying one concrete
origin or the wildcard `*`. So in order to support multiple allowed origins
we must split the header value server-side and return the header value that
matches the requests origin (if allowed). This is also what is suggested in
the last paragraph of
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin#examples.

This change also only appends the additional value of the Vary header
(which is set in accordance with
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin#cors_and_caching)
to avoid completely overriding it (e.g. preserving the Mojolicious
default).

I tested this locally configuring multiple or just one allowed origins and
verifying via the developer tools that the headers are returned correctly.

Related ticket: https://progress.opensuse.org/issues/159384
Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (master@f9fe556). Click here to learn what that means.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #5626   +/-   ##
=========================================
  Coverage          ?   98.40%           
=========================================
  Files             ?      393           
  Lines             ?    38317           
  Branches          ?        0           
=========================================
  Hits              ?    37707           
  Misses            ?      610           
  Partials          ?        0           

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

@mergify mergify bot merged commit e7d8f00 into os-autoinst:master May 8, 2024
42 checks passed
@Martchus Martchus deleted the cors branch May 8, 2024 14:02
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.

None yet

3 participants