fix(#48): enforce key/value size bounds explicitly with KeyValueLimits - #66
Merged
Conversation
FoundationDB length parameters in libfdb_c are 32-bit C int while PHP strlen() returns 64-bit, and FDB also enforces hard key (10 KB) and value (100 KB) limits — neither boundary was being checked at the PHP trust boundary before the FFI call. Add src/KeyValueLimits and wire it into every FFI surface that takes a length argument (Transaction/ReadTransaction writes, reads, range queries, conflict ranges, option setters, RangeResult, AdminClient tenant/server, Database tenant-name, NetworkOptions, Database setOption). Each call site now gets an eager \\InvalidArgumentException with the offending length at the offending line, instead of an opaque FDBException 2102/2103 at commit time. Also rejects any byte string exceeding the 32-bit FFI safety bound (2^31-1) so a > 2 GiB payload cannot silently truncate across the FFI boundary. The transaction-size aggregate (code 2101) is still reported by libfdb_c on commit; transact() does not retry guard rejections since they are programmer errors, not transient conflicts. - Add tests/Unit/KeyValueLimitValidationTest covering accepted and rejected boundaries, the FFI guard constant, and the named-policy message wording. - Augment tests/Integration/KeyValueLimitTest to prove the new fail-at-call-site behaviour (no opaque commit error, transactions recover after guard rejection) and to retain the existing accepted- size and transaction-aggregate coverage. - Document the limits in docs/transactions.md (#key--value-size-limits), docs/atomic-operations.md (#size-limits), and docs/error-handling.md. - CHANGELOG entry under Unreleased / Fixed. - composer lint / composer test:unit both stay green.
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
1 participant
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.
Fixes #48
Summary
FoundationDB's libfdb_c C API declares its length parameters as 32-bit C int. PHP strlen() returns a 64-bit integer. Neither the FoundationDB key limit (10 KB), the value limit (100 KB) nor the FFI 32-bit safety bound (2^31 - 1) were being checked at the PHP trust boundary before the FFI call.
This PR introduces src/KeyValueLimits and wires it into every FFI surface that takes a length argument:
Each call site now gets an eager \InvalidArgumentException with the offending length at the offending line, instead of an opaque FDBException code 2102/2103 at commit time. transact() does not retry guard rejections — they are programmer errors, not transient conflicts.
Tests
Documentation
CI Status