flow-development-collection
flow-development-collection copied to clipboard
Accidental? Feature removal: <form> tunnel request methods via field `__method`
This documentation is not up to date in projects since Flow 7.0: https://flowframework.readthedocs.io/en/stable/TheDefinitiveGuide/PartIII/Http.html?highlight=X-HTTP-Method-Override#request-methods
@kitsunet said
interesting, that seems to have been lost with the PSR-7 adoption, I found it back in Flow 5.3. We could/should re-add it
so to tunnel request methods like this wont work anymore:
<input type="hidden" name="__method" value="PUT" />
(in the code base is not a single occurrence of __method)
This was indeed accidentally removed with the switch to PSR-7. Last trace of this feature: https://github.com/neos/flow-development-collection/pull/1552/files#diff-be1687417ba866cbb06cf3eefe5cf80f9094a5890daa999b82b961f2a35df06aL115 And here the tests that should have covered it: https://github.com/neos/flow-development-collection/pull/1552/files#diff-ecafdf3d5592b4478c12c4195051c49f8b65cd1aeb1de5282495c9e6cbdbf890L176 So I'm not innocent on the disappearing of this
- Unit Tests sometimes just don't suffice at all to make sure a feature doesn't perish, because if the original class vanishes the test for this class also does (or maybe that just shouldn't happen?!)
- How do we want to make sure that every ServerRequest instance created like guzzles ServerRequest::fromGlobals() we currently use, does apply a logic to parse the request method from an override? Own factory that should only be used, effectively cutting back on the flexibility to swap the PSR implementation? Probably rather a new middleware, but then it only applies to requests that have passed through the middleware chain and not for ones that are created ad-hoc (we have this e.g. in the StandaloneView as a fallback)
Unit Tests sometimes just don't suffice at all to make sure a feature doesn't perish
In theory they should, as in: nobody should just remove a test without thinking twice. This case was a bit special just because of the sheer amount of broken tests with the whole HTTP stack being replaced.
How do we want to make sure that every ServerRequest instance created like guzzles ServerRequest::fromGlobals() we currently use, does apply a logic to parse the request method from an override?
ServerRequest::fromGlobals() should only be used in cases where a completely fresh request is needed (e.g. for CLI). Otherwise we should get the "current server request" from the middleware (or via Bootstrap::getActiveRequestHandler() work around if not possible otherwise).
So, we could use s.th. like https://github.com/middlewares/method-override (or create a custom one / extend an existing one)
Maybe we should just adjust the documentation on how to install/configure that middleware if this feature is required
In case we do want to add an own middleware as a BUGFIX, I created a PR for that. We could also adjust the documentation and hint towards installing some arbitrary middleware.