[Security] Fix login url matching when app is not run with url rewriting or from a sub folder #47317
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
Contributor
Author
|
Not sure about the the CI errors. They don't seem to be related to the fix 🤷♂️ |
carsonbot
changed the title
nicolas-grekas
changed the title
nicolas-grekas
changed the title
nicolas-grekas
approved these changes
Sep 28, 2022
chalasr
approved these changes
Sep 28, 2022
fabpot
approved these changes
Sep 29, 2022
…ing or from a sub folder
Member
|
Thank you @sgehrig. |
Merged
Contributor
Author
|
@fabpot Is there a reason why this fix has not been merged into 6.0, 6.1, etc.? |
Member
|
This PR was not merged up to 6.0 and 6.1 before the releases. So, it will be part of the next release in about a month. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Uses the fix suggested by @weaverryan in #44893 (comment). I also added three tests for scenarios which I could replicate from running a simple app on a real webserver (Apache and Nginx). This, however, might not be sufficient because there could be other combinations of server variables like
DOCUMENT_ROOT,PHP_SELF,SCRIPT_FILENAME,SCRIPT_NAMEand possibly others depending on the server configuration and setup. As long as\Symfony\Component\HttpFoundation\Request::getBaseUrl()and\Symfony\Component\HttpFoundation\Request::getPathInfo()work correctly, I assume that the fix will also be correct in all those constellations.The fix is based on the assumptions that:
\Symfony\Component\HttpFoundation\Request::getBaseUrl()always returns an empty string when the application is run from root without the front controller script in the URL (using URL rewriting for example)\Symfony\Component\HttpFoundation\Request::getBaseUrl()always returns the path from the server root to the application base path (possibly including the front controller script)\Symfony\Component\HttpFoundation\Request::getPathInfo()always returns just the routed part of the requestPlease advise if you'd need some more tests.