Le sam. 4 juil. 2026 à 10:47, Nicolas Grekas <[email protected]> a écrit :
> Hi, > > thanks for having a look > > Le lun. 15 juin 2026 à 21:30, Tim Düsterhus <[email protected]> a écrit : > >> Hi >> >> On 6/10/26 19:02, Nicolas Grekas wrote: >> > I'd like to open the discussion on a new RFC: >> > https://wiki.php.net/rfc/serializable_closures >> >> Don't forget to link the discussion thread in the RFC. For your >> convenience: https://news-web.php.net/php.internals/131198 >> >> As to the RFC itself: I think it might be useful to split this into two >> RFCs, similarly to how Volker and I split the initial support for >> Closures in const-expr into support for Closures and support for first >> class callables. >> > > The 8.5 split worked because closures and FCCs were separable features. > Here it's one mechanism, the FCC half alone or the anonymous-closure one > alone would be just missing its other half. > I'd keep it as one RFC. > > > >> Specifically I believe that the proposed serialization format is fragile >> and the stated security model, which I suspect heavily influenced the >> design of the serialization format, is probably overly cautious. >> >> I don't think it is a problem to make `unserialize()` a Closure factory, >> because the created Closure is “inert”. Contrary to arbitrary object >> unserialization (which will immediately call the deserialization hooks >> and then later __destruct()), the Closure will not actually do anything >> unless it is called. >> >> If folks are able to unserialize arbitrary payloads - which is >> documented to be unsafe - they already have capabilities that are much >> more powerful than “creating Closures”. >> > > Creation is inert, but the point of these payloads is to be called. Once > it's called, the only thing that matters is which callables a payload can > name. > Declared-set resolution: an attacker can swap one closure the class > already declares for another. > Unrestricted name-based: {function: "system"} with attacker-supplied args > is a call gadget shipped in the engine, against any app that unserializes > untrusted input. > "allowed_classes" exists and 8.6 tightened session defaults precisely > because we bound damage despite unserialize being documented unsafe. > So I'd keep the declared-set boundary as required defense-in-depth / > hardening. > > > >> For first class callables specifically, there is a very obvious and >> stable identifier to use: The underlying function name. >> >> For regular Closures, I don't like how changing something unrelated >> (namely adding new Closures to a class) can change the meaning of the >> serialized data. This is not how serialization works right now: The >> value in question is in full control of its serialization format and is >> able to “gracefully” support existing serialized payloads using an old >> format within an unserialization hook (e.g. by including a version >> number field). This allows to e.g. keep serialized queue jobs working >> across deploys. >> > > Agreed: first-class callables now serialize with the function name as > identifier, no ordinal involved. The id is `member@callable`, e.g. > `$billingAddress@Order::isStrict` for `#[When(self::isStrict(...))]` on > that property, `$p@strlen` for a plain function. The member prefix keeps > resolution local to one reflection element instead of scanning the whole > class on every cache read (fat classes would pay otherwise), and it gives > both closure forms the same staleness rule: a reference is valid while its > member and its declaration survive. Adding, removing or reordering anything > else in the class changes nothing; renaming the target method fails. > > One catch: the idiomatic form references a private helper of the same > class, #[When(self::isStrict(...))], and Closure::fromCallable('C::priv') > from global scope throws "cannot access private method". So resolution > doesn't resolve the name directly: it checks if the named member declares > that exact reference, then evaluates the declaration in class scope, name > as address, declaration as guard. > Private keeps working, {..., "$p@system"} stays rejected because no class > declares it. > > > >> As for the var_export() in the future scope: I think adding support for >> first class callables to `var_export()` would be a change that can just >> be done without an RFC. It might be a good first step that might already >> be helpful to your use case? >> > > Not really helpful for the main use cases I gathered, which require full > compat with serialize semantics, but yeah, I agree this can be dealt with > on its own. > > RFC updated! > I tried to reflect all your points in the update. > Side-note to all: I'd also be fine with a limited version of this RFC that'd remove the serialize-related part and that'd keep only the proposed Reflection-based API. This is the very core where engine support is needed. The serialize part would make attributes work seamlessly with backends that use serialize(), but my use cases build on the deepclone/VarExporter extension/components, and those need only reflection. In case that can help bring a broader consensus. Nicolas
