conjure-undertow supports OPTIONS requests. #480
Conversation
Previously they would result in 405 method not allowed responses.
...ndertow-runtime/src/main/java/com/palantir/conjure/java/undertow/runtime/ConjureHandler.java
Outdated
Show resolved
Hide resolved
...ndertow-runtime/src/main/java/com/palantir/conjure/java/undertow/runtime/ConjureHandler.java
Outdated
Show resolved
Hide resolved
...ndertow-runtime/src/main/java/com/palantir/conjure/java/undertow/runtime/ConjureHandler.java
Outdated
Show resolved
Hide resolved
...ndertow-runtime/src/main/java/com/palantir/conjure/java/undertow/runtime/ConjureHandler.java
Outdated
Show resolved
Hide resolved
|
Should we update the conjure spec to define the behavior for OPTIONS? |
|
(I'm also wondering, why exactly do we need support for OPTIONS?) |
|
@uschi2000 dropwizard/jetty+jersey transparently support OPTIONS requests because browsers send them as part of a 'CORS preflight' request. If your server doesn't respond to these, then you won't be able to hit these endpoints from a browser. We discovered this when we were setting up tests where conjure-typescript hits the conjure-verification from a browser! https://github.com/palantir/conjure-verification/blob/d99bad8677a4655cdc77e99406e91a3343bc7145/verification-http-server/src/lib.rs |
|
Yep, @iamdanfox is correct, that is the rationale for this change.
Absolutely, I chatted with @markelliot about adding a note describing OPTIONS support but I neglected to file a ticket to follow up. Filed: palantir/conjure#355 |
...ndertow-runtime/src/main/java/com/palantir/conjure/java/undertow/runtime/ConjureHandler.java
Show resolved
Hide resolved
| public void handleRequest(HttpServerExchange exchange) { | ||
| Preconditions.checkState(Methods.OPTIONS.equals(exchange.getRequestMethod()), "Expected an OPTIONS request"); | ||
| exchange.getResponseHeaders().put(Headers.ALLOW, allowValue); | ||
| exchange.setStatusCode(StatusCodes.NO_CONTENT); |
There was a problem hiding this comment.
Do we want Access-Control-Max-Age too? https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request I assume this lets browsers cache this for a bit?
There was a problem hiding this comment.
If we want to set Access-Control headers, I think that belongs in WebSecurityHandler. I don't think there's anything specific to options requests for access control headers beyond standard browser use, but I'm not completely confident on that.
|
Released 3.14.0 |
Before this PR
Previously they would result in 405 method not allowed responses.
After this PR
==COMMIT_MSG==
conjure-undertow supports OPTIONS requests.
==COMMIT_MSG==