fix(#41): validate caller-supplied DirectoryLayer::create prefix - #68
Merged
Conversation
Caller-supplied prefixes were written directly into the subdirs index without any conflict check, so a manual prefix overlapping existing directory metadata or content keys silently corrupted or overwrote data. Route every caller-supplied prefix through validateRawPrefix() which throws DirectoryException with a printable rendering of the offending key when: - the prefix is empty; - the node-metadata range under the prefix is not empty; - the content-key range under the prefix is not empty. The check runs inside the transaction before any set(), so a failed create leaves no partial state.
5 tasks
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.
Fixes #41
Summary
DirectoryLayer::create(...,$prefix)previously wrote any caller-suppliedprefix directly into the subdirs index without any conflict check, so a
manual prefix overlapping existing directory metadata or content keys
silently corrupted or overwrote data.
This change routes every caller-supplied prefix through
DirectoryLayer::validateRawPrefix(), which throwsDirectoryExceptionwith a printable rendering of the offending key when:
The check runs inside the transaction before any
set()call, so a failedcreate leaves no partial state and the same
pathcan be retried viaauto-allocation.
Changes
src/Directory/DirectoryLayer.php: Validation logic extracted into aprivate helper
validateRawPrefix($rawPrefix,$contentSubspaceKey, $nodeProbe,$contentProbe)so it can be unit-tested in pure PHPwithout needing a live Transaction. The instance-level adapter
assertValidCallerSuppliedPrefix($tr,$prefix)binds it to liveFoundationDB probes. Also tightened the existing auto-allocation error
message ("data" → "directory metadata") for consistency with the
new explicit-prefix diagnostics.
docs/directory-layer.md: Documents the explicit-prefixAPI andthe three rejection conditions in a four-row table.
CHANGELOG.md: New[Unreleased]entry underFixedwith thebug description, the validation contract, and a pointer to the two
new test files.
Tests
tests/Unit/DirectoryPrefixValidationTest.php(20 new tests):acceptance boundary (
'visited, non-empty prefix accepted, 1-byteprefix accepted, binary prefix accepted, content-key composition
observed at probes), rejection boundaries (empty prefix, metadata
conflict, content conflict, ordering diagnostic when both conflict),
error-message printable-rendering across printable, control, ~,
0xFF, mixed.
tests/Integration/DirectoryTest.php(4 new tests):end-to-end happy-path with auto-validation free prefix (
openround-trip, usable for normal CRUD), two-distinct-prefixes
acceptance, empty-prefix rejection (and no transaction state
mutated afterwards), explicit-prefix overlapping content keys
rejected.
All checks green locally:
CI will additionally exercise PHP 8.2/8.3/8.4 unit tests plus the
5-node FDB cluster
e2e-testsjob (which the integration suiteruns).