Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix assign cache 'this' context #27

Merged
merged 1 commit into from
Oct 17, 2022
Merged

Conversation

BesedinSasha
Copy link

This PR resolves bug with '$this' context. For some reason after getting closure from static cacheAssign variable and calling accessor, $this context doesn't change. I assumed that reson is using $this in closure instead of $instance

Part of proxy class code from callInitializer method

private function callInitializer0dc81($methodName, array $parameters)
    {
// hidden lines of code
        static $cacheAssignFoo;

        $cacheAssignFoo ?? $cacheAssignFoo = \Closure::bind(function ($instance, $nonReferenceableProperties) { // $instance is not using anywhere
            isset($nonReferenceableProperties->id_on_Foo) && $this->id = $nonReferenceableProperties->id_on_Foo;
            isset($nonReferenceableProperties->name_on_Foo) && $this->name = $nonReferenceableProperties->name_on_Foo;
        }, $this, 'Foo');

        $cacheAssignFoo($this, $nonReferenceableProperties); // after calling cached closure we have $this context without $id and $name properties

// hidden lines of code
}

Code that reproduces this bug:

<?php

declare(strict_types=1);

use ProxyManager\Factory\LazyLoadingGhostFactory;
use ProxyManager\Proxy\GhostObjectInterface;

require_once 'vendor/autoload.php';

class Foo
{
    private int $id;

    private string $name;

    public function __construct(int $id, string $name)
    {
        $this->id = $id;

        $this->name = $name;
    }

    public function getIt(): int
    {
        return $this->id;
    }

    public function setId(int $id): void
    {
        $this->id = $id;
    }

    public function getName(): string
    {
        return $this->name;
    }

    public function setName(string $name): void
    {
        $this->name = $name;
    }
}

$proxyFactory = new LazyLoadingGhostFactory();

function createInitializer(int $num)
{
    return function (
        GhostObjectInterface|Foo $proxy,
        string $method,
        array $parameters,
        ?Closure &$initializer,
        array $properties
    ) use ($num) {
        $initializer = null;

        $properties["\0".Foo::class."\0".'id'] = $num;
        $properties["\0".Foo::class."\0".'name'] = "Name_$num";

        return true;
    };
}

$foo1 = $proxyFactory->createProxy(Foo::class, createInitializer(1));

echo $foo1->getName().PHP_EOL;

$foo2 = $proxyFactory->createProxy(Foo::class, createInitializer(2));

echo $foo2->getName().PHP_EOL; // Fatal error: Uncaught Error: Cannot access uninitialized non-nullable property Foo::$name

@nicolas-grekas
Copy link
Collaborator

Good catch thanks.

@nicolas-grekas nicolas-grekas merged commit 5ca79bc into FriendsOfPHP:1.x Oct 17, 2022
@BesedinSasha
Copy link
Author

@nicolas-grekas 👍

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.

2 participants