Everything

php

[RFC] Pass Scope to Magic Accessors

php.internals

Nicolas Grekas

3 years ago

Hi internals,

Ilija and I would like to start a discussion about the following RFC:
https://wiki.php.net/rfc/pass_scope_to_magic_accessors

When using magic methods to access actual properties, respecting their
declared visibility is often desired. Yet, accessing the calling scope to
emulate the visibility restrictions is unreasonably difficult at the
moment. This RFC proposes to pass the calling scope to magic accessors to
make it trivial to get it.

Kind regards,
Nicolas

Tim Düsterhus

3 years ago

Hi

On 1/19/23 17:23, Nicolas Grekas wrote:

> Ilija and I would like to start a discussion about the following RFC:
> https://wiki.php.net/rfc/pass_scope_to_magic_accessors
>
> When using magic methods to access actual properties, respecting their
> declared visibility is often desired. Yet, accessing the calling scope to
> emulate the visibility restrictions is unreasonably difficult at the
> moment. This RFC proposes to pass the calling scope to magic accessors to
> make it trivial to get it.
>

Unless I missed anything, the RFC does not explain how the values that
are passed into the second parameter will look like. What kinds of
strings will be passed? When will the value be null?

Best regards
Tim Düsterhus

Ilija Tovilo

3 years ago

Hi Tim

>> Ilija and I would like to start a discussion about the following RFC:
>> https://wiki.php.net/rfc/pass_scope_to_magic_accessors
>
> Unless I missed anything, the RFC does not explain how the values that
> are passed into the second parameter will look like. What kinds of
> strings will be passed? When will the value be null?
>
> Best regards
> Tim Düsterhus

You're right, we'll try to improve the wording and provide a handful of
examples. Essentially, $callingScope should contain the class name of
the calling scope, so the place where the magic method was called. When
the calling scope is not a class (global scope, function, static
closure, etc.) null is passed instead.

https://gist.github.com/iluuu1994/c7950245c13c21f559c81776100b09e0

This is essentially trying to provide a way to implement visibility
checks for magic methods. Magic methods will be called from any scope,
so it is hard to tell whether the calling scope is allowed to access the
given property. This is not a problem when the property is supposed to
be public but becomes difficult when trying to restrict access to it. It
is possible through debug_backtrace() (as Nicolas demonstrates in the
RFC) but it's not very robust and becomes more complex when trying to
cover all cases (e.g. calls to parent::__get()).

The proposal aims to simplify this use case.

We'll try to clarify this in the RFC in the coming days.
Thanks for the feedback!

Ilija

Tim Düsterhus

3 years ago

Hi

On 1/19/23 18:43, Ilija Tovilo wrote:

> You're right, we'll try to improve the wording and provide a handful of
> examples. Essentially, $callingScope should contain the class name of
> the calling scope, so the place where the magic method was called. When
> the calling scope is not a class (global scope, function, static
> closure, etc.) null is passed instead.

Thank you, this is more clear now.

> given property. This is not a problem when the property is supposed to
> be public but becomes difficult when trying to restrict access to it. It

If I understand the proposal correctly, then any access restriction
would effectively be a Gentleman's agreement only, because nothing is
preventing me from calling '$someObj->__get('someProp',
$someObj::class);', no?

Honestly so far I fail to see the cost/benefit ratio to be acceptable
for the added complexity here. Not just the implementation complexity
within the language itself, but also because this will need to be
documented within the PHP manual and integrated within tooling, such as
static analyzers.

Is there some real-world example where I would employ this type of
access control within the magic methods and where "not documenting the
property to not expose it to the IDE autocompletion" would not be
sufficient, but this easily circumventable check would be?

My only use case of __get() so far was making properties visible to the
public, while restricting write access, so this is not a problem I
needed to solve before.

Best regards
Tim Düsterhus

Nicolas Grekas

3 years ago

> On 1/19/23 18:43, Ilija Tovilo wrote:
> > You're right, we'll try to improve the wording and provide a handful of
> > examples. Essentially, $callingScope should contain the class name of
> > the calling scope, so the place where the magic method was called. When
> > the calling scope is not a class (global scope, function, static
> > closure, etc.) null is passed instead.
>
> Thank you, this is more clear now.
>
> > given property. This is not a problem when the property is supposed to
> > be public but becomes difficult when trying to restrict access to it. It
>
> If I understand the proposal correctly, then any access restriction
> would effectively be a Gentleman's agreement only, because nothing is
> preventing me from calling '$someObj->__get('someProp',
> $someObj::class);', no?
>

Yep. That's already the case: using a rebound close, we can fake any scope
already so this is not new capability.

> Honestly so far I fail to see the cost/benefit ratio to be acceptable
> for the added complexity here. Not just the implementation complexity
> within the language itself, but also because this will need to be
> documented within the PHP manual and integrated within tooling, such as
> static analyzers.
>
> Is there some real-world example where I would employ this type of
> access control within the magic methods and where "not documenting the
> property to not expose it to the IDE autocompletion" would not be
> sufficient, but this easily circumventable check would be?
>
> My only use case of __get() so far was making properties visible to the
> public, while restricting write access, so this is not a problem I
> needed to solve before.
>

Legit concerns. I'm going to prepare a more extensive use case so the
motivations of RFC become more obvious.

I'll get back on this thread when ready. Stay tuned :)

Nicolas

Dan Ackroyd

3 years ago

On Mon, 23 Jan 2023 at 16:27, Nicolas Grekas
<nicolas.grekas+php@gmail.com> wrote:

>
> Legit concerns. I'm going to prepare a more extensive use case so the
> motivations of RFC become more obvious.
>
> I'll get back on this thread when ready. Stay tuned :)

