Gate closures over named callables behind an allow_named_closures option#26
Merged
Conversation
A by-name closure payload (a first-class callable over a named function or method) lets deepclone_from_array() mint a Closure over any function or method of that name, with no visibility or staticness restriction and including internal functions such as system() -- a stronger primitive than anything unserialize() exposes, and a one-step gadget when paired with an object whose __wakeup/__unserialize/destructor invokes a stored callback. Both deepclone_to_array() and deepclone_from_array() gain a third parameter, bool $allow_named_closures = false, that both ends must enable: with it off (the default) to_array refuses to encode such a closure, and from_array rejects any payload carrying a by-name closure marker before instantiating anything. So the attribute-cache use case does not regress, first-class callables over a method of their own declaring class declared in a constant expression (e.g. #[When(self::isStrict(...))]) are now encoded as a declaration-site reference (mask LONG 1), like anonymous const-expr closures, instead of by name. They resolve only to a closure the named class itself declares and round-trip without the opt-in. References whose declaring class cannot be derived from the closure (cross-class or global-function callables, inherited methods, runtime-created callables) keep the by-name form and need the opt-in. allowed_classes still gates Closure for both forms.
nicolas-grekas
added a commit
to symfony/polyfill
that referenced
this pull request
Jun 11, 2026
… allow_named_closures option (nicolas-grekas) This PR was merged into the 1.x branch. Discussion ---------- [DeepClone] Gate closures over named callables behind an allow_named_closures option | Q | A | ------------- | --- | Branch? | 1.x | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT A by-name closure payload (a first-class callable over a named function or method, e.g. `strlen(...)` or `Cls::method(...)`, and `Closure::fromCallable()`) makes `deepclone_from_array()` a factory for a `Closure` over any function or method of that name -- with no visibility or staticness restriction and including internal functions such as `system()`. That is a stronger primitive than anything `unserialize()` exposes, and a one-step gadget when paired with an object whose `__wakeup`/`__unserialize`/destructor invokes a stored callback. `deepclone_to_array()` and `deepclone_from_array()` gain a third parameter, `bool $allowNamedClosures = false`, that **both ends must enable**. With it off (the default) `to_array` refuses to encode such a closure and `from_array` rejects any payload carrying a by-name closure marker before instantiating anything. This is a behavior change: closures over named callables previously serialized and resolved unconditionally. - **The attribute-cache use case does not regress.** First-class callables over a method of their own declaring class declared in a constant expression (e.g. `#[Assert\When(self::isStrict(...))]`, symfony/symfony#63228) are now encoded as a declaration-site reference (mask `1`), like anonymous const-expr closures, instead of by name; they resolve only to a closure the named class itself declares and round-trip **without** the opt-in. Userland identifies them by the target method's reflection key over the existing per-class const-expr index. - **What still needs the opt-in.** Cross-class or global-function callables, inherited methods, and any first-class callable created at runtime keep the by-name form and require `$allowNamedClosures` on both ends. - **`allowed_classes` is unchanged** and still gates `Closure` for both forms. Payloads stay byte-identical and interchangeable with the extension (verified both directions). Tests cover the const-expr routing, both refusals, cross-end enforcement, a `system()` payload rejected by default, and wholesale rejection of a nested by-name closure; existing named-closure tests pass the opt-in. Companion extension implementation: symfony/php-ext-deepclone#26 Commits ------- 3c0c20c [DeepClone] Gate closures over named callables behind an allow_named_closures option
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.
A by-name closure payload (a first-class callable over a named function or method, e.g.
strlen(...)orCls::method(...)) letsdeepclone_from_array()mint aClosureover any function or method of that name, with no visibility or staticness restriction and including internal functions such assystem(). That is a stronger primitive than anythingunserialize()exposes: the closure is the sink, so a payload that pairs it with an object whose__wakeup/__unserialize/destructor invokes a stored callback is a one-step gadget.Both
deepclone_to_array()anddeepclone_from_array()gain a third parameter,bool $allow_named_closures = false, that both ends must enable. With it off (the default),deepclone_to_array()refuses to encode such a closure anddeepclone_from_array()rejects any payload carrying a by-name closure marker before instantiating anything. Enable it only between ends that trust each other.#[Assert\When(self::isStrict(...))], Disable static callable in attribute as it's not cacheable symfony#63228) are now encoded as a declaration-site reference (maskLONG 1), like anonymous const-expr closures, instead of by name. They resolve only to a closure the named class itself declares and round-trip without the opt-in. Identification reuses the existing const-expr machinery: a first-class callable shares its target method's op_array, so re-evaluating the declaration site yields an equivalent closure, matched by the same pointer walk.Validators::check(...)) or global-function callables, inherited methods, internal functions, and any first-class callable created at runtime -- keep the by-name form and requireallow_named_closureson both ends.allowed_classesis unchanged and still gatesClosurefor both forms; omit it and no closure resolves at all.Tests: a new
.phptcovers the const-expr routing of attribute first-class callables, theto_array/from_arrayrefusals, cross-end enforcement, a hostilesystem()payload rejected by default, wholesale rejection of a by-name closure nested in an object graph, andallowed_classesinteraction. The existing closure/lazy/allowed-classes tests are updated to pass the opt-in where they exercise runtime named closures.Companion polyfill implementation: symfony/polyfill#634