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

Fix accessibility checks for dynamic properties #3626

Closed
wants to merge 1 commit into from

Conversation

nikic
Copy link
Member

@nikic nikic commented Oct 20, 2018

A dynamic property may be shadowed by a private/protected property. Make sure we check property accessibility for non-indirect properties as well. Previously mentioned in #3573 (comment).

There is one more caveat here, in that access checks for the dynamic property case should also treat mangled names as accessible (per existing behavior). This is why I added the is_dynamic flag to check_property_access.

@petk petk added the Quickfix label Oct 20, 2018
A dynamic property may be shadowed by a private/protected property.
Make sure we check property accessibility for non-indirect
properties as well.
@nikic
Copy link
Member Author

nikic commented Oct 20, 2018

@dstogov Can you please check this? I don't like that we have to check access for dynamic properties, but I don't really see a better way.

@nikic nikic added Bug and removed Quickfix labels Oct 20, 2018
@dstogov
Copy link
Member

dstogov commented Oct 22, 2018

I think, this is OK.

@php php deleted a comment from dstogov Oct 22, 2018
@php php deleted a comment from dstogov Oct 22, 2018
@php php deleted a comment from dstogov Oct 22, 2018
@php-pulls php-pulls closed this in 149e6aa Oct 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants