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

fix(cors) send '*' in ACAO when 'conf.origins' is empty #2518

Merged
merged 1 commit into from
May 13, 2017

Conversation

thibaultcha
Copy link
Member

Summary

This fixes a regression introduced in 0.10.1, where the migration would
set the new conf.origins field to an empty table, but the code would
only set * if the table was nil, and not empty.

We are reluctant to update the migration due to the immutable nature
that they should carry (in order to avoid divergence between users who
already ran them, and users who will run it after this patch), so this
changes the business logic to be more robust instead, which we probably
want anyways.

This regression was documented post-release, in #2517.

Full changelog

  • handle empty config.origins in the CORS ACAO business logic
  • regression test

Relates to #2517

This fixes a regression introduced in 0.10.1, where the migration would
set the new `conf.origins` field to an empty table, but the code would
only set ACAO to `*` if the table was `nil`, and not empty.

We are reluctant to update the migration due to the immutable nature
that they should carry (in order to avoid divergence between users who
already ran them, and users who will run it after this patch), so this
changes the business logic to be more robust instead, which we probably
want anyways.

This regression was documented post-release, in #2517.
@thibaultcha
Copy link
Member Author

@p0pr0ck5 Mind taking a look at this? Thanks!

@@ -138,6 +151,28 @@ describe("Plugin: cors (access)", function()
assert.is_nil(res.headers["Access-Control-Max-Age"])
end)

it("gives * wildcard when origins is empty", function()
-- this test covers a regression introduced in 0.10.1, where
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 thank you for this comment!

@thibaultcha thibaultcha merged commit 7d0095e into master May 13, 2017
@thibaultcha thibaultcha deleted the fix/cors-default-behavior branch May 13, 2017 00:23
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.

2 participants