Inconsistencies when accessing protected members

18 years ago by Robin Fernandes — view source — reply

unread

Hi all,

The fix to bug 37212 (http://bugs.php.net/bug.php?id=37632) introduced
an unusual method accessibility rule. A class can access a protected
method declared outside of its own direct class hierarchy if that
method has a prototype in a common superclass.

<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {echo B1::f();}
}
B2::test(); // prints B1::f()
?>

This is achieved using by zend_get_function_root_class() when invoking
zend_check_protected(), e.g.:
zend_check_protected(zend_get_function_root_class(fbc), EG(scope))

Looking at other uses of zend_check_protected() reveals at least 5
cases where this rule is not enforced. They are illustrated below. So
is the rule itself incorrect? or should the inconsistent cases be
fixed?

The examples below were tested on 5.2.5 and the latest 5.3 and 6.0 snaps.

The visibility rule does not apply to properties (static or not):
<?php
class A {
protected $p = 'A::$p';
static protected $sp = 'A::$sp';
}
class B1 extends A {
protected $p = 'B1::$p';
static protected $sp = 'B1::$sp';
}
class B2 extends A {
static public function test() {
$b1 = new B1;
echo $b1->p; //Fatal error: Cannot access protected property B1::$p
echo B1::$sp; //Fatal error: Cannot access protected property B1::$sp
}
}
B2::test();
?>

It doesn't apply to callbacks either:
<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {
echo call_user_func('B1::f');
}
}
B2::test(); // Warning: call_user_func() expects parameter 1 to be a
valid callback, cannot access protected method B1::f()
?>

is_callable() doesn't know about this visibility rule:
<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {
var_dump(is_callable('B1::f')); // returns false
B1::f(); // works
}
}
B2::test();
?>

The rule does not apply to the clone magic method:
<?php
class A {
protected function f() {return 'A::f()';}
protected function __clone() {}
}
class B1 extends A {
protected function f() {return 'B1::f()';}
protected function __clone() {}
}
class B2 extends A {
static public function test($obj) {
echo $obj->f(); // works
clone $obj; // Fatal error: Call to protected B1::__clone() from context 'B2'
}
}
B2::test(new B1);
?>

The rule does not apply to destructors:
<?php
class A {
protected function __destruct() {}
}
class B1 extends A {
protected function __destruct() {}
}
class B2 extends A {
static public function test() {
$obj = new B1;
} // Fatal error: Call to protected B1::__destruct() from context 'B2'
}
B2::test();
?>

Many thanks,
Robin

18 years ago by Felipe Pena — view source — reply

unread

Hi all,
in my point view, the zend_check_protected should be used like:
http://ecl.mediain.com.br/diff/protected.diff

This patch breaks a test (Zend/tests/bug37632.phpt):

class A1
{
protected function test()
{
echo METHOD . "\n";
}
}

class B1 extends A1
{
public function doTest(A1 $obj)
{
echo METHOD . "\n";
$obj->test();
}
}

class C1 extends A1
{
protected function test()
{
echo METHOD . "\n";
}
}

$b = new B1;
$b->doTest(new C1);

[...]

However, this also looks wrong to me.

Em Ter, 2008-02-05 às 11:43 +0000, Robin Fernandes escreveu:

Hi all,

The fix to bug 37212 (http://bugs.php.net/bug.php?id=37632) introduced
an unusual method accessibility rule. A class can access a protected
method declared outside of its own direct class hierarchy if that
method has a prototype in a common superclass.

<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {echo B1::f();}
}
B2::test(); // prints B1::f()
?>

This is achieved using by zend_get_function_root_class() when invoking
zend_check_protected(), e.g.:
zend_check_protected(zend_get_function_root_class(fbc), EG(scope))

Looking at other uses of zend_check_protected() reveals at least 5
cases where this rule is not enforced. They are illustrated below. So
is the rule itself incorrect? or should the inconsistent cases be
fixed?