Please can you look at implementing it as a function, that returns a
more usable set of data than debug_backtrace does. This does sound
like useful functionality in other cases than __get.

If the amount of data returned by including all the backtrace is a
concern, that sounds to me like including an max number of frames
would solve that?

cheers
Dan
Ack

A.L.E.C

3 years ago

On 19.01.2023 17:23, Nicolas Grekas wrote:

> Hi internals,
>
> Ilija and I would like to start a discussion about the following RFC:
> https://wiki.php.net/rfc/pass_scope_to_magic_accessors
>
> When using magic methods to access actual properties, respecting their
> declared visibility is often desired. Yet, accessing the calling scope to
> emulate the visibility restrictions is unreasonably difficult at the
> moment. This RFC proposes to pass the calling scope to magic accessors to
> make it trivial to get it.

Why not a new function, e.g. func_call_scope()? It could work
everywhere, not only in magic methods.

I guess it would be tricky with calls to the parent magic method when
you need the "outer calling scope", but I'd like to see some reasoning
in the RFC.

--
Aleksander Machniak
Kolab Groupware Developer        [https://kolab.org]
Roundcube Webmail Developer  [https://roundcube.net]
----------------------------------------------------
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

Marco Pivetta

3 years ago

Not just that, but `debug_backtrace()` already exists for that purpose.

On Thu, 19 Jan 2023, 19:23 Aleksander Machniak, <alec@alec.pl> wrote:

Clint Priest

3 years ago

On 1/19/2023 12:40 PM, Marco Pivetta wrote:

> Not just that, but `debug_backtrace()` already exists for that purpose.
>

I have use debug_backtrace() exactly for this purpose in the past. It
was to make a protected property readable/writable, but only by certain
"friend classes."

It's pretty trivial to make happen with debug_backtrace() and even more
so if you wrap it in its own reusable function.

I don't see the need to implement this in the language since it's
possible to achieve in userland trivially.

Ilija Tovilo

3 years ago

Hi Aleksander

>> https://wiki.php.net/rfc/pass_scope_to_magic_accessors
>
> Why not a new function, e.g. func_call_scope()? It could work
> everywhere, not only in magic methods.
>
> I guess it would be tricky with calls to the parent magic method when
> you need the "outer calling scope", but I'd like to see some reasoning
> in the RFC.
>
> --
> Aleksander Machniak

This has been suggested before and we're consider it. Here's a
comparable PHP implementation.

https://3v4l.org/6Ua8t

This would only work when called directly from the magic method but that
seems appropriate given that you'd also have to pass $callingScope to
any function requiring it.

Nicolas has noted that this would make it impossible to pass something
other than the actual calling scope to parent::__get(). It's not clear
yet whether there's a use case for that.

---

Hi Marco

> Not just that, but `debug_backtrace()` already exists for that purpose.

As mentioned in the RFC and the last e-mail, yes this can be achieved
via debug_backtrace(). However, as demonstrated by the 3v4l, it's not
exactly trivial. It also requires the full stack trace, as well as all
frame objects and arguments. That's not going to be great for
performance. So saying debug_backtrace() exists for this purpose is
misleading, to say the least.

Ilija

Marco Pivetta

3 years ago

Hey Ilija!

On Thu, 19 Jan 2023, 20:26 Ilija Tovilo, <tovilo.ilija@gmail.com> wrote:

> However, as demonstrated by the 3v4l, it's not
> exactly trivial. It also requires the full stack trace, as well as all
> frame objects and arguments. That's not going to be great for
> performance. So saying debug_backtrace() exists for this purpose is
> misleading, to say the least.
>

Meh, I made the kinkiest code work that way, and it's mostly for edge
cases: I would raise this as a problem once:

1. The use-case is not kinky
2. The performance benefits are so large that they warrant a language
change

Integration-testing code that uses `debug_backtrace()` is usually
sufficient for making this stuff work.

Rowan Collins

3 years ago

On 19/01/2023 19:25, Ilija Tovilo wrote:

> This has been suggested before and we're consider it. Here's a
> comparable PHP implementation.
>
> https://3v4l.org/6Ua8t

I'm confused what all the complexity here is doing. I get the same
result for that example with this one liner, using a tightly limited
backtrace with no objects:

function get_calling_scope() {
    return debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2]['class']
?? null;
}

https://3v4l.org/VQARq3


In the RFC and this thread you talk a lot about parent::__get() needing
special cases, but I don't understand why. In my mind,
parent::__get('name') is just a regular method call, so why does it
change the meaning of "calling scope"?

Regards,

--
Rowan Tommins
[IMSoP]

Nicolas Grekas

2 years ago

Hi all,

Ilija and I would like to start a discussion about the following RFC:

> https://wiki.php.net/rfc/pass_scope_to_magic_accessors
>
> When using magic methods to access actual properties, respecting their
> declared visibility is often desired. Yet, accessing the calling scope to
> emulate the visibility restrictions is unreasonably difficult at the
> moment. This RFC proposes to pass the calling scope to magic accessors to
> make it trivial to get it.
>

Just a quick heads up on this thread to let you know that I'm withdrawing
the RFC.
I'm working with Arnaud Le Blanc on a native implementation of lazy objects
and I don't need fast access to the calling scope for any other use case.
If anyone has, feel free to follow up, but on my side I'm going to focus on
that new soon-to-be RFC for lazy objects. Stay tuned :)

Even if I'm withdrawing, I'm sending a Big Thank You to Ilija for writing
the prototype implementation.
Also thank you to everybody who took some time to review the proposal!

Cheers,
Nicolas