Skip to content

Gate closures over named callables behind an allow_named_closures option#26

Merged
nicolas-grekas merged 1 commit into
mainfrom
allow-named-closures
Jun 11, 2026
Merged

Gate closures over named callables behind an allow_named_closures option#26
nicolas-grekas merged 1 commit into
mainfrom
allow-named-closures

Conversation

@nicolas-grekas

@nicolas-grekas nicolas-grekas commented Jun 11, 2026

Copy link
Copy Markdown
Member

A by-name closure payload (a first-class callable over a named function or method, e.g. strlen(...) or Cls::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(). That is a stronger primitive than anything unserialize() 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() and deepclone_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 and deepclone_from_array() rejects any payload carrying a by-name closure marker before instantiating anything. Enable it only between ends that trust each other.

  • 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(...))], Disable static callable in attribute as it's not cacheable symfony#63228) 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. 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.
  • What still needs the opt-in. References whose declaring class cannot be derived from the closure -- cross-class (Validators::check(...)) or global-function callables, inherited methods, internal functions, and any first-class callable created at runtime -- keep the by-name form and require allow_named_closures on both ends.
  • allowed_classes is unchanged and still gates Closure for both forms; omit it and no closure resolves at all.

Tests: a new .phpt covers the const-expr routing of attribute first-class callables, the to_array/from_array refusals, cross-end enforcement, a hostile system() payload rejected by default, wholesale rejection of a by-name closure nested in an object graph, and allowed_classes interaction. 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

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
@nicolas-grekas nicolas-grekas merged commit a6d3f67 into main Jun 11, 2026
20 checks passed
@nicolas-grekas nicolas-grekas deleted the allow-named-closures branch June 11, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant