Repository navigation
Refuse page numbers and values that overflow or break their kind. - #12
Merged
Merged
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
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.
Summary
Four inputs reached a consumer as a 500 where the contract is a 422, because the library either threw an error it does
not translate or accepted a value the store then refused. All four were reproduced in the services that consume the
library, and each service worked around them at its own edge.
page[number]of9223372036854775807madeOffset::fromPagecompute(page - 1) * limitas a float, and theintconstructor then raised aTypeError. A page above the largest one whose offset still fits anintnow raisesPageNumberOutOfRange, and an offset ofPHP_INT_MAXraisesOffsetOutOfRange, both through newaboveMaximumfactories, so the consumers that already translate those exceptions answer 422. The ceiling depends on the page size:
intdiv(PHP_INT_MAX - 1, size) + 1, so no page is refused for an arbitrary reason and every navigation link staysrepresentable.
PageCountcomputed the page count through a float that loses precision above 2^53 and now usesinteger arithmetic.
UUIDpattern ended in$without theDmodifier, so<uuid>\nmatched, in a filter and in the id of acursor, and reached
UUID_TO_BINas an invalid value. TheINTEGERpattern had the same flaw and accepted a valuethat does not fit an
int.INTEGERnow compares the digits as text againstPHP_INT_MAXandPHP_INT_MINwithout converting the value, and leading zeros and
-0stay valid.DATETIMEchecked only the shape of the value.Iso8601::isValidnow also refuses a calendar that does not exist(
2026-02-30, month 13, year 0000), a clock outside00:00:00to23:59:59, an offset outside±23:59, afraction of more than nine digits, and an instant that falls outside the years 0001 to 9999 in UTC, such as
9999-12-31T22:00:00-05:00. The clock ranges live in the pattern,checkdatecovers the calendar, and the range ischecked once against two bounds with
DateTimeImmutable. A date alone, and a date-time without an offset, read asUTC, stay valid.
Public API: backward compatible.
PageNumberOutOfRange::aboveMaximumandOffsetOutOfRange::aboveMaximumare theadditions, and values that were never valid are now refused, so the release is
2.6.0.Consumers can drop their own guards after
composer update: theDmodifier in their regexes, the validation of thecursor id, and the check for a date-time outside the years 0001 to 9999. They keep a cap on
page[number]when anylayer handles the offset as a 32-bit integer, since the ceiling of the library is the 64-bit one.
Related issue
None.
Checklist
PHP_INT_MAX; theintegers
9223372036854775807,9223372036854775808,-9223372036854775808and-9223372036854775809; a UUIDfollowed by a line break; impossible dates, clock values, offsets and fractions; and both edges of the range.
the page number.
make reviewpasses.make testspasses. 399 tests, lines 100% (976/976), MSI 100% (767 of 767 killed).