Use @PreMatching for CORS Preflight request #11097
Conversation
| return Optional.of(HttpStatus.FORBIDDEN); | ||
| if (hasAllowedHeaders(request, config)) { | ||
| List<UriRouteMatch<Object, Object>> anyUriRoutes = router.findAny(request); | ||
| if (anyUriRoutes.isEmpty()) { |
There was a problem hiding this comment.
This is how it worked before, but I think that's not correct. methodToMatchOptional is OPTIONS for preflight check and there is no such method in mocked tests
There was a problem hiding this comment.
methodToMatchOptional can be not OPTIONS if there's a Access-Control-Request-Method header
There was a problem hiding this comment.
Shouldt'n we fail OPTIONS call if there is no route present under that path?
There was a problem hiding this comment.
No, in that case the preflight should return a good response and the actual cors request should return the 404. This will allow the cors client to see that the route is missing. Otherwise it will just get a permission error
|
seems to be failing? |
|
|
||
| then: | ||
| HttpStatus.OK == response.status() | ||
| HttpStatus.FORBIDDEN == response.status() |
# Conflicts: # http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy
| @@ -37,6 +37,12 @@ class OptionsRequestAttributesSpec extends Specification { | |||
| HttpClientResponseException e = thrown() | |||
| e.response.status == HttpStatus.METHOD_NOT_ALLOWED | |||
|
|
|||
| and: | |||
| MyFilter myFilter = ctx.getBean(MyFilter) | |||
| !myFilter.containsRouteInfo | |||
There was a problem hiding this comment.
@dstepanov I had to this change as well.
|
@dstepanov I have rewritten the test and the filter slightly. I think it is now correct. @yawkat can you check it out again. |
...ver-netty/src/test/groovy/io/micronaut/http/server/netty/OptionsRequestAttributesSpec.groovy
Show resolved
Hide resolved
sdelamo
changed the title
|
LGTM |
|
No description provided.