Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

remove HEAD/GET rewrite #15

Open
azjezz opened this issue May 14, 2019 · 3 comments · May be fixed by #20 or #31
Open

remove HEAD/GET rewrite #15

azjezz opened this issue May 14, 2019 · 3 comments · May be fixed by #20 or #31

Comments

@azjezz
Copy link
Contributor

azjezz commented May 14, 2019

https://github.com/hhvm/hack-router/blob/master/src/router/BaseRouter.php#L36

@fredemmott
Copy link
Contributor

fredemmott commented Nov 22, 2019

Still not convinced by this; security risk should be addressed by only doing this if GET is the only method registered for that uri (part of #13). Also don't think it's appropriate for the only way to do this to be middleware that's aware of the routing table.

That said, some potential improvements:

  • consider the routing table invalid and raise an error if a path has methods-other-than-GET but no HEAD responder; this should probably be done for correctness whether the HEAD -> GET rewrite is there or not
  • make it opt in

@azjezz
Copy link
Contributor Author

azjezz commented Nov 22, 2019

Also don't think it's appropriate for the only way to do this to be middleware that's aware of the routing table.

a middlware doesn't actually need the routing table in order to replicate this, it just needs a "matcher" ( can be the base router in hack-router ) : https://github.com/nuxed/framework/blob/rx/src/Nuxed/Http/Router/Middleware/ImplicitHeadMiddleware.hack#L6-L87

@fredemmott
Copy link
Contributor

To be safe and avoid the same same GET/POST -> HEAD issue even if it's multiple controllers or even different patterns that matcher must be the routing table

@azjezz azjezz linked a pull request Feb 8, 2020 that will close this issue
@lexidor lexidor linked a pull request May 25, 2023 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants