Respond to http OPTIONS method in StaticHandler

[ia3andy]

https://github.com/vert-x3/vertx-web/blob/45a280209f70daf7f760de04ef4becadb52630f4/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/StaticHandlerImpl.java#L170

I think it would make sense to respond to http OPTIONS method: https://stackoverflow.com/questions/38582773/proper-http-methods-for-serving-static-content

[ia3andy]

@vietj if you also think it's relevant could you assign it to me?

[ia3andy]

@pmlopes maybe?

[pmlopes]

Yes that should be fine. Just one note. If CORS was seen then it's a NOOP action, else just add the Allow header to the response.

What makes this a bit more complex is that we should only handle it if a file exists or we risk polluting the case where later handlers will be executed.

[ia3andy]

@pmlopes I was thinking about dealing with it there (at least we know the file exists already): https://github.com/vert-x3/vertx-web/blob/45a280209f70daf7f760de04ef4becadb52630f4/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/StaticHandlerImpl.java#L432

Could you confirm:

• Regarding CORS, do you mean NOOP because CORS don't make sense for static resources?
• By "if CORS was seen" you mean having headers Access-Control-Request-Method AND/OR Access-Control-Request-Headers set?

[pmlopes]

@ia3andy yes, that is the best place to handle this case.

When I mean "seen" CORS, I'm refering to some internal API :)

// here context is the RoutingContext you get on the method call
if (!((RoutingContextInternal) context).seenHandler(RoutingContextInternal.CORS_HANDLER)) {
  // no CORS Handler was executed on this route, so we should respond with the
  // `Allow` header as described on: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow
...
}

Now what still puzzles me, is that if we're returning this header, can we guarantee that if the request method is wrong we return 405? I think we do have already some checks in place but a unit test would be nice to confirm this too.

For example:

> HTTP DELETE /existing.file
< 405 Method Not allowed

> HTTP DELETE /non.existing.file
< 404 Not Found

WDYT?

[ia3andy]

You are right, currently if the method is not GET, HEAD it just passes it to the next, while it should check if the file exist to answer 405 in case the file exists but the method is not allowed.

Note that perf wise, I am not sure that reading the filesystem for all requests is the best (even for GET btw)

For CORS, I thought you meant this (which maybe require a special handling for static files): https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/OPTIONS#preflighted_requests_in_cors

[pmlopes]

We don't hit the file system all the time. There is a small cache of file metadata that keeps track of last requests so we know if a file exists or not.

CORS will be handled if CORSHandler is on the route.

So all those headers are set by the other handler which is responsible for that logic. No need for StaticHandler to "know" about it.

[ia3andy]

@pmlopes I was wondering if the Access-Control-Allow-Method should not depend on if the file exists or not?

• exists then Access-Control-Allow-Method should be the intersection between CorsHandler.allowedMethods & GET, HEAD, OPTIONS (I am not sure what we should answer if there is no CorsHandler)
• if the file doesn't exists then next anyway

[ia3andy]

I will try something next week, sorry I've got sidetracked..

[zenonwch]

Hi @is3andy, have you any progress with this issue?

[ia3andy]

Hi @is3andy, have you any progress with this issue?

Hi @zenonwch,

Sadly no, it got back down my todo list again. I should probably have time to look at this this in the next few days. I popped it up in my todo list. Unless you want to have a look?

Cheers,