Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plack::App::File should check request method #660

Closed
robrwo opened this issue Feb 11, 2021 · 4 comments · Fixed by #662
Closed

Plack::App::File should check request method #660

robrwo opened this issue Feb 11, 2021 · 4 comments · Fixed by #662

Comments

@robrwo
Copy link
Contributor

robrwo commented Feb 11, 2021

It would make sense to modify Plack::App::File to check the request method.

Currently it seems to accept any request method.

If it gets an OPTIONS request, it could return HTTP 204 with the Allow header set to "GET, HEAD, OPTIONS" (perhaps with an optional callback attribute that allows one to hook into it to handle CORS preflight requests).

If it gets something other than a GET or HEAD request, it should return HTTP 405 (method not allowed).

(Possibly for backwards compatibility one could specify a lit of allowed request methods, in case one wants to accept POST requests.)

@robrwo
Copy link
Contributor Author

robrwo commented Feb 11, 2021

I'm working on a PR for it. I think the simplest way forward is to simply behave as normal if no methods are set.

If methods are set, then it validates against the list and returns an HTTP 405 if not an allowed method.

If methods are set and it gets an OPTIONS request, it returns a simple response, as per https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/OPTIONS

If there is a CORS handler configured, it passes the environment, file and response to the handler.

@miyagawa
Copy link
Member

miyagawa commented Feb 11, 2021

https://metacpan.org/pod/Plack::Middleware::CrossOrigin implements more comprehensive solution and you can enable it in front of Plack::App::File, or any other PSGI app.

@robrwo
Copy link
Contributor Author

robrwo commented Feb 11, 2021

CORS is less important. I care more about restricting the request methods, and adding OPTIONS support (since I have a site that often gets OPTIONS requests for some things.)

robrwo added a commit to robrwo/Plack that referenced this issue Feb 11, 2021
@miyagawa
Copy link
Member

I'm sure the middleware above has configuration options to let you do what I want, but if not, you can write your own middleware (possibly inline) to return 204 with GET, HEAD, and OPTIONS to OPTIONS requests. My argument here is that your need should not be specific to App::File app at all.

robrwo added a commit to robrwo/Plack that referenced this issue Feb 11, 2021
robrwo added a commit to robrwo/Plack that referenced this issue Feb 11, 2021
robrwo added a commit to robrwo/Plack that referenced this issue Feb 11, 2021
@robrwo robrwo changed the title Plack::App::File should check request method and handle OPTIONS requests Plack::App::File should check request method Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants