Skip to content

Commit

Permalink
Make ReflectionProperty/Method always accessible
Browse files Browse the repository at this point in the history
With this patch, it is no longer required to call
`ReflectionProperty#setAccessible()` or
`ReflectionMethod#setAccessible()` with `true`.

If a userland consumer already got to the point of accessing
object/class information via reflection, it makes little sense
for `ext/reflection` to disallow accessing `private`/`protected`
symbols by default.

After this patch, calling `ReflectionProperty#setAccessible(true)`
or `ReflectionMethod#setAccessible(true)` on newly instantiated
`ReflectionProperty` or `ReflectionMethod` respectively will have
no effect.

RFC: https://wiki.php.net/rfc/make-reflection-setaccessible-no-op

Closes GH-5412.
  • Loading branch information
Ocramius authored and nikic committed Jul 8, 2021
1 parent 2763f14 commit 6e16e1d
Show file tree
Hide file tree
Showing 19 changed files with 122 additions and 274 deletions.
6 changes: 6 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,12 @@ PHP 8.1 UPGRADE NOTES
. PDO::getAttributes() with PDO::ATTR_SERVER_INFO and PDO::ATTR_SERVER_VERSION
now return values instead of throwing PDOException.

- Reflection:
. ReflectionProperty::setAccessible() and ReflectionMethod::setAccessible()
no longer have an effect. Properties and methods are always considered
accessible through reflection.
RFC: https://wiki.php.net/rfc/make-reflection-setaccessible-no-op

========================================
6. New Functions
========================================
Expand Down
1 change: 0 additions & 1 deletion Zend/tests/bug63762.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ Bug #63762 - Sigsegv when Exception::$trace is changed by user
$e = new Exception();

$ref = new ReflectionProperty($e, 'trace');
$ref->setAccessible(TRUE);

echo "Array of NULL:\n";
$ref->setValue($e, array(NULL));
Expand Down
1 change: 0 additions & 1 deletion Zend/tests/bug72177.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class Parnt
$this->child = new Child();

$prop = new \ReflectionProperty($this, 'child');
$prop->setAccessible(true);
$prop->setValue($this, null);
}
}
Expand Down
1 change: 0 additions & 1 deletion Zend/tests/bug72177_2.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class Bar extends Foo

$r = new ReflectionProperty(Foo::class, 'bar');

$r->setAccessible(true);
echo "OK\n";
?>
--EXPECT--
Expand Down
1 change: 0 additions & 1 deletion Zend/tests/bug78921.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class OtherClass

$reflectionClass = new ReflectionClass('OtherClass');
$reflectionProperty = $reflectionClass->getProperty('prop');
$reflectionProperty->setAccessible(true);
$value = $reflectionProperty->getValue();
echo "Value is $value\n";

Expand Down
46 changes: 0 additions & 46 deletions ext/reflection/php_reflection.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ typedef struct {
void *ptr;
zend_class_entry *ce;
reflection_type_t ref_type;
unsigned int ignore_visibility:1;
zend_object zo;
} reflection_object;

Expand Down Expand Up @@ -1440,7 +1439,6 @@ static void reflection_property_factory(zend_class_entry *ce, zend_string *name,
intern->ptr = reference;
intern->ref_type = REF_TYPE_PROPERTY;
intern->ce = ce;
intern->ignore_visibility = 0;
ZVAL_STR_COPY(reflection_prop_name(object), name);
ZVAL_STR_COPY(reflection_prop_class(object), prop ? prop->ce->name : ce->name);
}
Expand All @@ -1463,7 +1461,6 @@ static void reflection_class_constant_factory(zend_string *name_str, zend_class_
intern->ptr = constant;
intern->ref_type = REF_TYPE_CLASS_CONSTANT;
intern->ce = constant->ce;
intern->ignore_visibility = 0;

ZVAL_STR_COPY(reflection_prop_name(object), name_str);
ZVAL_STR_COPY(reflection_prop_class(object), constant->ce->name);
Expand All @@ -1482,7 +1479,6 @@ static void reflection_enum_case_factory(zend_class_entry *ce, zend_string *name
intern->ptr = constant;
intern->ref_type = REF_TYPE_CLASS_CONSTANT;
intern->ce = constant->ce;
intern->ignore_visibility = 0;

ZVAL_STR_COPY(reflection_prop_name(object), name_str);
ZVAL_STR_COPY(reflection_prop_class(object), constant->ce->name);
Expand Down Expand Up @@ -3313,15 +3309,6 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic)
RETURN_THROWS();
}

