first class callable syntax support in _wp_filter_build_unique_id() / remove_filter()

[jave.web]

PHP 8.1 introduced something called "First class callable" or First-class Callable Syntax - ​https://www.php.net/releases/8.1/en.php#first_class_callable_syntax

Which basically means much easier way to reference any function, especially class methods - both dynamic and static like so:

$this->myDynamicCallback(...);
self::myStaticCallback(...);

The 3 dots "..." are not omission - it is the syntax

Now this syntax actually works for add_filter however, it can fail (and I've encountered cases where it fails) for remove_filter which leads to very unexpected results.

Temporary bypass can be done using remove_all_filters or the old-school syntax [__CLASS__, 'myStaticCallback'].

The core issue is in the function _wp_filter_build_unique_id which generates the key under which the callback is stored, more specifically, in the way how the PHP's spl_object_hash work - this sentence from docs "This id can be used as a hash key for storing objects, or for identifying an object, as long as the object is not destroyed."

Now this is an educated wild guess, but I would guess that the callable objects created by the First-class callable syntax may not live that long and garbage collector can destroy them if it sees fit.

Anyways... the core is - spl_object_hash is NOT the right tool to cache ALL callables, but can only be used where the callable is passed around (e.g. stored in a variable) and therefore it is ensured it's still the same object (I would hope).

Solution
...for such cases is to create a \ReflectionFunction instance and check for getClosureScopeClass() and getName() :-)

​https://www.php.net/manual/en/class.reflectionfunction.php
​https://www.php.net/manual/en/reflectionfunctionabstract.getclosurescopeclass.php
​https://www.php.net/manual/en/reflectionfunctionabstract.getname.php

[swissspidy]

#59129 was marked as a duplicate.

[jkfoiztmcjeikfp]

@swissspidy thanks for handling my duplicate.

I think my ticket contributed the additional point, that add_filterand add_action also behave differently, in cases where plugins/themes just carelessly call them multiple times to rely on their idempotent behaviour.

I am new to contributing, do you think a pull request contributing, what @javeweb and I independently suggested, has the chance to be merged or should I wait for the ticket to be assigned a different status?

[jave.web]

If you have a time to create a PR, go ahead! Even if it is not accepted, it can inspire the final developer :-) I think that the point from your ticket is implied by description in this one - and it's even worse - because spl_object_hash also says "Once the object is destroyed, its hash may be reused for other objects." Now this could potentially lead to security issues where other filters could be removed than the ones intended.

[jrf]

Linking to ticket #46635 for visibility. Ticket #46635 is about supporting removable closures as callbacks and, to be honest, I'm inclined to mark this ticket as a duplicate of that one as any solution for that ticket should take first class callables into account as well.

These tickets should have a coordinated joint solution.