neos / flow-development-collection

The unified repository containing the Flow core packages, used for Flow development.
https://flow.neos.io/
MIT License
137 stars 188 forks source link

BUG: EEL/`ObjectAccess::getProperty` doenst work on object with `__call` and `ArrayAccess` #2785

Open mhsdesign opened 2 years ago

mhsdesign commented 2 years ago

Description

EEL uses ObjectAccess::getProperty https://github.com/neos/flow-development-collection/blob/1956920c41b5eb9348daaaf03543efaa45b098b2/Neos.Eel/Classes/Context.php#L65

when you put into ObjectAccess::getProperty an object with magic __call and ArrayAccess it will check first if the property has an getter before it tries to use ArrayAccess offsetExists and offsetGet.

This order is okay unless one implements the magic __call method. Then the getter check done via is_callable on the $object will return true for any input. https://github.com/neos/flow-development-collection/blob/a340ca830356e80f71bcfaebfd983b512d362133/Neos.Utility.ObjectHandling/Classes/ObjectAccess.php#L170

so this magic method will be called - although there is a working ArrayAccess for this property.

Steps to Reproduce

have a php object like:

class ObjectWithCallAndArrayAccess implements \ArrayAccess, ProtectedContextAwareInterface
{
    // only for demo
    public static function create()
    {
        return new self();
    }

    public function offsetExists($offset)
    {
        return in_array($offset, ['propertyA', 'propertyB']);
    }

    public function offsetGet($offset)
    {
        \Neos\Flow\var_dump('offsetGet ' . $offset);
        die();
    }

    public function __call(string $name, array $arguments)
    {
        \Neos\Flow\var_dump('__call ' . $name);
        die();
    }

    public function allowsCallOfMethod($methodName)
    {
        return in_array($methodName, ['callableA', 'callableB']);
    }

    public function offsetSet($offset, $value) { throw new \BadMethodCallException('Not supported.'); }
    public function offsetUnset($offset) { throw new \BadMethodCallException('Not supported.'); }
}

make it available for testing in eel by retrieving it via Helper. or via:

Neos:
  Fusion:
    defaultContext:
      getFancyObject: ObjectWithCallAndArrayAccess::create

run:

root = ${getFancyObject().propertyA}
root = ${getFancyObject().nonExistantProperty}
root = ${getFancyObject().callableA()}

Expected behavior

following var_dumps

Actual behavior

following var_dumps

Affected Versions

Flow: 5.3

Solution?

might be breaking, but move the ArrayAccess check before trying to use any getters. https://github.com/neos/flow-development-collection/blob/a340ca830356e80f71bcfaebfd983b512d362133/Neos.Utility.ObjectHandling/Classes/ObjectAccess.php#L147

mhsdesign commented 2 years ago

linking this: https://neos-project.slack.com/archives/C050KKBEB/p1656766553350719?thread_ts=1656766553.350719&cid=C050KKBEB

kitsunet commented 2 years ago

Sounds as it could be pretty breaking indeed. To me a case of rather won't fix and fix the edge case on the other side? What kind of nasty object is this 😂

mhsdesign commented 2 years ago

Yes pretty nasty - i was just levering every php trick ^^

much more critical is, that call and public object properties cannot co-exist this way as @nezaniel pointed out in the slack thread above. Im for fixing this like suggested: use call only as last resort.

Current workaround would be: in the __call strip the get part of the methodName and make the first letter lower case, then proceed as you want: eg return a public property or similar.

kdambekalns commented 2 years ago

I wonder what kind of real-life object has ArrayAccess and __call() implemented together…

mhsdesign commented 2 years ago

No one. But some proxy object for EEL use only might.