Skip to content

Fix #79969: Wrong error message in strict mode in ReflectionMethod::_construct #5995

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Fix #79969: Wrong error message in strict mode in ReflectionMethod::_construct #5995

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff76978
Fix #79969: Wrong error message in strict mode in ReflectionMethod::_…
jdomenechb Aug 16, 2020
8bdd5c9
Improved correction for fix #79969 after feedback
jdomenechb Aug 16, 2020
897b491
Improved fix #79969 given feedback by assuring parameter types
jdomenechb Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 34 additions & 19 deletions ext/reflection/php_reflection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2954,26 +2954,41 @@ ZEND_METHOD(reflection_method, __construct)
size_t name_len, tmp_len;
zval ztmp;

if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "zs", &classname, &name_str, &name_len) == FAILURE) {
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "s", &name_str, &name_len) == FAILURE) {
return;
}
switch (ZEND_NUM_ARGS()) {
case 2:
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "zs", &classname, &name_str, &name_len) == FAILURE) {
return;
}

if ((tmp = strstr(name_str, "::")) == NULL) {
zend_throw_exception_ex(reflection_exception_ptr, 0,
"Invalid method name %s", name_str);
return;
}
classname = &ztmp;
tmp_len = tmp - name_str;
ZVAL_STRINGL(classname, name_str, tmp_len);
name_len = name_len - (tmp_len + 2);
name_str = tmp + 2;
orig_obj = NULL;
} else if (Z_TYPE_P(classname) == IS_OBJECT) {
orig_obj = classname;
} else {
orig_obj = NULL;
if (Z_TYPE_P(classname) == IS_OBJECT) {
orig_obj = classname;
} else {
orig_obj = NULL;
}

break;

case 1:
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "s", &name_str, &name_len) == FAILURE) {
return;
}

if ((tmp = strstr(name_str, "::")) == NULL) {
zend_throw_exception_ex(reflection_exception_ptr, 0, "Invalid method name %s", name_str);
return;
}

classname = &ztmp;
tmp_len = tmp - name_str;
ZVAL_STRINGL(classname, name_str, tmp_len);
name_len = name_len - (tmp_len + 2);
name_str = tmp + 2;
orig_obj = NULL;

break;

default:
ZEND_WRONG_PARAM_COUNT();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ZEND_WRONG_PARAM_COUNT();
zend_wrong_parameters_count_exception(1, 2);

Should do the trick, for PHP 7.3 and 7.4, the person doing the merge-up to master/PHP 8.0 will need to change it to zend_wrong_parameters_count_error() as the former API has been removed in master.

}

object = getThis();
Expand Down
18 changes: 6 additions & 12 deletions ext/reflection/tests/ReflectionMethod_006.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,9 @@ Steve Seear <[email protected]>
--FILE--
<?php

try {
new ReflectionMethod();
} catch (TypeError $re) {
echo "Ok - ".$re->getMessage().PHP_EOL;
}
try {
new ReflectionMethod('a', 'b', 'c');
} catch (TypeError $re) {
echo "Ok - ".$re->getMessage().PHP_EOL;
}
new ReflectionMethod();
new ReflectionMethod('a', 'b', 'c');


class C {
public function f() {}
Expand Down Expand Up @@ -43,8 +36,9 @@ var_dump($rm->getName(1));

?>
--EXPECTF--
Ok - ReflectionMethod::__construct() expects exactly 1 parameter, 0 given
Ok - ReflectionMethod::__construct() expects exactly 1 parameter, 3 given
Warning: Wrong parameter count for ReflectionMethod::__construct() in %s on line %d

Warning: Wrong parameter count for ReflectionMethod::__construct() in %s on line %d

Warning: ReflectionMethod::isFinal() expects exactly 0 parameters, 1 given in %s on line %d
NULL
Expand Down
27 changes: 10 additions & 17 deletions ext/reflection/tests/ReflectionMethod_constructor_error2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,11 @@ class TestClass
}
}

echo "Too few arguments:\n";
$methodInfo = new ReflectionMethod();

try {
echo "Too few arguments:\n";
$methodInfo = new ReflectionMethod();
} catch (TypeError $re) {
echo "Ok - ".$re->getMessage().PHP_EOL;
}
try {
echo "\nToo many arguments:\n";
$methodInfo = new ReflectionMethod("TestClass", "foo", true);
} catch (TypeError $re) {
echo "Ok - ".$re->getMessage().PHP_EOL;
}

echo "\nToo many arguments:\n";
$methodInfo = new ReflectionMethod("TestClass", "foo", true);

try {
//invalid class
Expand All @@ -50,12 +41,14 @@ try{
}

?>
--EXPECT--
--EXPECTF--
Too few arguments:
Ok - ReflectionMethod::__construct() expects exactly 1 parameter, 0 given

Warning: Wrong parameter count for ReflectionMethod::__construct() in %s on line %d

Too many arguments:
Ok - ReflectionMethod::__construct() expects exactly 1 parameter, 3 given

Warning: Wrong parameter count for ReflectionMethod::__construct() in %s on line %d
Ok - Class InvalidClassName does not exist
Ok - The parameter class is expected to be either a string or an object
Ok - ReflectionMethod::__construct() expects exactly 1 parameter, 2 given
Ok - ReflectionMethod::__construct() expects parameter 2 to be string, array given
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: missing new line before EOF

14 changes: 14 additions & 0 deletions ext/reflection/tests/bug79969.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Bug #79969: ReflectionMethod::__construct throws incorrect exception when 2nd parameter is int
--FILE--
<?php
declare(strict_types=1);

new ReflectionMethod('xxx', 1);
?>
--EXPECTF--
Fatal error: Uncaught TypeError: ReflectionMethod::__construct() expects parameter 2 to be string, int given in %s
Stack trace:
#0 %s(%d): ReflectionMethod->__construct('xxx', 1)
#1 {main}
thrown in %s on line %d