The examples below were tested on 5.2.5 and the latest 5.3 and 6.0 snaps.

The visibility rule does not apply to properties (static or not):
<?php
class A {
protected $p = 'A::$p';
static protected $sp = 'A::$sp';
}
class B1 extends A {
protected $p = 'B1::$p';
static protected $sp = 'B1::$sp';
}
class B2 extends A {
static public function test() {
$b1 = new B1;
echo $b1->p; //Fatal error: Cannot access protected property B1::$p
echo B1::$sp; //Fatal error: Cannot access protected property B1::$sp
}
}
B2::test();
?>

It doesn't apply to callbacks either:
<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {
echo call_user_func('B1::f');
}
}
B2::test(); // Warning: call_user_func() expects parameter 1 to be a
valid callback, cannot access protected method B1::f()
?>

is_callable() doesn't know about this visibility rule:
<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {
var_dump(is_callable('B1::f')); // returns false
B1::f(); // works
}
}
B2::test();
?>

The rule does not apply to the clone magic method:
<?php
class A {
protected function f() {return 'A::f()';}
protected function __clone() {}
}
class B1 extends A {
protected function f() {return 'B1::f()';}
protected function __clone() {}
}
class B2 extends A {
static public function test($obj) {
echo $obj->f(); // works
clone $obj; // Fatal error: Call to protected B1::__clone() from context 'B2'
}
}
B2::test(new B1);
?>

The rule does not apply to destructors:
<?php
class A {
protected function __destruct() {}
}
class B1 extends A {
protected function __destruct() {}
}
class B2 extends A {
static public function test() {
$obj = new B1;
} // Fatal error: Call to protected B1::__destruct() from context 'B2'
}
B2::test();
?>

Many thanks,
Robin

--
Regards,
Felipe Pena.

18 years ago by Felipe Pena — view source — reply

unread

Hello,

Do we keep the support added in http://bugs.php.net/bug.php?id=37632
(that isn't supported in C++, for instance) or fix the
zend_is_callable_check_func() ?

Thanks.

2008/2/5, Robin Fernandes robinf@php.net:

Hi all,

The fix to bug 37212 (http://bugs.php.net/bug.php?id=37632) introduced
an unusual method accessibility rule. A class can access a protected
method declared outside of its own direct class hierarchy if that
method has a prototype in a common superclass.

<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {echo B1::f();}
}
B2::test(); // prints B1::f()
?>

This is achieved using by zend_get_function_root_class() when invoking
zend_check_protected(), e.g.:
zend_check_protected(zend_get_function_root_class(fbc), EG(scope))

Looking at other uses of zend_check_protected() reveals at least 5
cases where this rule is not enforced. They are illustrated below. So
is the rule itself incorrect? or should the inconsistent cases be
fixed?

The examples below were tested on 5.2.5 and the latest 5.3 and 6.0 snaps.

The visibility rule does not apply to properties (static or not):
<?php
class A {
protected $p = 'A::$p';
static protected $sp = 'A::$sp';
}
class B1 extends A {
protected $p = 'B1::$p';
static protected $sp = 'B1::$sp';
}
class B2 extends A {
static public function test() {
$b1 = new B1;
echo $b1->p; //Fatal error: Cannot access protected property B1::$p
echo B1::$sp; //Fatal error: Cannot access protected property B1::$sp
}
}
B2::test();
?>

It doesn't apply to callbacks either:
<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {
echo call_user_func('B1::f');
}
}
B2::test(); // Warning: call_user_func() expects parameter 1 to be a
valid callback, cannot access protected method B1::f()
?>

is_callable() doesn't know about this visibility rule:
<?php
class A {
static protected function f() {return 'A::f()';}
}
class B1 extends A {
static protected function f() {return 'B1::f()';}
}
class B2 extends A {
static public function test() {
var_dump(is_callable('B1::f')); // returns false
B1::f(); // works
}
}
B2::test();
?>

