Skip to content

Improve support for port numbers in allowedOriginPattern of CorsConfiguration #26927

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

Closed

Conversation

korektur
Copy link
Contributor

This is to support ports in cors origin patterns.
Closes gh-26926

Example:

https://*.example.com:[8080]
https://*.example.com:[8080,9000]
https://*.example.com:[*]

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 10, 2021
Copy link

@cuspymd cuspymd left a comment

Choose a reason for hiding this comment

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

It would be nice if a description of the added patterns was added to the document of the setAllowedOriginPatterns() method.

Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. This looks good overall.

One observation. It looks like I can now match an implicit port (e.g. "https://abc.org") or any port (including an implicit port) with "*", but if I want to match both implicit and specific others, then I need two patterns. I suppose if one defines "https://abc.org:[8080,8081]" it's impossible to know just by looking whether it matches the implicit port or not, but it's probably best to err on the side of allowing less.

In summary I agree with the way you've done it. However, we'll need to make sure the syntax and these nuances are covered well in CorsConfiguration#setAllowedOriginPatterns. Let me know if you intend to do that or otherwise I can take it from there.

@korektur
Copy link
Contributor Author

@rstoyanchev sure, I can add some documentation on that.

@korektur
Copy link
Contributor Author

@rstoyanchev I've added some documentation, can you review please?

@korektur korektur requested a review from rstoyanchev May 16, 2021 15:55
@rstoyanchev rstoyanchev self-assigned this May 17, 2021
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 17, 2021
@rstoyanchev rstoyanchev added this to the 5.3.8 milestone May 17, 2021
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve support for port numbers in allowedOriginPattern of CorsConfiguration
4 participants