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 CorsSupport default behavior #1714

Merged
merged 13 commits into from
May 5, 2020
Merged

Fix CorsSupport default behavior #1714

merged 13 commits into from
May 5, 2020

Conversation

tjquinno
Copy link
Member

@tjquinno tjquinno commented May 1, 2020

Resolves #1713

There are two aspects to this.

A. If the developer creates a CorsSupport instance it will provide the default (wild carded) behavior if no specific settings were provided. In particular, passing a missing config node to CorsSupport.create(Config) creates a new CorsSupport instance with the default wildcarding behavior; CORS also logs an INFO message to this effect in case the developer actually expected the config node to contain something.

There is a new CrossOriginConfig.create() method which developers can use to include the default wild carded behavior as they build a CorsSupport instance.

(I refactored many methods from Aggregator to its new Builder inner class so the diffs look deceptively large.)

B. All cross-origin info added to a CorsSupport instance is now searched sequentially in order of addition to the builder. The Aggregator which collects the cross-origin information (basically as CrossOriginConfig instances) used to use a map, keyed by the path expression. This could lead to confusion if the developer provided multiple CrossOriginConfig objects with the same path (or with no path which is essentially the same thing) because later additions would overwrite earlier ones.

This PR changes the internal data structure to a List and maintains the order in which the developer supplies the CrossOriginConfig information. When requests arrive, the Helidon CORS implementation scans the list in order-of-addition and uses the first that matches the request's path and HTTP method.

This gives the developer complete control by making sure the order in which the application adds CrossOriginConfigs matches the needs of the application, without the Helidon CORS code imposing a "one-setting-per-path" restriction.

…default behavior

Signed-off-by: tim.quinn@oracle.com <tim.quinn@oracle.com>
@tjquinno tjquinno requested a review from spericas May 1, 2020 20:54
@tjquinno tjquinno self-assigned this May 1, 2020
@tjquinno tjquinno changed the title Fix CorsSupport default behavior WIP - Fix CorsSupport default behavior May 2, 2020
…ive developers more control over the routing they build; developers need to add CrossOriginConfigs in the order they should be checked
@tjquinno tjquinno changed the title WIP - Fix CorsSupport default behavior Fix CorsSupport default behavior May 2, 2020
@tjquinno tjquinno changed the title Fix CorsSupport default behavior WIP - Fix CorsSupport default behavior May 4, 2020
@tjquinno tjquinno added 2.0-M3 bug Something isn't working labels May 4, 2020
@tjquinno tjquinno changed the title WIP - Fix CorsSupport default behavior Fix CorsSupport default behavior May 5, 2020
…e defensive about presence or absence of leading / in MP request path
Copy link
Member

@tomas-langer tomas-langer left a comment

Choose a reason for hiding this comment

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

LGTM

@tjquinno tjquinno merged commit b842c91 into helidon-io:master May 5, 2020
@tjquinno tjquinno deleted the cors-fix-default branch May 5, 2020 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CORS default CorsSupport instance does not have expected wildcard behavior
2 participants