flow-development-collection icon indicating copy to clipboard operation
flow-development-collection copied to clipboard

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

Open mhsdesign opened this issue 3 years ago • 5 comments

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

  • 'offsetGet propertyA'
  • // nothing here
  • '__call callableA'

Actual behavior

following var_dumps

  • '__call getPropertyA'
  • '__call getNonExistantProperty'
  • '__call callableA'

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 avatar Mar 28 '22 13:03 mhsdesign

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

mhsdesign avatar Jul 04 '22 06:07 mhsdesign

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 😂

kitsunet avatar Jul 04 '22 07:07 kitsunet

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.

mhsdesign avatar Jul 06 '22 11:07 mhsdesign

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

kdambekalns avatar Jul 06 '22 11:07 kdambekalns

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

mhsdesign avatar Jul 06 '22 11:07 mhsdesign