Skip to content

Commit

Permalink
bug #54194 [PropertyAccess] Fix checking for missing properties (nico…
Browse files Browse the repository at this point in the history
…las-grekas)

This PR was merged into the 6.4 branch.

Discussion
----------

[PropertyAccess] Fix checking for missing properties

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #48970, fix #54194, fix #51919
| License       | MIT

PropertyAccess and PropertyInfo make a few assumptions when magic methods are involved.
Namely they assume that if a `__get` method is defined, then any property name is accessible.
This assumption is generally wrong, and this becomes more evident with the introduction of lazy objects.
The linked issue gives some more details.

In this PR, I tweak these assumptions in order to make less of them.
Note that there is no bullet-proof way to decide if a *virtual* property exists in PHP. We're missing an `__exists()` magic method for that. Because of this, I'm submitting this PR to 6.4 and not 5.4. Let 5.4 end its life quietly and ensure 6.4 works at its best with lazy ghosts, where they're mainstream.

Commits
-------

9610a7c [PropertyAccess] Fix checking for missing properties
  • Loading branch information
nicolas-grekas committed Mar 13, 2024
2 parents ad56682 + 9610a7c commit dc4f3fb
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 9 deletions.
14 changes: 12 additions & 2 deletions src/Symfony/Component/PropertyAccess/PropertyAccessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,18 @@ private function readProperty(array $zval, string $property, bool $ignoreInvalid
throw $e;
}
} elseif (PropertyReadInfo::TYPE_PROPERTY === $type) {
if ($access->canBeReference() && !isset($object->$name) && !\array_key_exists($name, (array) $object) && !(new \ReflectionProperty($class, $name))->hasType()) {
throw new UninitializedPropertyException(sprintf('The property "%s::$%s" is not initialized.', $class, $name));
if (!isset($object->$name) && !\array_key_exists($name, (array) $object)) {
try {
$r = new \ReflectionProperty($class, $name);

if ($r->isPublic() && !$r->hasType()) {
throw new UninitializedPropertyException(sprintf('The property "%s::$%s" is not initialized.', $class, $name));
}
} catch (\ReflectionException $e) {
if (!$ignoreInvalidProperty) {
throw new NoSuchPropertyException(sprintf('Can\'t get a way to read the property "%s" in class "%s".', $property, $class));
}
}
}

$result[self::VALUE] = $object->$name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,9 @@ public function __get(string $property)
return 'constant value';
}
}

public function __isset(string $property)
{
return \in_array($property, ['magicProperty', 'constantMagicProperty'], true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -990,12 +990,19 @@ public function testGetValuePropertyThrowsExceptionIfUninitializedWithLazyGhost(

public function testGetValueGetterThrowsExceptionIfUninitializedWithLazyGhost()
{
$lazyGhost = $this->createUninitializedObjectPropertyGhost();

$this->expectException(UninitializedPropertyException::class);
$this->expectExceptionMessage('The property "Symfony\Component\PropertyAccess\Tests\Fixtures\UninitializedObjectProperty::$privateUninitialized" is not readable because it is typed "DateTimeInterface". You should initialize it or declare a default value instead.');

$this->propertyAccessor->getValue($lazyGhost, 'privateUninitialized');
}

public function testIsReadableWithMissingPropertyAndLazyGhost()
{
$lazyGhost = $this->createUninitializedObjectPropertyGhost();

$this->propertyAccessor->getValue($lazyGhost, 'privateUninitialized');
$this->assertFalse($this->propertyAccessor->isReadable($lazyGhost, 'dummy'));
}

private function createUninitializedObjectPropertyGhost(): UninitializedObjectProperty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,12 @@ public function getReadInfo(string $class, string $property, array $context = []
return new PropertyReadInfo(PropertyReadInfo::TYPE_METHOD, $getsetter, $this->getReadVisiblityForMethod($method), $method->isStatic(), false);
}

if ($allowMagicGet && $reflClass->hasMethod('__get') && ($reflClass->getMethod('__get')->getModifiers() & $this->methodReflectionFlags)) {
return new PropertyReadInfo(PropertyReadInfo::TYPE_PROPERTY, $property, PropertyReadInfo::VISIBILITY_PUBLIC, false, false);
if ($allowMagicGet && $reflClass->hasMethod('__get') && (($r = $reflClass->getMethod('__get'))->getModifiers() & $this->methodReflectionFlags)) {
return new PropertyReadInfo(PropertyReadInfo::TYPE_PROPERTY, $property, PropertyReadInfo::VISIBILITY_PUBLIC, false, $r->returnsReference());
}

if ($hasProperty && ($reflClass->getProperty($property)->getModifiers() & $this->propertyReflectionFlags)) {
$reflProperty = $reflClass->getProperty($property);

return new PropertyReadInfo(PropertyReadInfo::TYPE_PROPERTY, $property, $this->getReadVisiblityForProperty($reflProperty), $reflProperty->isStatic(), true);
if ($hasProperty && (($r = $reflClass->getProperty($property))->getModifiers() & $this->propertyReflectionFlags)) {
return new PropertyReadInfo(PropertyReadInfo::TYPE_PROPERTY, $property, $this->getReadVisiblityForProperty($r), $r->isStatic(), true);
}

if ($allowMagicCall && $reflClass->hasMethod('__call') && ($reflClass->getMethod('__call')->getModifiers() & $this->methodReflectionFlags)) {
Expand Down

0 comments on commit dc4f3fb

Please sign in to comment.