fix(test): align OrderBySessionHardeningTest with shipped get_order_string#7185
Merged
TheWitness merged 2 commits intoJun 3, 2026
Merged
Conversation
…tring Cacti#7098 reworked get_order_string() to the cacti_build_sort_fragment design but left this test asserting the earlier implode($order_parts) shape, so it failed on the 1.2.x tip. Assert the helpers the function actually uses. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the unit test that inspects get_order_string() to assert the hardened sorting flow (normalize/build fragment + validation) rather than the prior session-string rebuild behavior.
Changes:
- Renamed the test to describe sort-column hardening via normalization + fragment building.
- Updated assertions to check for
cacti_normalize_sort_column(...),cacti_build_sort_fragment(...), andvalidate_sort_column(...)usage.
Drop the fixed 1800-byte substr window so the assertions do not break if the function grows, and rename the test to reflect the validate_sort_column check it enforces. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
TheWitness
approved these changes
Jun 3, 2026
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 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.
OrderBySessionHardeningTest(added in #7054) asserts thatget_order_string()rebuilds the ORDER BY viaimplode(', ', $order_parts)from$_SESSION['sort_data']. #7098 later reworked the function to thecacti_normalize_sort_column/cacti_build_sort_fragmentdesign and the test was not updated, so it is red on the current 1.2.x tip.This updates the source-inspection assertions to the helpers the shipped function actually uses (
cacti_normalize_sort_column,cacti_build_sort_fragment, allowlistvalidate_sort_column). No production code change. Base: 1.2.x.