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

[PropertyAccess][VarExporter] PropertyAccessor::isReadable() triggers an error when called on a lazy object’s missing property #51919

Closed
MatTheCat opened this issue Oct 9, 2023 · 3 comments

Comments

@MatTheCat
Copy link
Contributor

MatTheCat commented Oct 9, 2023

Symfony version(s) affected

≥ 6.2

Description

Calling PropertyAccessor::isReadable() on a lazy object’s missing property will trigger an error, even if the object is fully loaded.

This is because PropertyAccessor::getReadInfo() will return the property as public as its class has a __get method, and trying to access it will call trigger_error:

trigger_error(sprintf('Undefined property: %s::$%s in %s on line %s', $this::class, $name, $frame['file'], $frame['line']), \E_USER_NOTICE);

How to reproduce

Call PropertyAccessor::isReadable() on a lazy object’s (like a Doctrine proxy) missing property. Instead of false, you’ll get an ErrorException.

Possible Solution

No response

Additional Context

No response

@nicolas-grekas
Copy link
Member

Could you please submit a PR with a failing test case? That'd help a lot here. 🙏

@nicolas-grekas
Copy link
Member

Possibly fixed by #54194, please confirm if you can.

@MatTheCat
Copy link
Contributor Author

I confirm your PR makes #54068’s test pass 🎉 Guess I can close it!

nicolas-grekas added a commit that referenced this issue Mar 13, 2024
…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
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

4 participants