[UPMERGE] 5.0 -> 5.1 - #3181
Merged
Merged
Conversation
Co-authored-by: dpfaffenbauer <5981845+dpfaffenbauer@users.noreply.github.com>
…haracters Co-authored-by: dpfaffenbauer <5981845+dpfaffenbauer@users.noreply.github.com>
…duplication Co-authored-by: dpfaffenbauer <5981845+dpfaffenbauer@users.noreply.github.com>
The "@" rejection tested the whole URL, so a same-host redirect carrying an "@" in its query or fragment was silently downgraded to the default. It now looks at the parsed authority only, which also makes the preceding user check meaningful instead of redundant. Backslashes and control characters are now rejected for absolute URLs as well, not just for relative ones. Beyond the "_redirect" parameter, the login form feeds "_target_path" and "_failure_path" to Symfony, which accepts any value starting with "/" or "http". Both are now run through the same allow-list, in the controller that renders the form and in a success/failure handler pair, so a target that is neither a relative path nor a same-host URL falls back to the configured default. The success handler covers the Referer as well, which the firewall uses via use_referer.
Psalm rejects calls into @internal Symfony code. The rejected failure path is now read through the public Request API, and the redirect the parent handler produced is validated as well, which also covers a failure path configured as a nested parameter.
Symfony marks Request::get() as internal as well, which Psalm rejects, so the parameter is looked up in the attribute, query and request bags instead.
FailedMessageDetails::jsonSerialize() emitted "failedAt" while every consumer - the classic grid column and the Studio MessengerFailedGrid - reads "failed_at", and the DTO itself already declares #[SerializedName(\"failed_at\")]. Because JsonSerializableNormalizer outranks ObjectNormalizer in the framework serializer, jsonSerialize() decides the payload keys and that attribute never took effect, so the column was empty for every transport. Emit "failed_at" instead, which makes the existing attribute true rather than dead code, and align the classic store field with it. The Studio grid already reads "failed_at" and needs no change.
Messenger: fix empty "Failed At" column by emitting the key the UI reads
Fix open redirect vulnerabilities in redirect parameter handling
dpfaffenbauer
force-pushed
the
upmerge/5.0_5.1
branch
from
August 24, 2026 13:29
1a1ccc0 to
560be5f
Compare
|
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Upmerge of
5.0into5.1.Remember! The upmerge should always be merged with the
Merge pull requestbutton.What it carries forward
_redirectparameter, including the new login success/failure handlers (arrived on5.0via [UPMERGE] 4.1 -> 5.0 #3183)Security-relevant files carried by this merge
All merged verbatim — nothing had to be adapted:
ResourceBundle/Controller/RedirectUrlValidationTrait.phpvalidateRedirectUrl()implementation (rejects dangerous schemes/special characters, scopes the authority check)CoreBundle/Security/ShopUserAuthenticationSuccessHandler.phpCoreBundle/Security/ShopUserAuthenticationFailureHandler.phpCoreBundle/Resources/config/services/handler.yml@security.http_utils,@http_kernel)CoreBundle/Resources/config/pimcore/security.ymlsuccess_handler/failure_handleron the shop firewallFrontendBundle/Controller/{Cart,Customer,Frontend,Register,Security}Controller.php_redirectStorageListBundle/Controller/StorageListController.phpfeatures/ui/frontend/account/login.feature,Behat/Context/Ui/Frontend/LoginContext.phpAll three new classes are present on the merged tree (verified by path, not just by diff).
Conflicts
None — the merge applied cleanly, zero conflicted files.
Verification
Net diff against
5.1is exactly #2963's 13 files plus #3179's 2, and nothing else:security.ymldid not overwrite5.1's version. The file touched isCoreBundle/Resources/config/pimcore/security.yml(the shop firewall). Diffed against5.1's committed copy, the merged result adds exactly the two handler lines and changes nothing else.config/packages/security.yamlis not touched, so Fix typo in coreshop.security.frontend_regex breaking the pimcore-studio exclusion #3164's correctedcoreshop.security.frontend_regex(^/(?!admin|pimcore-studio)[^/]*) is untouched — confirmed by reading it back off the merged tree.validateRedirectUrl()definition on the merged tree.failedAtfield is still absent fromMessengerFailedMessage, so the TypeScript cleanup already on5.1is not reverted.composer.json: not touched;replaceset equality holds — 66 split packages, 66 entries, missing[], extra[].checked=66 bad=0(siblings^5.1, bothbranch-aliaskeys at5.1-dev).Resources/public/studio/touched.php -lclean on every changed PHP file.Ordering
Merge order for this round: #3183 (merged) → this PR → #3182 (
[UPMERGE] 5.1 -> 2026.x). #3182 must be refreshed once this lands, so that #2963 and #3179 reach2026.x.