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

ObjectAccess with direct access fails on private properties of proxied classes #2388

Closed
kdambekalns opened this issue Feb 17, 2021 · 2 comments
Assignees
Labels

Comments

@kdambekalns
Copy link
Member

kdambekalns commented Feb 17, 2021

Description

Classes with private properties do not react as epxected to ObjectAccess::setProperty() with $forceDirectAccess enabled.

Steps to Reproduce

Have a class wirh a private property that is proxied for DI/AOP purposes. You end up with a structure like this:

class SomeClass_Original {
    private $property = 'initial';
    public function showProperty() {
        echo $this->property;
    }
}
class SomeClass extends SomeClass_Original {
    // DI and AOP code built by Flow
}

$subject = new SomeClass();
  1. ObjectAccess::setProperty($subject, 'changed', true)
  2. $subject->showProperty

Expected behavior

Property is set in "original" class and output is: changed

Actual behavior

Property is set in proxy class and output is: initial

Affected Versions

Flow: effectively all, the code in question is unchanged for the last 6 years.

Possible solution

Change

$className = TypeHandling::getTypeForValue($subject);
if (property_exists($className, $propertyName)) {
$propertyReflection = new \ReflectionProperty($className, $propertyName);
$propertyReflection->setAccessible(true);
$propertyReflection->setValue($subject, $propertyValue);
} else {
$subject->$propertyName = $propertyValue;
}
to

$className = TypeHandling::getTypeForValue($subject);
if (property_exists($className, $propertyName)) {
    $propertyReflection = new \ReflectionProperty($className, $propertyName);
    $propertyReflection->setAccessible(true);
    $propertyReflection->setValue($subject, $propertyValue);
} elseif (substr_compare(get_parent_class($className), '_Original', -9) === 0 && property_exists(get_parent_class($className), $propertyName)) {
    $propertyReflection = new \ReflectionProperty(get_parent_class($className), $propertyName);
    $propertyReflection->setAccessible(true);
    $propertyReflection->setValue($subject, $propertyValue);
} else {
    $subject->$propertyName = $propertyValue;
}

Question – do we want this?

@bwaidelich
Copy link
Member

I don't think that this change would break anything so +1 for the idea.

The hard-coded '_Original' should probably replaced with Neos\Flow\ObjectManagement\Proxy\Compiler::ORIGINAL_CLASSNAME_SUFFIX though.
Or even with sth like:

} elseif ($subject instanceof ProxyInterface) {
...

@kdambekalns
Copy link
Member Author

The hard-coded '_Original' should probably replaced with Neos\Flow\ObjectManagement\Proxy\Compiler::ORIGINAL_CLASSNAME_SUFFIX though.

Right, forgot about that one!

Or even with sth like:

} elseif ($subject instanceof ProxyInterface) {
...

Probably even better, indeed.

kdambekalns added a commit to kdambekalns/flow-development-collection that referenced this issue Feb 18, 2021
…ied classes

With this classes with private properties do react as expected to
ObjectAccess::getProperty() and ObjectAccess::setProperty() with
$forceDirectAccess enabled, even when they have been subclassed by the
proxy building of Flow.

Fixes neos#2388
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants