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

CORS Headers Added to Pre-Flight Request on Ambiguous Match Even Without @CrossOrigin #26490

Closed
AlexErmer opened this issue Feb 1, 2021 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@AlexErmer
Copy link

AlexErmer commented Feb 1, 2021

By default CORS support is disabled but in case of multipe valid method mappings, Spring just adds CORS Headers to preflight requests with allowed origin *!

In AbstractHandlerMethodMapping.lookupHandlerMethod():410 there is a check, whether a mapping has more than one matches. If its a perfect match, everything is fine here.
But in the case, that there a multiple mappings EVERY preflight requests will be handled by PREFLIGHT_AMBIGUOUS_MATCH.

		if (!matches.isEmpty()) {
			Match bestMatch = matches.get(0);
			if (matches.size() > 1) {
				Comparator<Match> comparator = new MatchComparator(getMappingComparator(request));
				matches.sort(comparator);
				bestMatch = matches.get(0);
				if (logger.isTraceEnabled()) {
					logger.trace(matches.size() + " matching mappings: " + matches);
				}
				if (CorsUtils.isPreFlightRequest(request)) {
					return PREFLIGHT_AMBIGUOUS_MATCH;
				}
[..]

Just a few lines later, the cors configurations will be gathered and for this special handler, the CORS-configuration will allow everything to everyone.

You can simply check this behavior by making a brand new Spring Boot application with Spring Web without any changes. Just send an OPTIONS request to /error. BasicErrorController has two mappings for this path and voila.

curl -v -H "Origin: http://any.origin" -H "Access-Control-Request-Method: GET" -X OPTIONS http://localhost:8080/error
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8080 (#0)
> OPTIONS /error HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.55.1
> Accept: */*
> Origin: http://any.origin
> Access-Control-Request-Method: GET
>
< HTTP/1.1 200
< Vary: Origin
< Vary: Access-Control-Request-Method
< Vary: Access-Control-Request-Headers
< Access-Control-Allow-Origin: http://any.origin
< Access-Control-Allow-Methods: GET
< Access-Control-Allow-Credentials: true
< Allow: GET, HEAD, POST, PUT, DELETE, OPTIONS, PATCH
< Content-Length: 0
< Date: Mon, 01 Feb 2021 22:46:36 GMT
<
* Connection #0 to host localhost left intact

All in all: I don't want my app to respond with CORS-headers at all, as I do not have any CORS configuration, but unfortunately Spring just adds those headers to my responses.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 1, 2021
@rstoyanchev rstoyanchev self-assigned this Feb 2, 2021
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 2, 2021
@rstoyanchev rstoyanchev added this to the 5.3.4 milestone Feb 2, 2021
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Feb 2, 2021

A pre-flight request is mapped based on information in CORS headers about the would-be, actual request. However those headers describe only a subset of what @RequestMapping can map on, and therefore in case of multiple matches we can't rely on the order in which they are sorted. Essentially we don't know which will match the exact actual request.

I can refine the behavior so that for 0 matches with CORS config, we don't return PREFLIGHT_AMBIGUOUS_MATCH but the actual top match. The sort order doesn't matter in this case, we just need to return 403. However if there is 1 or more matches with CORS config, then the same behavior as currently has to apply because we can't decide until the actual request.

Does that sound alright?

@AlexErmer
Copy link
Author

Thanks for fast investigation. That sounds alright for me.
No CORS config leads to no PREFLIGHT_AMBIGUOUS_MATCH and therefore no CORS headers. That solves my issue.

I get the point where you can not choose the "right" one and the current behavior must apply.

@rstoyanchev
Copy link
Contributor

No CORS config leads to no PREFLIGHT_AMBIGUOUS_MATCH and therefore no CORS headers.

Not quite. In the absence of CORS configuration, it should result in a rejection.

@AlexErmer
Copy link
Author

Thats exactly what I am looking for. I am totally fine with 403. Thanks

@rstoyanchev rstoyanchev changed the title CORS Header added with CORS generally disabled CORS Headers Added to Pre-Flight Request on Ambiguous Match Even Without @CrossOrigin Feb 3, 2021
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: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants