test: add failing tests for #126 — objects with __serialize and closure array properties#127
Draft
esetnik wants to merge 2 commits intolaravel:2.xfrom
Draft
Conversation
…y properties Objects implementing __serialize that have closures in array properties (or other non-typed-Closure properties) fail to serialize since v2.0.9. The wrapClosures/mapByReference change in laravel#122 skips all objects with __serialize, so nested closures are never wrapped with Native instances. When PHP later calls __serialize() on these objects, the raw Closure values pass through and hit "Serialization of 'Closure' is not allowed". These tests cover: - Closure capturing such an object via use variable - Closure bound to such an object via $this - Round-trip preserving closure array values All 9 tests (3 cases x 3 serializers) currently fail. Ref: laravel#126 Co-authored-by: Cursor <cursoragent@cursor.com>
…d closure array properties Objects implementing __serialize that have closures in array properties (or other non-typed-Closure properties) fail to serialize since v2.0.9. The wrapClosures/mapByReference change in laravel#122 skips all objects with __serialize, so nested closures are never wrapped with Native instances. When PHP later calls __serialize() on these objects, the raw Closure values pass through and hit "Serialization of 'Closure' is not allowed". These tests cover: - Closure capturing such an object via use variable - Closure bound to such an object via $this - Round-trip preserving closure array values All 9 tests (3 cases x 3 serializers) currently fail. Also adds the existing TypedClosurePropertyTest and the new test to phpunit.xml.dist so they are actually run in CI (previously both were missing from the explicit file list). Ref: laravel#126 Co-authored-by: Cursor <cursoragent@cursor.com>
Member
|
Any suggestions on fix? |
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
2 participants
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
Adds failing test cases for #126 — objects implementing
__serializethat have closures in array properties (or other non-typed-Closureproperties) fail to serialize since v2.0.9.Tests added
closure use variable referencing object with __serialize and closure array propertyuseclosure bound to object with __serialize and closure array property$thisclosure use variable referencing object with __serialize preserves closure array valuesAll 9 tests (3 cases × 3 serializers: Native, Signed, Unsigned) currently fail with
Serialization of 'Closure' is not allowed.phpunit.xml.distfixThe new test file and the existing
TypedClosurePropertyTest.php(added in #122) were both missing fromphpunit.xml.dist, which uses explicit<file>entries rather than directory scanning. This meant those tests were never executed in CI. This PR adds both to a newserializer-regressiontest suite in the config.Root cause
The
wrapClosures()/mapByReference()change in #122 skips all objects with__serialize, so nested closures inside those objects are never wrapped withNativeinstances. When PHP later calls__serialize(), the raw\Closurevalues pass through and hit PHP's native serialization restriction.Note: this is distinct from the
TypedClosurePropertyTestcases which pass becauseClassWithTypedClosurePropertymanually wraps its closure inSerializableClosureinside__serialize(). The new fixture (ClassWithSerializeAndClosureArrayProperty) does not manually wrap — it just returns its properties as-is, which is the common case for user-defined classes that aren't aware oflaravel/serializable-closure.Ref: #126