Fixed handling of CORS within Vert.x #4232
Conversation
sergiuszkierat
force-pushed
the
fix/interceptor_with_vertex
branch
2 times, most recently
from
7a41568 to
e06cec8
Compare
| @@ -25,6 +26,14 @@ object PathMapping { | |||
| case (None, path) => router.route(path) | |||
| } | |||
|
|
|||
| private[vertx] def createOptionsRoute(router: Router, route: RouteDefinition): Option[Route] = | |||
There was a problem hiding this comment.
I don't think I understand the purpose of this method. Is it "Options" as in the OPTIONS HTTP method? If so, why are we creating an options-route if the method is get/head/post/etc.?
There was a problem hiding this comment.
Based on this Vert.x will signal a 405 error if a route matches the path, but doesn’t match the HTTP Method. As I understand this is our case, so I've artificially added its counterpart OPTIONS route for the same path in order to get a chance to pass through. Do you see a better approach?
| @@ -26,7 +26,9 @@ trait VertxFutureServerInterpreter extends CommonServerInterpreter with VertxErr | |||
| * A function, that given a router, will attach this endpoint to it | |||
| */ | |||
| def route[A, U, I, E, O](e: ServerEndpoint[VertxStreams with WebSockets, Future]): Router => Route = { router => | |||
| mountWithDefaultHandlers(e)(router, extractRouteDefinition(e.endpoint), vertxFutureServerOptions) | |||
| val routeDef = extractRouteDefinition(e.endpoint) | |||
| optionsRoute(e)(router, routeDef).foreach(_.handler(endpointHandler(e))) | |||
There was a problem hiding this comment.
I think this would need some more explanation, as to why it's needed here and what's the purpose. Won't we run the routes twice?
There was a problem hiding this comment.
Having only i.e. GET /path is not sufficient 🫨 , so by adding its counterpart OPTIONS /path I am explaining to Vert.x : please accept my preflight request instead of rejecting it quickly
*Technical changes related to this issue:* - registered options handler explicitly *Technical changes NOT related to this issue:* - added CORSInterceptor example for Vert.x server Closes #3651
|
We have CORS tests which pass smoothly here 😭 I've added the draft PR which needs to be continued. As fixed tests might raise the issues within other servers. |
sergiuszkierat
force-pushed
the
fix/interceptor_with_vertex
branch
from
e06cec8 to
d9205e0
Compare
Technical changes related to this issue:
Technical changes NOT related to this issue:
Closes #3651