From ada90d3b0a10249d0bff73a71f81da9b3627dd75 Mon Sep 17 00:00:00 2001 From: RFD Date: Tue, 13 Jan 2015 15:12:25 -0500 Subject: [PATCH 1/4] [ticket/13502] Controller resolver should handle callable functions and objects PHPBB3-13502 --- phpBB/phpbb/controller/resolver.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/controller/resolver.php b/phpBB/phpbb/controller/resolver.php index 4f432c3323..233e2e94a4 100644 --- a/phpBB/phpbb/controller/resolver.php +++ b/phpBB/phpbb/controller/resolver.php @@ -126,9 +126,18 @@ class resolver implements ControllerResolverInterface */ public function getArguments(Request $request, $controller) { - // At this point, $controller contains the object and method name - list($object, $method) = $controller; - $mirror = new \ReflectionMethod($object, $method); + // At this point, $controller should be a callable + if (is_array($controller)) + { + list($object, $method) = $controller; + $mirror = new \ReflectionMethod($object, $method); + } else if (is_object($controller) && !$controller instanceof \Closure) + { + $mirror = new \ReflectionObject($controller); + $mirror = $mirror->getMethod('__invoke'); + } else { + $mirror = new \ReflectionFunction($controller); + } $arguments = array(); $parameters = $mirror->getParameters(); From 3f4cf728724528e2c37a3bad3f9c705c71225b26 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 30 Mar 2016 00:08:02 +0200 Subject: [PATCH 2/4] [ticket/13502] Fix coding style PHPBB3-13502 --- phpBB/phpbb/controller/resolver.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/controller/resolver.php b/phpBB/phpbb/controller/resolver.php index 233e2e94a4..f8dffc12de 100644 --- a/phpBB/phpbb/controller/resolver.php +++ b/phpBB/phpbb/controller/resolver.php @@ -131,11 +131,14 @@ class resolver implements ControllerResolverInterface { list($object, $method) = $controller; $mirror = new \ReflectionMethod($object, $method); - } else if (is_object($controller) && !$controller instanceof \Closure) + } + else if (is_object($controller) && !$controller instanceof \Closure) { $mirror = new \ReflectionObject($controller); $mirror = $mirror->getMethod('__invoke'); - } else { + } + else + { $mirror = new \ReflectionFunction($controller); } From 91045879df2ee9918c7a03a58c64a74335ac67c1 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 30 Mar 2016 17:44:38 +0200 Subject: [PATCH 3/4] [ticket/13502] Test getArguments() method of controller resolver PHPBB3-13502 --- tests/controller/controller_test.php | 57 +++++++++++++++---- .../controller/ext/vendor2/foo/controller.php | 12 +++- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index abc1124a90..8bd7bf678c 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -11,6 +11,9 @@ * */ +include_once(__DIR__ . '/ext/vendor2/foo/controller.php'); +include_once(__DIR__.'/phpbb/controller/foo.php'); + use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -64,7 +67,7 @@ class phpbb_controller_controller_test extends phpbb_test_case $this->assertNull($routes->get('controller_noroute')); } - public function test_controller_resolver() + protected function get_foo_container() { $container = new ContainerBuilder(); // YamlFileLoader only uses one path at a time, so we need to loop @@ -75,26 +78,58 @@ class phpbb_controller_controller_test extends phpbb_test_case $loader->load('services.yml'); } - // Autoloading classes within the tests folder does not work - // so I'll include them manually. - if (!class_exists('vendor2\\foo\\controller')) - { - include(__DIR__ . '/ext/vendor2/foo/controller.php'); - } - if (!class_exists('phpbb\\controller\\foo')) - { - include(__DIR__.'/phpbb/controller/foo.php'); - } + return $container; + } + + public function test_controller_resolver() + { + $container = $this->get_foo_container(); $resolver = new \phpbb\controller\resolver($container, dirname(__FILE__) . '/'); $symfony_request = new Request(); $symfony_request->attributes->set('_controller', 'foo.controller:handle'); $this->assertEquals($resolver->getController($symfony_request), array(new foo\controller, 'handle')); + $this->assertEquals(array('foo'), $resolver->getArguments($symfony_request, $resolver->getController($symfony_request))); $symfony_request = new Request(); $symfony_request->attributes->set('_controller', 'core_foo.controller:bar'); $this->assertEquals($resolver->getController($symfony_request), array(new phpbb\controller\foo, 'bar')); + $this->assertEquals(array(), $resolver->getArguments($symfony_request, $resolver->getController($symfony_request))); + } + + public function data_get_arguments() + { + return array( + array(array(new foo\controller(), 'handle2'), array('foo', 0)), + array(array(new foo\controller(), 'handle_fail'), array('default'), array('no_default' => 'default')), + array(array(new foo\controller(), 'handle_fail'), array(), array(), '\phpbb\controller\exception', 'CONTROLLER_ARGUMENT_VALUE_MISSING'), + array('', array(), array(), '\ReflectionException', 'Function () does not exist'), + array(new foo\controller(), array(), array(), '\ReflectionException', 'Method __invoke does not exist'), + ); + } + + /** + * @dataProvider data_get_arguments + */ + public function test_get_arguments($input, $expected, $set_attributes = array(), $exception = '', $exception_message = '') + { + $container = $this->get_foo_container(); + + $resolver = new \phpbb\controller\resolver($container, dirname(__FILE__) . '/'); + $symfony_request = new Request(); + + foreach ($set_attributes as $name => $value) + { + $symfony_request->attributes->set($name, $value); + } + + if (!empty($exception)) + { + $this->setExpectedException($exception, $exception_message); + } + + $this->assertEquals($expected, $resolver->getArguments($symfony_request, $input)); } } diff --git a/tests/controller/ext/vendor2/foo/controller.php b/tests/controller/ext/vendor2/foo/controller.php index ce2233b3c9..b2c3df616c 100644 --- a/tests/controller/ext/vendor2/foo/controller.php +++ b/tests/controller/ext/vendor2/foo/controller.php @@ -11,8 +11,18 @@ class controller * * @return null */ - public function handle() + public function handle($optional = 'foo') { return new Response('Test', 200); } + + public function handle2($foo = 'foo', $very_optional = 0) + { + return new Response('Test2', 200); + } + + public function handle_fail($no_default) + { + return new Response('Test_fail', 200); + } } From 01d56673889cb12a11fca24b62f6e93e1c9f8dd8 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 30 Mar 2016 17:49:10 +0200 Subject: [PATCH 4/4] [ticket/13502] Also cover passing object to resolver in tests PHPBB3-13502 --- tests/controller/controller_test.php | 3 ++- tests/controller/ext/vendor2/foo/controller.php | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 8bd7bf678c..d921d0eade 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -104,9 +104,10 @@ class phpbb_controller_controller_test extends phpbb_test_case return array( array(array(new foo\controller(), 'handle2'), array('foo', 0)), array(array(new foo\controller(), 'handle_fail'), array('default'), array('no_default' => 'default')), + array(new foo\controller(), array(), array()), array(array(new foo\controller(), 'handle_fail'), array(), array(), '\phpbb\controller\exception', 'CONTROLLER_ARGUMENT_VALUE_MISSING'), array('', array(), array(), '\ReflectionException', 'Function () does not exist'), - array(new foo\controller(), array(), array(), '\ReflectionException', 'Method __invoke does not exist'), + array(new phpbb\controller\foo, array(), array(), '\ReflectionException', 'Method __invoke does not exist'), ); } diff --git a/tests/controller/ext/vendor2/foo/controller.php b/tests/controller/ext/vendor2/foo/controller.php index b2c3df616c..cabcae042b 100644 --- a/tests/controller/ext/vendor2/foo/controller.php +++ b/tests/controller/ext/vendor2/foo/controller.php @@ -25,4 +25,9 @@ class controller { return new Response('Test_fail', 200); } + + public function __invoke() + { + $this->handle(); + } }