[3.x] Validate page query parameter in SupportPagination - #10311
Merged
calebporzio merged 3 commits intoJun 18, 2026
Conversation
Casting a raw query string to int on PHP 8.4+ throws ErrorException when the value overflows PHP_INT_MAX. SupportPagination now validates the page value with FILTER_VALIDATE_INT and falls back to page 1 for invalid, out-of-range, zero, or negative values, mirroring Laravel's stock currentPageResolver in PaginationState. Validation is applied inside the registered currentPageResolver closure rather than in resolvePage(), because the URL hook overwrites the paginators property with the raw query string after resolvePage returns. Validating at the read point covers both paths.
pkoury
marked this pull request as draft
May 27, 2026 13:34
pkoury
force-pushed
the
fix/pagination-page-overflow-3x
branch
from
May 27, 2026 13:37
b8857e0 to
1db89be
Compare
pkoury
marked this pull request as ready for review
May 27, 2026 13:41
ghabriel25
reviewed
May 27, 2026
joshhanley
approved these changes
Jun 1, 2026
joshhanley
left a comment
Member
There was a problem hiding this comment.
@pkoury thanks for the PR! I made one small tweak to reassign back to the paginators array once the cast/ reset is done to ensure it's kept up to date.
Contributor
Author
|
Thanks @joshhanley! That makes sense. Appreciate you tightening that up a bit. |
Collaborator
|
great ,thanks everyone! |
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.
1️⃣ Is this something that is wanted/needed? Did you create a discussion about it first?
Yes. Production 500 on PHP 8.5 triggered by a Bingbot crawl of a
WithPaginationpage with an overflow-large?page=value. No prior discussion. The regression is described in section 5️⃣ below.2️⃣ Did you create a branch for your fix/feature?
Yes.
fix/pagination-page-overflow-3xoff3.x.3️⃣ Does it contain multiple, unrelated changes?
No. One method changed, one regression test added.
4️⃣ Does it include tests?
Yes. New unit test in
src/Features/SupportPagination/UnitTest.phpcovering overflow-large strings, non-numeric strings, zero, negative numbers, and the happy path. Tests fail on the unfixed3.xcheckout and pass with the fix applied.5️⃣ Please include a thorough description (including small code snippets if possible) of the improvement and reasons why it's useful.
Problem
On PHP 8.4+,
SupportPagination's registeredcurrentPageResolverclosure does an unguarded(int)cast on a value sourced from the untrusted?page=query string. When the value overflowsPHP_INT_MAX, the cast throwsErrorExceptionand the request 500s. Search engine crawlers reliably reproduce this in production.Non-numeric strings, zero, and negative numbers are also accepted without validation, which doesn't crash but produces incorrect page numbers downstream.
Fix
Validate inside the resolver closure with
FILTER_VALIDATE_INT, falling back to page 1 for invalid, out-of-range, zero, or negative values. Mirrors Laravel's stockcurrentPageResolverinPaginationState::resolveUsing()line for line.The validation has to be at the resolver closure (the read point) rather than inside
resolvePage().addUrlHook()registers aPaginationUrlattribute that callssetPropertyFromQueryString(), which overwritespaginators[$pageName]with the raw query string afterresolvePage()has returned. Validating at the closure covers both the initial-resolution path and the URL-hook path.Cursor pagination is unaffected. The
CursorPaginator::currentCursorResolverclosure passes the value toCursor::fromEncoded()and never casts to int.Companion
A matching PR against
mainwill be opened for the same bug in the 4.x line.