Repository navigation
fix(serialize): strict validation in __unserialize() rejection paths - #225
Merged
Merged
Conversation
Harden __unserialize() against hand-crafted or corrupt payloads. A payload is now rejected unless it is exactly what __serialize() can emit: - Unknown top-level keys (including integer keys) throw "Invalid serialization data for Judy array". - optimizeIteration must be a literal bool, not merely truthy. - Out-of-range types are rejected before JTYPE, so the constructor-facing E_WARNING in the JTYPE macro no longer leaks out of unserialize(). - optimizeIteration=true on a type that cannot honour it throws "Invalid Judy type in serialized data". The optimizeIteration/type check runs before judy_free_array_internal(), so a rejected payload leaves an already-populated object fully intact rather than half-freed. It reuses judy_type_can_mirror() so the accepted set cannot drift from the constructor's own rule. Round-trips are unaffected: __serialize() only emits optimizeIteration when mirror_payload is set, which judy_set_optimize_iteration() only does for the two mirror-capable types. tests/serialize_error_001.phpt is extended in place (same test identity) with regression cases for each rejection path, the type-12/type-99 warning-free boundary, the intact-object-on-rejection guarantee, and the honoured optimizeIteration roundtrip.
discipline: passedBase
|
Test Results
Benchmark Results
Run Status
Memory (Linux, PHP 8.6)Peak RSS of a child process building one structure, n = 500,000, minus an empty-process floor. Judy's memory advantage is type- and scale-dependent — see BENCHMARK.md for the curve across sizes on a dedicated host.
API Speedups (Linux, PHP 8.6)Speedup = PHP time / Judy time. Bold = Judy faster.
Core Types (Linux, PHP 8.6)Speedup = PHP time / Judy time. Bold = Judy faster.
All types detailCore Types (Linux, PHP 8.6) — AllSpeedup = PHP time / Judy time. Bold = Judy faster.
Cross-version detail (Linux)Core Types — LinuxEach cell: Judy ms (speedup vs PHP array).
API — Linux
Cross-version detail (Windows)Core Types — WindowsEach cell: Judy ms (speedup vs PHP array).
API — Windows
Windows results (PHP 8.6)Memory (Windows, PHP 8.6)Peak RSS of a child process building one structure, n = 500,000, minus an empty-process floor. Judy's memory advantage is type- and scale-dependent — see BENCHMARK.md for the curve across sizes on a dedicated host.
API Speedups (Windows, PHP 8.6)Speedup = PHP time / Judy time. Bold = Judy faster.
Core Types (Windows, PHP 8.6)Speedup = PHP time / Judy time. Bold = Judy faster.
All types detailCore Types (Windows, PHP 8.6) — AllSpeedup = PHP time / Judy time. Bold = Judy faster.
Release Comparison (2.6.0 → 2.7.1)
Summary: 0 regressions, 100 unchanged
Memory ComparisonMemory: 0 regressions, 10 unchanged
Full benchmark outputLinux — PHP 8.6Windows — PHP 8.6 |
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.
What
Judy::__unserialize()now validates its payload strictly, rejecting anything__serialize()cannot produce:"Invalid serialization data for Judy array".optimizeIterationmust be a literalbool, not merely truthy.JTYPE, so the constructor-facingE_WARNINGin theJTYPEmacro no longer leaks out ofunserialize().optimizeIteration=trueon a type that cannot honour it throws"Invalid Judy type in serialized data".The optimizeIteration/type check runs before
judy_free_array_internal(), so a rejected payload leaves an already-populated object fully intact rather than half-freed and empty. It reusesjudy_type_can_mirror(), so the accepted set cannot drift from the constructor's own rule.Why round-trips are safe
__serialize()only emitsoptimizeIterationwhenintern->mirror_payloadis set, andjudy_set_optimize_iteration()only sets that for the two mirror-capable types (STRING_TO_INT_HASH,STRING_TO_INT_ADAPTIVE). A payload produced byserialize()can therefore never trip these checks — they only fire on hand-crafted, corrupt, or foreign payloads.Fixes over the draft
Three defects in the draft were corrected before landing:
Z_TYPE_P(ztype) != IS_LONGclause (copy-paste) removed.> 12, but the enum ends atTYPE_STRING_TO_ENTRY = 11; now enum-driven (type < TYPE_BITSET || type > TYPE_STRING_TO_ENTRY), so type 12 is correctly rejected.judy_free_array_internal(), destroying a populated object's contents on rejection; moved before it.Integer top-level keys are also rejected (the draft skipped them), for consistent strictness.
Tests
tests/serialize_error_001.phptis extended in place (same--TEST--title, so the test-floor junit identity ratchet stays intact — no test added or dropped). Added cases cover: unknown key, integer key, non-booloptimizeIteration, unhonourableoptimizeIteration, the type-12/type-99 warning-free boundary, object-intact-on-rejection, and the honouredoptimizeIterationroundtrip.Verification
ld/LTO warnings are pre-existing environment noise, confirmed on the unpatched tree).E_WARNING, and a rejected payload leaves a populated object intact (count=1 k=7 opt=true).