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

[11.x] Enhance malformed request handling #50735

Merged
merged 4 commits into from
Mar 25, 2024

Conversation

jnoordsij
Copy link
Contributor

(Rebased and retargetted version of #50470)

Solves #50466.

This PR aims to have better handling of malformed requests; in particular those which throw exceptions when certain methods on the request are called, like getMethod with content as described in issue mentioned above.

To Do:

Implementation notes:

  • I've broadened matching from just SuspiciousOperationException to RequestExceptionInterface, given that Symfony recommends this per documentation in this interface. This is a separate that can be removed if wanted, however it seems more accurate and should generally not make any difference for valid requests. No additional tests have been added for other possible exceptions as part of this broadening; this can be part of a follow-up issue/MR.
  • Due to CompiledRouteCollection using a copy of the Request instance in the scope of the match method (due to requestWithoutTrailingSlash call), the getMethod call is never called on the 'original' request within the route handling and thus causes an exception when isMethod('HEAD') is performed in the scope of preparing the error response. This is once again caught by the exception handler and then correctly rendered on the second attempt. Albeit technically a little ugly, this is probably not worth addressing further as the outcome is correct and these types of requests should be extremly rare anyways.
  • The existing Bad hostname provided message seems to be based on this line, but given that the exception also happens on incorrect method values (see test), a generic message seems to be more appropriate.

@taylorotwell taylorotwell merged commit bf02fde into laravel:11.x Mar 25, 2024
30 checks passed
@jnoordsij jnoordsij deleted the malformed-request-handling branch March 26, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants