feat: dispatch OPTIONS requests #10011

sdelamo · 2023-10-23T11:49:46Z

It adds micronaut.server.dispatch-options-requests configuration setting which by default is false. If set to true, an OPTION request, unless it is a CORS Preflight request, returns 204 and the Allow HTTP Header populated.

Spring Boot 3.1. by default setsspring.mvc.dispatch-options-request to true and it behaves the same way as we will in this PR. Spring Boot returns 200, I returned 204 which I think it is more correct.

Add micronaut.server.dispatch-options-requests configuration setting which by default is false

sdelamo · 2023-10-23T11:50:11Z

@yawkat @graemerocher @dstepanov let me know if you agree with this PR and I will add more docs.

dstepanov · 2023-10-23T12:02:07Z

http-server/src/main/java/io/micronaut/http/server/OptionsFilter.java

+        if (request.getMethod() != HttpMethod.OPTIONS) {
+            return null; // proceed
+        }
+        if (CorsUtil.isPreflightRequest(request)) {


It might be better to run the CORS filter first instead of this check.

I have added a commit to ensure the CorsFilter happens before the Optionsfilter. However, I think it is more correct to leave this if check in place.

yawkat · 2023-10-24T10:12:01Z

this essentially hijacks all our options processing? that seems wrong

yawkat · 2023-10-24T10:12:44Z

also 200 is better here, cant actually say what content length will be returned by GET

sdelamo · 2023-10-24T10:20:03Z

this essentially hijacks all our options processing? that seems wrong

what "Option processing" are we currently doing?

sdelamo · 2023-10-24T10:20:49Z

also 200 is better here, cant actually say what content length will be returned by GET

if the response has no content as in this PR, I think a 204 is more correct.

yawkat · 2023-10-24T11:20:52Z

i mixed up HEAD and OPTIONS, both my points don't apply to OPTIONS.

dstepanov · 2023-10-24T13:04:04Z

@sdelamo Jonas is right, what if somebody defines the OPTIONS methods manually? How do we handle the same scenario with HEAD?

sdelamo · 2023-10-24T13:21:19Z

@sdelamo Jonas is right; what if somebody defines the OPTIONS methods manually? How do we handle the same scenario with HEAD?

Do you mean if someone annotates a controller method with @Options? We don't have any test of that in our codebase. The @Options annotation only appears in the docs ;-)

I can modify the filter and check for a route match, and if there is a route match, proceed. I assume a route match will happen when you have a @controller method annotated with @Options. Will that fulfill your concerns? @dstepanov

dstepanov · 2023-10-24T13:33:56Z

Sound good, please test the HEAD use case as well

sdelamo · 2023-10-24T15:39:35Z

Sound good, please test the HEAD use case as well

Please, note this filter proceeds if the request methods is other than option

http-server/src/main/java/io/micronaut/http/server/OptionsFilter.java

http-server/src/main/java/io/micronaut/http/server/cors/CorsUtil.java

http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java

sdelamo · 2023-10-25T10:48:09Z

@sdelamo Jonas is right; what if somebody defines the OPTIONS methods manually? How do we handle the same scenario with HEAD?

Do you mean if someone annotates a controller method with @Options? We don't have any test of that in our codebase. The @Options annotation only appears in the docs ;-)

I can modify the filter and check for a route match, and if there is a route match, proceed. I assume a route match will happen when you have a @controller method annotated with @Options. Will that fulfill your concerns? @dstepanov

@dstepanov I added it here 2f54ebe

graemerocher · 2023-10-25T10:59:28Z

src/main/docs/guide/httpServer/routing.adoc

+----
+micronaut:
+  server:
+    dispatch-options-requests: true


this should be under CorsConfigration

That's doesn't need to related to CORS, Spring MVC has it the sam way

This is not related to CORS or necessary for CORS.

http-server/src/main/java/io/micronaut/http/server/OptionsFilter.java

http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java

…onfiguration.java Co-authored-by: Jonas Konrad <[email protected]>

sonarcloud · 2023-11-10T15:20:39Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

89.5% Coverage
0.0% Duplication

feat: dispatch OPTIONS requests

4b2b779

Add micronaut.server.dispatch-options-requests configuration setting which by default is false

sdelamo added the type: enhancement New feature or request label Oct 23, 2023

sdelamo requested review from graemerocher, dstepanov and yawkat October 23, 2023 11:49

dstepanov requested changes Oct 23, 2023

View reviewed changes

OrderFilter after CorsFilter

79ea596

sdelamo requested a review from dstepanov October 23, 2023 13:43

dstepanov approved these changes Oct 23, 2023

View reviewed changes

yawkat approved these changes Oct 24, 2023

View reviewed changes

graemerocher approved these changes Oct 25, 2023

View reviewed changes

graemerocher requested changes Oct 25, 2023

View reviewed changes

http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java Show resolved Hide resolved

http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java Show resolved Hide resolved

sdelamo added 7 commits October 25, 2023 12:20

add @SInCE javadoc

417f651

annotate CorsUtil with @internal

28b3300

use PREFIX constant

9ae496b

annotate OptionsFilter with @internal

d05b56a

check for Options RouteMatch

2f54ebe

checkstyle: order public static final

58c2161

add docs

644231e

sdelamo requested review from yawkat, graemerocher and dstepanov October 25, 2023 10:48

dstepanov approved these changes Oct 25, 2023

View reviewed changes

graemerocher approved these changes Oct 25, 2023

View reviewed changes

graemerocher requested changes Oct 25, 2023

View reviewed changes

remove empty line

a694a5f

jeremyg484 reviewed Oct 25, 2023

View reviewed changes

http-server/src/main/java/io/micronaut/http/server/OptionsFilter.java Outdated Show resolved Hide resolved

sdelamo requested review from graemerocher, dstepanov and jeremyg484 October 26, 2023 10:17

sdelamo mentioned this pull request Oct 26, 2023

OPTIONS method not allowed after enable cors #9992

Closed

sdelamo added 2 commits October 26, 2023 17:34

Merge branch '4.2.x' into dispatch-options

86ba084

use 200 instead of 204

c050642

jeremyg484 approved these changes Oct 26, 2023

View reviewed changes

sdelamo added this to the 4.2.0 milestone Nov 10, 2023

dstepanov approved these changes Nov 10, 2023

View reviewed changes

yawkat reviewed Nov 10, 2023

View reviewed changes

http-server/src/main/java/io/micronaut/http/server/HttpServerConfiguration.java Outdated Show resolved Hide resolved

yawkat approved these changes Nov 10, 2023

View reviewed changes

Update http-server/src/main/java/io/micronaut/http/server/HttpServerC…

9136aac

…onfiguration.java Co-authored-by: Jonas Konrad <[email protected]>

graemerocher approved these changes Nov 10, 2023

View reviewed changes

sdelamo merged commit 9c2454f into 4.2.x Nov 10, 2023
16 checks passed

sdelamo deleted the dispatch-options branch November 10, 2023 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: dispatch OPTIONS requests #10011

feat: dispatch OPTIONS requests #10011

sdelamo commented Oct 23, 2023 •

edited

Loading

sdelamo commented Oct 23, 2023

dstepanov Oct 23, 2023

sdelamo Oct 23, 2023

yawkat commented Oct 24, 2023

yawkat commented Oct 24, 2023

sdelamo commented Oct 24, 2023

sdelamo commented Oct 24, 2023

yawkat commented Oct 24, 2023

dstepanov commented Oct 24, 2023 •

edited

Loading

sdelamo commented Oct 24, 2023

dstepanov commented Oct 24, 2023

sdelamo commented Oct 24, 2023

sdelamo commented Oct 25, 2023

graemerocher Oct 25, 2023

dstepanov Oct 25, 2023

sdelamo Oct 25, 2023

sonarcloud bot commented Nov 10, 2023

feat: dispatch OPTIONS requests #10011

feat: dispatch OPTIONS requests #10011

Conversation

sdelamo commented Oct 23, 2023 • edited Loading

sdelamo commented Oct 23, 2023

dstepanov Oct 23, 2023

Choose a reason for hiding this comment

sdelamo Oct 23, 2023

Choose a reason for hiding this comment

yawkat commented Oct 24, 2023

yawkat commented Oct 24, 2023

sdelamo commented Oct 24, 2023

sdelamo commented Oct 24, 2023

yawkat commented Oct 24, 2023

dstepanov commented Oct 24, 2023 • edited Loading

sdelamo commented Oct 24, 2023

dstepanov commented Oct 24, 2023

sdelamo commented Oct 24, 2023

sdelamo commented Oct 25, 2023

graemerocher Oct 25, 2023

Choose a reason for hiding this comment

dstepanov Oct 25, 2023

Choose a reason for hiding this comment

sdelamo Oct 25, 2023

Choose a reason for hiding this comment

sonarcloud bot commented Nov 10, 2023

sdelamo commented Oct 23, 2023 •

edited

Loading

dstepanov commented Oct 24, 2023 •

edited

Loading