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

[5.7] MethodNotAllowedHTTPException on Intended Redirect #25739

Merged
merged 10 commits into from Sep 23, 2018
Merged

[5.7] MethodNotAllowedHTTPException on Intended Redirect #25739

merged 10 commits into from Sep 23, 2018

Conversation

mcordingley
Copy link
Contributor

@mcordingley mcordingley commented Sep 22, 2018

Environment:

  • Laravel Version: 5.5.40
  • PHP Version: 7.0
  • Database Driver & Version: N/A

Description:

redirect()->intended() can redirect the user to a bad route if the user interacts with a form after session timeout. This leads to errors being shown to a user from reasonable use of the application and is surprisingly difficult to avoid or work around.

Steps To Reproduce:

  • Have an expired session or no session.
  • POST to a route protected by the 'auth' middleware that does not have a corresponding GET route.
    • Easy way to do this is to login, navigate to a form, wait a while, then submit the form.
  • Authenticate and be redirected to the POST route.
  • Receive MethodNotAllowedHTTPException.

Proposed Solution:

Alter \Illuminate\Routing\Redirector::guest() to check if the current request is suitable for redirection and if the requested route isn't suitable for redirect, don't store it as url.intended and instead store the "previous" URL as the intended route to get the user back to being as close as possible to the failed request.

@taylorotwell Asked:

What if the previous route isn't valid either?

I responded:

@taylorotwell It has to be, else it wouldn't have been placed into the session for previous to fetch. Again, see Illuminate\Session\Middleware\StartSession::storeCurrentUrl() where that gets set. And assuming that there's nothing even in there, it'll return a null. That'll have the intended redirect then go to the default location.

@X-Coder264 then informed me:

Please create a new PR then since Taylor does not read PR comments after he already closed it.

@X-Coder264
Copy link
Contributor

@mcordingley Told ya 😄

And yes, I know that contributing to Laravel in any shape or form can be sometimes frustrating, but you gotta keep trying and if there really is no reason why something didn't get merged the first time it was PRed it's gonna be merged the next time.

@mcordingley
Copy link
Contributor Author

@X-Coder264 Thank you. :)

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.

None yet

3 participants