if (!(mptr->common.fn_flags & ZEND_ACC_PUBLIC) && intern->ignore_visibility == 0) {
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Trying to invoke %s method %s::%s() from scope %s",
mptr->common.fn_flags & ZEND_ACC_PROTECTED ? "protected" : "private",
ZSTR_VAL(mptr->common.scope->name), ZSTR_VAL(mptr->common.function_name),
ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
RETURN_THROWS();
}

if (variadic) {
ZEND_PARSE_PARAMETERS_START(1, -1)
Z_PARAM_OBJECT_OR_NULL(object)
Expand Down Expand Up @@ -3696,16 +3683,11 @@ ZEND_METHOD(ReflectionMethod, getPrototype)
/* {{{ Sets whether non-public methods can be invoked */
ZEND_METHOD(ReflectionMethod, setAccessible)
{
reflection_object *intern;
bool visible;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "b", &visible) == FAILURE) {
RETURN_THROWS();
}

intern = Z_REFLECTION_P(ZEND_THIS);

intern->ignore_visibility = visible;
}
/* }}} */

Expand Down Expand Up @@ -3745,7 +3727,6 @@ ZEND_METHOD(ReflectionClassConstant, __construct)
intern->ptr = constant;
intern->ref_type = REF_TYPE_CLASS_CONSTANT;
intern->ce = constant->ce;
intern->ignore_visibility = 0;
ZVAL_STR_COPY(reflection_prop_name(object), constname);
ZVAL_STR_COPY(reflection_prop_class(object), constant->ce->name);
}
Expand Down Expand Up @@ -5453,7 +5434,6 @@ ZEND_METHOD(ReflectionProperty, __construct)
intern->ptr = reference;
intern->ref_type = REF_TYPE_PROPERTY;
intern->ce = ce;
intern->ignore_visibility = 0;
}
/* }}} */

Expand Down Expand Up @@ -5580,13 +5560,6 @@ ZEND_METHOD(ReflectionProperty, getValue)

GET_REFLECTION_OBJECT_PTR(ref);

if (!(prop_get_flags(ref) & ZEND_ACC_PUBLIC) && intern->ignore_visibility == 0) {
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Cannot access non-public property %s::$%s",
ZSTR_VAL(intern->ce->name), ZSTR_VAL(ref->unmangled_name));
RETURN_THROWS();
}

if (prop_get_flags(ref) & ZEND_ACC_STATIC) {
member_p = zend_read_static_property_ex(intern->ce, ref->unmangled_name, 0);
if (member_p) {
Expand Down Expand Up @@ -5630,13 +5603,6 @@ ZEND_METHOD(ReflectionProperty, setValue)

GET_REFLECTION_OBJECT_PTR(ref);

if (!(prop_get_flags(ref) & ZEND_ACC_PUBLIC) && intern->ignore_visibility == 0) {
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Cannot access non-public property %s::$%s",
ZSTR_VAL(intern->ce->name), ZSTR_VAL(ref->unmangled_name));
RETURN_THROWS();
}

if (prop_get_flags(ref) & ZEND_ACC_STATIC) {
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "z", &value) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "zz", &tmp, &value) == FAILURE) {
Expand Down Expand Up @@ -5669,13 +5635,6 @@ ZEND_METHOD(ReflectionProperty, isInitialized)

GET_REFLECTION_OBJECT_PTR(ref);

if (!(prop_get_flags(ref) & ZEND_ACC_PUBLIC) && intern->ignore_visibility == 0) {
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Cannot access non-public property %s::$%s",
ZSTR_VAL(intern->ce->name), ZSTR_VAL(ref->unmangled_name));
RETURN_THROWS();
}

if (prop_get_flags(ref) & ZEND_ACC_STATIC) {
member_p = zend_read_static_property_ex(intern->ce, ref->unmangled_name, 1);
if (member_p) {
Expand Down Expand Up @@ -5762,16 +5721,11 @@ ZEND_METHOD(ReflectionProperty, getAttributes)
/* {{{ Sets whether non-public properties can be requested */
ZEND_METHOD(ReflectionProperty, setAccessible)
{
reflection_object *intern;
bool visible;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "b", &visible) == FAILURE) {
RETURN_THROWS();
}

intern = Z_REFLECTION_P(ZEND_THIS);

intern->ignore_visibility = visible;
}
/* }}} */

Expand Down
30 changes: 13 additions & 17 deletions ext/reflection/tests/ReflectionClass_getProperty_003.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,9 @@ function showInfo($name) {
echo $e->getMessage() . "\n";
return;
}
try {
var_dump($rp);
var_dump($rp->getValue($myC));
} catch (Exception $e) {
echo $e->getMessage() . "\n";
return;
}

