Skip to content

[PropertyAccess] [Bug] isReadable return true for an array index even if not set. #18969

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

Closed
chalasr opened this issue Jun 5, 2016 · 9 comments

Comments

@chalasr
Copy link
Member

chalasr commented Jun 5, 2016

Description

Let's take the following empty array:

$array = array();

And the following empty class:

$object = new class {};

Check if an undefined property is readable from the class:

$accessor = PropertyAccess::createPropertyAccessor();
$accessor->isReadable($object, 'foo'); // false

Same check on the array:

$accessor = PropertyAccess::createPropertyAccessor();
$accessor->isReadable($array, '[foo]'); // true

As $array['foo'] doesn't exist, I assume it's a bug, right?

Possible solution

In PropertyAccessor::readIndex, in the following condition:

if (isset($zval[self::VALUE][$index])) {
    $result[self::VALUE] = $zval[self::VALUE][$index];

    if (!isset($zval[self::REF])) {
        // Save creating references when doing read-only lookups
    } elseif (is_array($zval[self::VALUE])) {
        $result[self::REF] = &$zval[self::REF][$index];
    } elseif (is_object($result[self::VALUE])) {
        $result[self::REF] = $result[self::VALUE];
    }
}

Add an else statement like:

if (isset($zval[self::VALUE][$index])) {
    // ...
} else {
    throw new NoSuchIndexException(...);
}

Sort as isReadable properly returns false on unreadable index.

@chalasr
Copy link
Member Author

chalasr commented Jun 5, 2016

If the bug is confirmed, I have a ready patch.

@sstok
Copy link
Contributor

sstok commented Jun 5, 2016

When you call getValue() does it give an exception in this case? (non-existing key).
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php#L103-L104

@chalasr
Copy link
Member Author

chalasr commented Jun 5, 2016

@sstok, for $array it gives NULL, for $object it throws an exception saying that no getter/isser/hasser found.

@sstok
Copy link
Contributor

sstok commented Jun 5, 2016

OK, so it's directly confirmable. Hmm..

@chalasr chalasr changed the title [PropertyAccess] isReadable return true for an array index even if not set. [PropertyAccess] [Bug] isReadable return true for an array index even if not set. Jun 5, 2016
@chalasr
Copy link
Member Author

chalasr commented Jun 5, 2016

@sstok It seems that tests expect this behavior. That sounds strange to me, seems that for you too.

@chalasr
Copy link
Member Author

chalasr commented Jun 6, 2016

After investigation, it's the expected behavior. The second argument of PropertyAccessor::__construct() determines if exceptions should be thrown or not, if it is set to true, isReadable properly returns false.
I'll close this issue, the attached PR #18970 , then open a new PR to allow passing this argument in PropertyAccess::createPropertyAccessor (e.g. createPropertyAccessor($throwExceptionOnInvalidIndex = false)).

@khusseini
Copy link

May I ask how this was decided? I find it a bit confusing that the exceptions differ from objects and arrays.

@xabbuh
Copy link
Member

xabbuh commented Aug 9, 2017

There was nothing to change as this is the expected behaviour (accessing non-existing array keys just returns null, see http://symfony.com/doc/current/components/property_access.html#reading-from-arrays).

@Vorotyagin-Anton
Copy link

Vorotyagin-Anton commented Apr 12, 2022

It can be useful with multidimensional array.

$array = array();
$accessor->isReadable($array, '[foo]'); // true
$accessor->isReadable($array, '[foo][bar]'); // false

Because

$array = array();
$accessor->getValue($array, '[foo]'); // null
$accessor->getValue($array, '[foo][bar]'); // Exception\UnexpectedTypeException - "PropertyAccessor requires a graph of objects or arrays to operate on, but it found type "NULL" while trying to traverse path "[foo][bar]" at property "bar"."

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

No branches or pull requests

7 participants