ProxyManager icon indicating copy to clipboard operation
ProxyManager copied to clipboard

ValueHolder proxy for class with accessing to private property within final method

Open kim-skyeng opened this issue 6 years ago • 15 comments

I have class with accessing to private property within final method like this:

class ClassWithFinalMethodAndPrivateProperty
{
    private $prop = 'privet';

    final public function finalMethod()
    {
        return $this->prop;
    }
}

I get Cannot access private property $prop error if I call finalMethod on valueHolder proxy of this class.

Is there such limitation for Value HolderProxy using or did I something wrong? Can't find any mentions about it in documentation or issues.

kim-skyeng avatar Jul 05 '19 21:07 kim-skyeng

Could you make an example of something triggering this, in the context of ProxyManager?

Ocramius avatar Jul 05 '19 21:07 Ocramius

I tried launch this code. It triggers error.

<?php

namespace App;

use ProxyManager\Factory\LazyLoadingValueHolderFactory;
use ProxyManager\Proxy\LazyLoadingInterface;

require_once __DIR__ . '/vendor/autoload.php';

$factory = new LazyLoadingValueHolderFactory();
$initializer = function (& $wrappedObject, LazyLoadingInterface $proxy, $method, array $parameters, & $initializer) {
    $initializer = null; // disable initialization
    $wrappedObject = new ClassWithFinalMethodAndPrivateProperty(); // fill your object with values here

    return true; // confirm that initialization occurred correctly
};

$proxy = $factory->createProxy(ClassWithFinalMethodAndPrivateProperty::class, $initializer);

echo $proxy->finalMethod();

kim-skyeng avatar Jul 05 '19 22:07 kim-skyeng

Interesting. I don't think that's fixable (the final method cannot be replaced). Do you have a stack trace for this?

Ocramius avatar Jul 05 '19 23:07 Ocramius

Trace:

PHP Fatal error:  Uncaught Error: Cannot access private property App\ClassWithFinalMethodAndPrivateProperty::$prop in ~/vendor/ocramius/proxy-manager/src/ProxyManager/GeneratorStrategy/EvaluatingGeneratorStrategy.php(70) : eval()'d code:96
Stack trace:
#0 ~/vendor/ocramius/proxy-manager/src/ProxyManager/GeneratorStrategy/EvaluatingGeneratorStrategy.php(70) : eval()'d code(101): ProxyManagerGeneratedProxy\__PM__\App\ClassWithFinalMethodAndPrivateProperty\Generated4fba263a26adebb7c3e20106c2b98b63->ProxyManagerGeneratedProxy\__PM__\App\ClassWithFinalMethodAndPrivateProperty\{closure}()
#1 ~/src/ClassWithFinalMethodAndPrivateProperty.php(11): ProxyManagerGeneratedProxy\__PM__\App\ClassWithFinalMethodAndPrivateProperty\Generated4fba263a26adebb7c3e20106c2b98b63->__get('prop')
#2 ~/src/a.php(20): App\ClassWithFinalMethodAndPrivateProperty->finalMethod()
#3 {main}

Next Error: Cannot access private property App\Class in ~/vendor/ocramius/proxy-manager/src/ProxyManager/GeneratorStrategy/EvaluatingGeneratorStrategy.php(70) : eval()'d code on line 96

I also tried to debug this code a bit and saw that php evaluate ClassWithFinalMethodAndPrivateProperty::finalMethod code with ProxyClass object as $this instead of call special inherited ClassWithFinalMethodAndPrivateProperty_123123::finalMethod like it does for non-final methods.

kim-skyeng avatar Jul 06 '19 01:07 kim-skyeng

Yeah, the issue here is that no initialisation can happen, so method calls cannot be redirected to the wrapped value.

Ocramius avatar Jul 06 '19 01:07 Ocramius

So it's pretty expected behavior, and ProxyManager users just should keep in mind that ValueHolder does not really good work with final methods?

kim-skyeng avatar Jul 06 '19 01:07 kim-skyeng

Pretty much. I could alter the behaviour in a major release, so that it throws during code generation (since final methods aren't proxied).

See #187 and #189

Ocramius avatar Jul 06 '19 01:07 Ocramius

Well I would like also to tell my background story of this case. Hope it could be helpfully for people who google this problem.

It happened when I use Symfony lazy service with final methods as described above. Symfony DI uses ValueHolder for lazy services so toggling off laziness fix the problem. Also code works fine when I declare property protected or make methods non final. Of course all final methods without accessing to private members could be evaluated too.

kim-skyeng avatar Jul 06 '19 01:07 kim-skyeng

A final method would be broken anyway, so I think that throwing an exception upfront is a wiser choice here...

Ocramius avatar Jul 06 '19 01:07 Ocramius

Do you mean ValueHolder should not support classes with final methods even if it can work until accessing private members?

kim-skyeng avatar Jul 06 '19 01:07 kim-skyeng

Nono, it will be broken even in case of public state.

Try proxying this:

class Counter
{
    public $count = 0;
    final public function increment () : int
    {
        return $this->count += 1;
    }
    public function decrement () : int
    {
        return $this->count -= 1;
    }
}

$counter = makeProxy(Counter::class);

var_dump($counter->increment());
var_dump($counter->increment());
var_dump($counter->decrement());

This should print:

1
2
-1

Ocramius avatar Jul 06 '19 01:07 Ocramius

Actually, it may even work, now that I think of it (because of the __get tricks I wrote), but still very broken due to split method behaviour: one method acts on the state of the proxy, the other acts on the state of the wrapped instance.

Ocramius avatar Jul 06 '19 01:07 Ocramius

Your code works okay. Upd. I meant your code with using proxy

1
2
1

But I see what do you mean, there is could be more problems with final methods. So it should either be fully supported or throws exception during code generation.

kim-skyeng avatar Jul 06 '19 01:07 kim-skyeng

Let's re-open here to track the BC break to be introduced

Ocramius avatar Jul 06 '19 02:07 Ocramius

This issue can be fixed by duplicating the logic that ghost objects use: unset all properties on the virtual proxy and forward all access to them to the target object.

nicolas-grekas avatar Aug 11 '22 12:08 nicolas-grekas