var_dump($rp);
var_dump($rp->getValue($myC));
}


Expand Down Expand Up @@ -110,7 +106,7 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "A"
}
Cannot access non-public property C::$protA
string(10) "protA in A"
--- (Reflecting on privA) ---
Property C::$privA does not exist
--- (Reflecting on pubB) ---
Expand All @@ -128,7 +124,7 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "B"
}
Cannot access non-public property C::$protB
string(10) "protB in B"
--- (Reflecting on privB) ---
Property C::$privB does not exist
--- (Reflecting on pubC) ---
Expand All @@ -146,15 +142,15 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "C"
}
Cannot access non-public property C::$protC
string(10) "protC in C"
--- (Reflecting on privC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
string(5) "privC"
["class"]=>
string(1) "C"
}
Cannot access non-public property C::$privC
string(10) "privC in C"
--- (Reflecting on doesNotExist) ---
Property C::$doesNotExist does not exist
--- (Reflecting on A::pubC) ---
Expand All @@ -172,15 +168,15 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "A"
}
Cannot access non-public property A::$protC
string(10) "protC in A"
--- (Reflecting on A::privC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
string(5) "privC"
["class"]=>
string(1) "A"
}
Cannot access non-public property A::$privC
string(10) "privC in A"
--- (Reflecting on B::pubC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
Expand All @@ -196,15 +192,15 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "B"
}
Cannot access non-public property B::$protC
string(10) "protC in B"
--- (Reflecting on B::privC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
string(5) "privC"
["class"]=>
string(1) "B"
}
Cannot access non-public property B::$privC
string(10) "privC in B"
--- (Reflecting on c::pubC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
Expand All @@ -230,15 +226,15 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "C"
}
Cannot access non-public property C::$protC
string(10) "protC in C"
--- (Reflecting on C::privC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
string(5) "privC"
["class"]=>
string(1) "C"
}
Cannot access non-public property C::$privC
string(10) "privC in C"
--- (Reflecting on X::pubC) ---
Fully qualified property name X::$pubC does not specify a base class of C
--- (Reflecting on X::protC) ---
Expand Down
20 changes: 10 additions & 10 deletions ext/reflection/tests/ReflectionClass_getProperty_004.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "A"
}
Cannot access non-public property C::$protA
string(10) "protA in A"
--- (Reflecting on privA) ---
Property C::$privA does not exist
--- (Reflecting on pubB) ---
Expand All @@ -128,7 +128,7 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "B"
}
Cannot access non-public property C::$protB
string(10) "protB in B"
--- (Reflecting on privB) ---
Property C::$privB does not exist
--- (Reflecting on pubC) ---
Expand All @@ -146,15 +146,15 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "C"
}
Cannot access non-public property C::$protC
string(10) "protC in C"
--- (Reflecting on privC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
string(5) "privC"
["class"]=>
string(1) "C"
}
Cannot access non-public property C::$privC
string(10) "privC in C"
--- (Reflecting on doesNotExist) ---
Property C::$doesNotExist does not exist
--- (Reflecting on A::pubC) ---
Expand All @@ -172,15 +172,15 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "A"
}
Cannot access non-public property A::$protC
string(10) "protC in C"
--- (Reflecting on A::privC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
string(5) "privC"
["class"]=>
string(1) "A"
}
Cannot access non-public property A::$privC
string(10) "privC in A"
--- (Reflecting on B::pubC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
Expand All @@ -196,15 +196,15 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "B"
}
Cannot access non-public property B::$protC
string(10) "protC in C"
--- (Reflecting on B::privC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
string(5) "privC"
["class"]=>
string(1) "B"
}
Cannot access non-public property B::$privC
string(10) "privC in B"
--- (Reflecting on c::pubC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
Expand All @@ -230,15 +230,15 @@ object(ReflectionProperty)#%d (2) {
["class"]=>
string(1) "C"
}
Cannot access non-public property C::$protC
string(10) "protC in C"
--- (Reflecting on C::privC) ---
object(ReflectionProperty)#%d (2) {
["name"]=>
string(5) "privC"
["class"]=>
string(1) "C"
}
Cannot access non-public property C::$privC
string(10) "privC in C"
--- (Reflecting on X::pubC) ---
Fully qualified property name X::$pubC does not specify a base class of C
--- (Reflecting on X::protC) ---
Expand Down

0 comments on commit 6e16e1d

Please sign in to comment.