Removed redundant POST request exclusion info #3883
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The default ``ExceptionListener::setTargetPath()`` already excludes POSTs/PUTs, so suggesting that people who implement their own listener to do this is a bit misleading. However, doing this to prevent XMLHttpRequest URIs from being saved is still valuable.
| method, the user is redirected to this route only to get a 404 error. | ||
| On some occasions, this is unexpected. For example when the last request before logout | ||
| was an XMLHttpRequest route, the user may be redirected back to an invalid | ||
| route. |
There was a problem hiding this comment.
I like all of this! But, this is actually related to logging in, not logging out:
- I go to a page in my browser and it redirects me to
/login - While I'm typing in my user/pass, another tab I have open is making an AJAX request, which is being redirected to
/login(and which is causingsetTargetPathto be called). - I login, and now I'm redirected to that AJAX URL
So, if you clarify this (i.e. don't talk about logout), we're good! If I'm missing something, please let me know!
Thanks!
|
@ryancastle can you please take a look at @weaverryan 's comment? |
|
@wouterj ,@weaverryan Hopefully, this is improved? |
| On some occasions, this is unexpected. For example when the last request | ||
| URI was an HTTP POST against a route which is configured to allow only a POST | ||
| method, the user is redirected to this route only to get a 404 error. | ||
| In some situations, this is not ideal. For example when the last request |
There was a problem hiding this comment.
I'd add a comma after "For example".
|
@xabbuh Comma added! |
|
Great job Ryan - this is very clean now. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The default
ExceptionListener::setTargetPath()already excludes POSTs/PUTs, so suggesting that people should implement their own listener to do this is a bit misleading.However, doing this to prevent XMLHttpRequest URIs from being saved is still valuable.