The rule does not apply to the clone magic method:
<?php
class A {
protected function f() {return 'A::f()';}
protected function __clone() {}
}
class B1 extends A {
protected function f() {return 'B1::f()';}
protected function __clone() {}
}
class B2 extends A {
static public function test($obj) {
echo $obj->f(); // works
clone $obj; // Fatal error: Call to protected B1::__clone() from context 'B2'
}
}
B2::test(new B1);
?>

The rule does not apply to destructors:
<?php
class A {
protected function __destruct() {}
}
class B1 extends A {
protected function __destruct() {}
}
class B2 extends A {
static public function test() {
$obj = new B1;
} // Fatal error: Call to protected B1::__destruct() from context 'B2'
}
B2::test();
?>

Many thanks,
Robin

18 years ago by Robin Fernandes — view source — reply

unread

Hi Felipe,

Hello,

Do we keep the support added in http://bugs.php.net/bug.php?id=37632
(that isn't supported in C++, for instance)

My preference would be to completely remove this behaviour, by which
protected methods can be invoked from outside of their declaring
class's hierarchy. In other words, remove all uses of
zend_get_function_root_class() (it is only ever used in conjunction
with zend_check_protected()).

So I'm in favour of your patch. And I think the declaration of
zend_get_function_root_class() could be removed too, as it isn't used
anymore after applying the patch.

Many thanks,
Robin

18 years ago by Lars Strojny — view source — reply

unread

Hi,

Am Dienstag, den 25.03.2008, 11:04 +0000 schrieb Robin Fernandes:
[...]

My preference would be to completely remove this behaviour, by which
protected methods can be invoked from outside of their declaring
class's hierarchy. In other words, remove all uses of
zend_get_function_root_class() (it is only ever used in conjunction
with zend_check_protected()).

Would that mean that the following code does not work anymore?

<?php
class Foo
{
protected function method()
{
}

public function doSomething(Foo $foo)
{
	$foo->method();
}

}

$foo1 = new Foo();
$foo1->doSomething(new Foo());

[...]

cu, Lars

18 years ago by Felipe Pena — view source — reply

unread

Em Ter, 2008-03-25 às 12:35 +0100, Lars Strojny escreveu:

Would that mean that the following code does not work anymore?

<?php
class Foo
{
protected function method()
{
}

public function doSomething(Foo $foo)
{
$foo->method();
}
}

$foo1 = new Foo();
$foo1->doSomething(new Foo());

This still will works.

18 years ago by Richard Quadling — view source — reply

unread

Em Ter, 2008-03-25 às 12:35 +0100, Lars Strojny escreveu:
Would that mean that the following code does not work anymore?

<?php
class Foo
{
protected function method()
{
}
  public function doSomething(Foo $foo)
  {
          $foo->method();
  }
}

$foo1 = new Foo();
$foo1->doSomething(new Foo());
This still will works.

Surely it shouldn't work at all unless the $foo === $this?

I understand that the checking is based upon the class and not the instance.

Shouldn't the instance be the limiting factor?

Richard.

Richard Quadling
Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731
"Standing on the shoulders of some very clever giants!"

18 years ago by Alexey Zakhlestin — view source — reply

unread

Shouldn't the instance be the limiting factor?

it shouldn't
public/protected/private are related to classes, not to objects.

--
Alexey Zakhlestin
http://blog.milkfarmsoft.com/

18 years ago by Richard Quadling — view source — reply

unread

Em Ter, 2008-03-25 às 12:35 +0100, Lars Strojny escreveu:
Would that mean that the following code does not work anymore?

<?php
class Foo
{
protected function method()
{
}
  public function doSomething(Foo $foo)
  {
          $foo->method();
  }
}

$foo1 = new Foo();
$foo1->doSomething(new Foo());
This still will works.
Surely it shouldn't work at all unless the $foo === $this?

I understand that the checking is based upon the class and not the instance.

Shouldn't the instance be the limiting factor?

Richard.

Maybe what I am saying is a little clearer if you change protected to private.

<?php
class Foo {
private function priv() {
echo METHOD, ':', LINE, PHP_EOL;
}

protected function prot() {
    echo __METHOD__, ':', __LINE__, PHP_EOL;
}

public function doSomething(Foo $foo) {
    $foo->prot();
    $foo->priv();
}

}

class Bar extends Foo {
protected function prot() {
echo METHOD, ':', LINE, PHP_EOL;
}
}

$foo1 = new Foo();
$foo1->doSomething(new Bar());

outputs (PHP 5.3.0-dev (cli) (built: Mar 18 2008 04:17:56))

Bar::prot:19
Foo::priv:4

It just doesn't seem right to be able to call a private or protected
method of another instance. Sort of isn't private any more.

And as for being able to call a protected method of a completely
different class, just because it shares the same ancestry. That seems
REALLY wrong.

Richard.

--

Richard Quadling
Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731
"Standing on the shoulders of some very clever giants!"

18 years ago by Alexey Zakhlestin — view source — reply

unread

It just doesn't seem right to be able to call a private or protected
method of another instance. Sort of isn't private any more.

And as for being able to call a protected method of a completely
different class, just because it shares the same ancestry. That seems
REALLY wrong.

it is needed for operations on several instances of the same class-hierarchy.

"private" means, that you are the author of the class and you know
what you are doing with instances of this class, but you do not want
to export that inner-functionality.

use case for "protected" is similiar, but relates to cases when you
have hierarchy of classes, which still have some common functionality,
which might be usable for multi-instance operations

--
Alexey Zakhlestin
http://blog.milkfarmsoft.com/

18 years ago by Marcus Boerger — view source — reply

unread

Hello Alexey,

Wednesday, March 26, 2008, 6:05:12 PM, you wrote:

It just doesn't seem right to be able to call a private or protected
method of another instance. Sort of isn't private any more.

And as for being able to call a protected method of a completely
different class, just because it shares the same ancestry. That seems
REALLY wrong.

it is needed for operations on several instances of the same class-hierarchy.

"private" means, that you are the author of the class and you know
what you are doing with instances of this class, but you do not want
to export that inner-functionality.

use case for "protected" is similiar, but relates to cases when you
have hierarchy of classes, which still have some common functionality,
which might be usable for multi-instance operations

Thanks. This is the exact way we see this. Anything that violates this rule
is a bug.

Best regards,
Marcus

18 years ago by Robin Fernandes — view source — reply

unread

Hi all, thanks for thinking about this.

use case for "protected" is similiar, but relates to cases when you
have hierarchy of classes, which still have some common functionality,
which might be usable for multi-instance operations

Thanks. This is the exact way we see this. Anything that violates this rule
is a bug.

Ok, good. So to get back to the original question
(http://turl.ca/kfanjo), in the following case, should B1::f() be
visible from B2 in the code example below? Note that:

B1 and B2 both extend A. B2 is neither an ancestor nor a
descendant of B1, but I suppose they could be considered to be part of
the same class hierarchy because they are siblings.
f() is declared as protected in A and B1, but not declared at all in B2.
The docs state: "Protected limits access to inherited and parent
classes (and to the class that defines the item)" -
http://php.net/manual/en/language.oop5.visibility.php .

Currently, B1::f() is visible from B2.

--> If this is a bug, Felipe's patch
(http://ecl.mediain.com.br/diff/protected.diff) fixes the issue. Let's
commit it! :)

--> If this is not a bug then the docs need to be clarified.
Furthermore, callbacks, is_callable(), lookups to the clone & destruct
magic methods and possibly property access should all be changed,
because they currently do not adhere to this rule (as illustrated in
the code snippets in http://turl.ca/kfanjo ).

The code:

<?php
class A {
static protected function f() {return 'In A::f';}
}
class B1 extends A {
static protected function f() {return 'In B1::f';}
}
class B2 extends A {
static public function test() {echo B1::f();} // Currently, this works.
}
B2::test();
?>

Regards,
Robin.

18 years ago by Stanislav Malyshev — view source — reply

unread

Hi!

B1 and B2 both extend A. B2 is neither an ancestor nor a
descendant of B1, but I suppose they could be considered to be part of
the same class hierarchy because they are siblings.

f() is declared as protected in A and B1, but not declared at all in B2.

Which means there exists A::f(), B1::f() and B2::f() (being the same as
A::f()), all of which are protected.

The docs state: "Protected limits access to inherited and parent
classes (and to the class that defines the item)" -
http://php.net/manual/en/language.oop5.visibility.php .

Currently, B1::f() is visible from B2.

A::f() is visible from B2, and B1 can not have access to f() more
restricted than A since it's violate the Liskov principle - objects of
B1 couldn't be used the same way as objects of A in all contexts.

Look at this also this way: suppose we had no B1::f. Then if we call f()
from B2, we always get A::f() which is protected, so it works. Now we
copy A::f definition to B1 verbatim - with same access, same everything.
Should now code that calls f() on B1 start failing?

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

18 years ago by Stanislav Malyshev — view source — reply

unread

It just doesn't seem right to be able to call a private or protected
method of another instance. Sort of isn't private any more.

Why not? Private/protected is meant to separate APIs, it's not a
security check on objects. Private means this API belongs to this class
only, protected means this API belongs to this class and the children -
i.e. to whoever develops this class further, but not to the clients.
However, I see no reason to restrict class's code access to its own
functions because the passed object is not $this.

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

18 years ago by Stanislav Malyshev — view source — reply

unread

This still will works.

Surely it shouldn't work at all unless the $foo === $this?

Why not? If the context is right, why shouldn't it be able to call this
content's protected functions?
As I understand, protected funciton means no code outside the class can
call it, since it's not a part of the public API. However, here we have
clear case of code inside the class, so what's the problem with
calling it?

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

18 years ago by Richard Lynch — view source — reply

unread

Do we keep the support added in http://bugs.php.net/bug.php?id=37632
(that isn't supported in C++, for instance) or fix the
zend_is_callable_check_func() ?

Personally, it makes sense to me for a PROTECTED function (et al) with
a common prototype ancestor to be able to be called by subclasses.

PRIVATE, not so clear-cut...

It should certainly be self-consistent with the other use cases,
whatever is decided.

--
Some people have a "gift" link here.
Know what I want?
I want you to buy a CD from some indie artist.
http://cdbaby.com/from/lynch
Yeah, I get a buck. So?

18 years ago by Lokrain — view source — reply

unread

Hello Internals,

This discussion was very interesting to me so I made some research about all
languages OOP.
Each time I saw definition of public, protected, private there was an
explanation which never
mentioned instances, but classes. I certainly thought that Richard is right
saying:

Surely it shouldn't work at all unless the $foo === $this?

I was even amazed that I haven't thought about this ever...and the
conclusion of my research
is that as, like Stanislav said, this keywords(public, etc) are for classes
not for instances...

I learned something new today :) Thanks for this discussion.

Best Regards, Dimitar Isusov

Inconsistencies when accessing protected members View as Markdown

Richard.

--

Look at this also this way: suppose we had no B1::f. Then if we call f() from B2, we always get A::f() which is protected, so it works. Now we copy A::f definition to B1 verbatim - with same access, same everything. Should now code that calls f() on B1 start failing?

Look at this also this way: suppose we had no B1::f. Then if we call f()
from B2, we always get A::f() which is protected, so it works. Now we
copy A::f definition to B1 verbatim - with same access, same everything.
Should now code that calls f() on B1 start failing?