From 51273f6fb1421b68c1931c3960f68cd483f1ee95 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 7 Mar 2014 12:32:38 +0100 Subject: [PATCH 1/9] [ticket/12090] Pass route name to url() to allow admins to change the routes PHPBB3-12090 --- phpBB/config/services.yml | 1 + phpBB/phpbb/controller/helper.php | 38 ++++-- tests/controller/controller_test.php | 2 +- tests/controller/ext/foo/config/routing_2.yml | 3 - .../ext/{ => vendor2}/foo/config/routing.yml | 0 .../ext/vendor2/foo/config/routing_2.yml | 6 + .../ext/{ => vendor2}/foo/config/services.yml | 0 .../ext/{ => vendor2}/foo/controller.php | 0 tests/controller/helper_route_test.php | 126 ++++++++++++++++++ tests/controller/helper_url_test.php | 119 ----------------- 10 files changed, 162 insertions(+), 133 deletions(-) delete mode 100644 tests/controller/ext/foo/config/routing_2.yml rename tests/controller/ext/{ => vendor2}/foo/config/routing.yml (100%) create mode 100644 tests/controller/ext/vendor2/foo/config/routing_2.yml rename tests/controller/ext/{ => vendor2}/foo/config/services.yml (100%) rename tests/controller/ext/{ => vendor2}/foo/controller.php (100%) create mode 100644 tests/controller/helper_route_test.php delete mode 100644 tests/controller/helper_url_test.php diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 735626810f..0699cfcc6d 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -91,6 +91,7 @@ services: controller.helper: class: phpbb\controller\helper arguments: + - @ext.finder - @template - @user - @config diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index 05a05d1e57..daa52ec672 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -10,6 +10,8 @@ namespace phpbb\controller; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Routing\Generator\UrlGenerator; +use Symfony\Component\Routing\RequestContext; /** * Controller helper class, contains methods that do things for controllers @@ -56,13 +58,17 @@ class helper * @param string $phpbb_root_path phpBB root path * @param string $php_ext PHP extension */ - public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, $phpbb_root_path, $php_ext) + public function __construct(\phpbb\extension\finder $finder, \phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, $phpbb_root_path, $php_ext) { $this->template = $template; $this->user = $user; $this->config = $config; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; + + $provider = new \phpbb\controller\provider(); + $this->route_collection = $provider->import_paths_from_finder($finder)->find($this->phpbb_root_path); + } /** @@ -87,21 +93,33 @@ class helper } /** - * Generate a URL + * Generate a URL to a route * - * @param string $route The route to travel - * @param mixed $params String or array of additional url parameters + * @param string $route Name of the route to travel + * @param array $params String or array of additional url parameters * @param bool $is_amp Is url using & (true) or & (false) * @param string $session_id Possibility to use a custom session id instead of the global one * @return string The URL already passed through append_sid() */ - public function url($route, $params = false, $is_amp = true, $session_id = false) + public function route($route, array $params = array(), $is_amp = true, $session_id = false) { - $route_params = ''; - if (($route_delim = strpos($route, '?')) !== false) + $anchor = ''; + if (isset($params['#'])) { - $route_params = substr($route, $route_delim); - $route = substr($route, 0, $route_delim); + $anchor = '#' . $params['#']; + unset($params['#']); + } + $url_generator = new UrlGenerator($this->route_collection, new RequestContext()); + $route_url = $url_generator->generate($route, $params); + + if (strpos($route_url, '/') === 0) + { + $route_url = substr($route_url, 1); + } + + if ($is_amp) + { + $route_url = str_replace(array('&', '&'), array('&', '&'), $route_url); } // If enable_mod_rewrite is false, we need to include app.php @@ -111,7 +129,7 @@ class helper $route_prefix .= 'app.' . $this->php_ext . '/'; } - return append_sid($route_prefix . "$route" . $route_params, $params, $is_amp, $session_id); + return append_sid($route_prefix . $route_url . $anchor, false, $is_amp, $session_id); } /** diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 588adbcfb1..b65e677dbe 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -62,7 +62,7 @@ class phpbb_controller_controller_test extends phpbb_test_case // so I'll include them manually. if (!class_exists('foo\\controller')) { - include(__DIR__.'/ext/foo/controller.php'); + include(__DIR__ . '/ext/foo/controller.php'); } if (!class_exists('phpbb\\controller\\foo')) { diff --git a/tests/controller/ext/foo/config/routing_2.yml b/tests/controller/ext/foo/config/routing_2.yml deleted file mode 100644 index 35fff27037..0000000000 --- a/tests/controller/ext/foo/config/routing_2.yml +++ /dev/null @@ -1,3 +0,0 @@ -controller2: - pattern: /bar - defaults: { _controller: foo.controller:handle } diff --git a/tests/controller/ext/foo/config/routing.yml b/tests/controller/ext/vendor2/foo/config/routing.yml similarity index 100% rename from tests/controller/ext/foo/config/routing.yml rename to tests/controller/ext/vendor2/foo/config/routing.yml diff --git a/tests/controller/ext/vendor2/foo/config/routing_2.yml b/tests/controller/ext/vendor2/foo/config/routing_2.yml new file mode 100644 index 0000000000..d987a65aea --- /dev/null +++ b/tests/controller/ext/vendor2/foo/config/routing_2.yml @@ -0,0 +1,6 @@ +controller2: + pattern: /bar + defaults: { _controller: foo.controller:handle } +controller3: + pattern: /bar/p-{p} + defaults: { _controller: foo.controller:handle } diff --git a/tests/controller/ext/foo/config/services.yml b/tests/controller/ext/vendor2/foo/config/services.yml similarity index 100% rename from tests/controller/ext/foo/config/services.yml rename to tests/controller/ext/vendor2/foo/config/services.yml diff --git a/tests/controller/ext/foo/controller.php b/tests/controller/ext/vendor2/foo/controller.php similarity index 100% rename from tests/controller/ext/foo/controller.php rename to tests/controller/ext/vendor2/foo/controller.php diff --git a/tests/controller/helper_route_test.php b/tests/controller/helper_route_test.php new file mode 100644 index 0000000000..3cd8a7237d --- /dev/null +++ b/tests/controller/helper_route_test.php @@ -0,0 +1,126 @@ +user = $this->getMock('\phpbb\user'); + $phpbb_path_helper = new \phpbb\path_helper( + new \phpbb\symfony_request( + new phpbb_mock_request() + ), + new \phpbb\filesystem(), + $phpbb_root_path, + $phpEx + ); + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); + $this->template = new phpbb\template\twig\twig($phpbb_path_helper, $this->config, $this->user, new \phpbb\template\context()); + + $this->finder = new \phpbb\extension\finder( + new phpbb_mock_extension_manager( + dirname(__FILE__) . '/', + array( + 'vendor2/foo' => array( + 'ext_name' => 'vendor2/foo', + 'ext_active' => '1', + 'ext_path' => 'ext/vendor2/foo/', + ), + ) + ), + new \phpbb\filesystem(), + dirname(__FILE__) . '/', + new phpbb_mock_cache() + ); + } + + public function helper_url_data_no_rewrite() + { + return array( + array('controller2', array('t' => 1, 'f' => 2), true, false, 'app.php/foo/bar?t=1&f=2', 'parameters in params-argument as array'), + array('controller2', array('t' => 1, 'f' => 2), false, false, 'app.php/foo/bar?t=1&f=2', 'parameters in params-argument as array'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2), true, false, 'app.php/foo/bar/p-3?t=1&f=2', 'parameters in params-argument as array'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2), false, false, 'app.php/foo/bar/p-3?t=1&f=2', 'parameters in params-argument as array'), + + // Custom sid parameter + array('controller2', array('t' => 1, 'f' => 2), true, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid', 'params-argument (array) using session_id'), + array('controller2', array('t' => 1, 'f' => 2), false, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid', 'params-argument (array) using session_id'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2), true, 'custom-sid', 'app.php/foo/bar/p-3?t=1&f=2&sid=custom-sid', 'params-argument (array) using session_id'), + + // Testing anchors + array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'app.php/foo/bar?t=1&f=2#anchor', 'anchor in params-argument (array)'), + array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), false, false, 'app.php/foo/bar?t=1&f=2#anchor', 'anchor in params-argument (array)'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'app.php/foo/bar/p-3?t=1&f=2#anchor', 'anchor in params-argument (array)'), + + // Anchors and custom sid + array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), + array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), false, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'app.php/foo/bar/p-3?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), + + // Empty parameters should not append the & or ? + array('controller2', array(), true, false, 'app.php/foo/bar', 'no params using empty array'), + array('controller2', array(), false, false, 'app.php/foo/bar', 'no params using empty array'), + array('controller3', array('p' => 3), true, false, 'app.php/foo/bar/p-3', 'no params using empty array'), + ); + } + + /** + * @dataProvider helper_url_data_no_rewrite() + */ + public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description) + { + $this->helper = new \phpbb\controller\helper($this->finder, $this->template, $this->user, $this->config, dirname(__FILE__) . '/', 'php'); + $this->assertEquals(dirname(__FILE__) . '/' . $expected, $this->helper->route($route, $params, $is_amp, $session_id)); + } + + public function helper_url_data_with_rewrite() + { + return array( + array('controller2', array('t' => 1, 'f' => 2), true, false, 'foo/bar?t=1&f=2', 'parameters in params-argument as array'), + array('controller2', array('t' => 1, 'f' => 2), false, false, 'foo/bar?t=1&f=2', 'parameters in params-argument as array'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2), true, false, 'foo/bar/p-3?t=1&f=2', 'parameters in params-argument as array'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2), false, false, 'foo/bar/p-3?t=1&f=2', 'parameters in params-argument as array'), + + // Custom sid parameter + array('controller2', array('t' => 1, 'f' => 2), true, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid', 'params-argument (array) using session_id'), + array('controller2', array('t' => 1, 'f' => 2), false, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid', 'params-argument (array) using session_id'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2), true, 'custom-sid', 'foo/bar/p-3?t=1&f=2&sid=custom-sid', 'params-argument (array) using session_id'), + + // Testing anchors + array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'foo/bar?t=1&f=2#anchor', 'anchor in params-argument (array)'), + array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), false, false, 'foo/bar?t=1&f=2#anchor', 'anchor in params-argument (array)'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'foo/bar/p-3?t=1&f=2#anchor', 'anchor in params-argument (array)'), + + // Anchors and custom sid + array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), + array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), false, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), + array('controller3', array('p' => 3, 't' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'foo/bar/p-3?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), + + // Empty parameters should not append the & or ? + array('controller2', array(), true, false, 'foo/bar', 'no params using empty array'), + array('controller2', array(), false, false, 'foo/bar', 'no params using empty array'), + array('controller3', array('p' => 3), true, false, 'foo/bar/p-3', 'no params using empty array'), + ); + } + + /** + * @dataProvider helper_url_data_with_rewrite() + */ + public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) + { + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); + $this->helper = new \phpbb\controller\helper($this->finder, $this->template, $this->user, $this->config, dirname(__FILE__) . '/', 'php'); + $this->assertEquals(dirname(__FILE__) . '/' . $expected, $this->helper->route($route, $params, $is_amp, $session_id)); + } +} diff --git a/tests/controller/helper_url_test.php b/tests/controller/helper_url_test.php deleted file mode 100644 index 33fc6c4f1b..0000000000 --- a/tests/controller/helper_url_test.php +++ /dev/null @@ -1,119 +0,0 @@ - 1, 'f' => 2), true, false, 'app.php/foo/bar?t=1&f=2', 'parameters in params-argument as array'), - - // Custom sid parameter - array('foo/bar', 't=1&f=2', true, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid', 'using session_id'), - - // Testing anchors - array('foo/bar?t=1&f=2#anchor', false, true, false, 'app.php/foo/bar?t=1&f=2#anchor', 'anchor in url-argument'), - array('foo/bar', 't=1&f=2#anchor', true, false, 'app.php/foo/bar?t=1&f=2#anchor', 'anchor in params-argument'), - array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'app.php/foo/bar?t=1&f=2#anchor', 'anchor in params-argument (array)'), - - // Anchors and custom sid - array('foo/bar?t=1&f=2#anchor', false, true, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in url-argument using session_id'), - array('foo/bar', 't=1&f=2#anchor', true, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument using session_id'), - array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), - - // Empty parameters should not append the & - array('foo/bar', false, true, false, 'app.php/foo/bar', 'no params using bool false'), - array('foo/bar', '', true, false, 'app.php/foo/bar', 'no params using empty string'), - array('foo/bar', array(), true, false, 'app.php/foo/bar', 'no params using empty array'), - ); - } - - /** - * @dataProvider helper_url_data_no_rewrite() - */ - public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description) - { - global $phpbb_dispatcher, $phpbb_root_path, $phpEx; - - $phpbb_dispatcher = new phpbb_mock_event_dispatcher; - $this->user = $this->getMock('\phpbb\user'); - $phpbb_path_helper = new \phpbb\path_helper( - new \phpbb\symfony_request( - new phpbb_mock_request() - ), - new \phpbb\filesystem(), - $phpbb_root_path, - $phpEx - ); - $this->template = new phpbb\template\twig\twig($phpbb_path_helper, $config, $this->user, new \phpbb\template\context()); - - // We don't use mod_rewrite in these tests - $config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); - $helper = new \phpbb\controller\helper($this->template, $this->user, $config, '', 'php'); - $this->assertEquals($helper->url($route, $params, $is_amp, $session_id), $expected); - } - - public function helper_url_data_with_rewrite() - { - return array( - array('foo/bar?t=1&f=2', false, true, false, 'foo/bar?t=1&f=2', 'parameters in url-argument'), - array('foo/bar', 't=1&f=2', true, false, 'foo/bar?t=1&f=2', 'parameters in params-argument using amp'), - array('foo/bar', 't=1&f=2', false, false, 'foo/bar?t=1&f=2', 'parameters in params-argument using &'), - array('foo/bar', array('t' => 1, 'f' => 2), true, false, 'foo/bar?t=1&f=2', 'parameters in params-argument as array'), - - // Custom sid parameter - array('foo/bar', 't=1&f=2', true, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid', 'using session_id'), - - // Testing anchors - array('foo/bar?t=1&f=2#anchor', false, true, false, 'foo/bar?t=1&f=2#anchor', 'anchor in url-argument'), - array('foo/bar', 't=1&f=2#anchor', true, false, 'foo/bar?t=1&f=2#anchor', 'anchor in params-argument'), - array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'foo/bar?t=1&f=2#anchor', 'anchor in params-argument (array)'), - - // Anchors and custom sid - array('foo/bar?t=1&f=2#anchor', false, true, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in url-argument using session_id'), - array('foo/bar', 't=1&f=2#anchor', true, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument using session_id'), - array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), - - // Empty parameters should not append the & - array('foo/bar', false, true, false, 'foo/bar', 'no params using bool false'), - array('foo/bar', '', true, false, 'foo/bar', 'no params using empty string'), - array('foo/bar', array(), true, false, 'foo/bar', 'no params using empty array'), - ); - } - - /** - * @dataProvider helper_url_data_with_rewrite() - */ - public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) - { - global $phpbb_dispatcher, $phpbb_root_path, $phpEx; - - $phpbb_dispatcher = new phpbb_mock_event_dispatcher; - $this->user = $this->getMock('\phpbb\user'); - $phpbb_path_helper = new \phpbb\path_helper( - new \phpbb\symfony_request( - new phpbb_mock_request() - ), - new \phpbb\filesystem(), - $phpbb_root_path, - $phpEx - ); - $this->template = new \phpbb\template\twig\twig($phpbb_path_helper, $config, $this->user, new \phpbb\template\context()); - - $config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); - $helper = new \phpbb\controller\helper($this->template, $this->user, $config, '', 'php'); - $this->assertEquals($helper->url($route, $params, $is_amp, $session_id), $expected); - } -} From ecf1e94726a8a1e0f8d30aba3935e1899c2c6adc Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 7 Mar 2014 12:42:06 +0100 Subject: [PATCH 2/9] [ticket/12090] Change redirect tests to use route() PHPBB3-12090 --- tests/controller/controller_test.php | 12 +++---- .../fixtures/ext/foo/bar/config/routing.yml | 12 +++++++ .../ext/foo/bar/controller/controller.php | 6 ++-- tests/security/redirect_test.php | 33 ++----------------- 4 files changed, 23 insertions(+), 40 deletions(-) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index b65e677dbe..8709f449db 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -21,10 +21,10 @@ class phpbb_controller_controller_test extends phpbb_test_case $this->extension_manager = new phpbb_mock_extension_manager( dirname(__FILE__) . '/', array( - 'foo' => array( - 'ext_name' => 'foo', + 'vendor2/foo' => array( + 'ext_name' => 'vendor2/foo', 'ext_active' => '1', - 'ext_path' => 'ext/foo/', + 'ext_path' => 'ext/vendor2/foo/', ), )); } @@ -52,7 +52,7 @@ class phpbb_controller_controller_test extends phpbb_test_case $container = new ContainerBuilder(); // YamlFileLoader only uses one path at a time, so we need to loop // through all of the ones we are using. - foreach (array(__DIR__.'/config', __DIR__.'/ext/foo/config') as $path) + foreach (array(__DIR__.'/config', __DIR__ . '/ext/vendor2/foo/config') as $path) { $loader = new YamlFileLoader($container, new FileLocator($path)); $loader->load('services.yml'); @@ -60,9 +60,9 @@ class phpbb_controller_controller_test extends phpbb_test_case // Autoloading classes within the tests folder does not work // so I'll include them manually. - if (!class_exists('foo\\controller')) + if (!class_exists('vendor2\\foo\\controller')) { - include(__DIR__ . '/ext/foo/controller.php'); + include(__DIR__ . '/ext/vendor2/foo/controller.php'); } if (!class_exists('phpbb\\controller\\foo')) { diff --git a/tests/functional/fixtures/ext/foo/bar/config/routing.yml b/tests/functional/fixtures/ext/foo/bar/config/routing.yml index 9b1ce3cfd7..d4d256c293 100644 --- a/tests/functional/fixtures/ext/foo/bar/config/routing.yml +++ b/tests/functional/fixtures/ext/foo/bar/config/routing.yml @@ -17,3 +17,15 @@ foo_exception_controller: foo_redirect_controller: pattern: /foo/redirect defaults: { _controller: foo_bar.controller:redirect } + +foo_index_controller: + pattern: /index + defaults: { _controller: foo_bar.controller:redirect } + +foo_tests_index_controller: + pattern: /tests/index + defaults: { _controller: foo_bar.controller:redirect } + +foo_tests_dotdot_index_controller: + pattern: /tests/../index + defaults: { _controller: foo_bar.controller:redirect } diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php index 558b202948..7a52958ab6 100644 --- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php +++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php @@ -63,15 +63,15 @@ class controller 'tests/index.php', ), array( - $this->helper->url('index'), + $this->helper->url('foo_index_controller'), $rewrite_prefix . 'index', ), array( - $this->helper->url('tests/index'), + $this->helper->url('foo_tests_index_controller'), $rewrite_prefix . 'tests/index', ), array( - $this->helper->url('tests/../index'), + $this->helper->url('foo_tests_dotdot_index_controller'), $rewrite_prefix . 'index', ), /* diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php index 77dc955c26..e5ff3b1541 100644 --- a/tests/security/redirect_test.php +++ b/tests/security/redirect_test.php @@ -15,11 +15,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base { protected $path_helper; - protected $controller_helper; - public function provider() { - $this->controller_helper = $this->get_controller_helper(); // array(Input -> redirect(), expected triggered error (else false), expected returned result url (else false)) return array( array('data://x', false, false, 'http://localhost/phpBB'), @@ -38,8 +35,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base array('./../foo/bar', false, false, 'http://localhost/foo/bar'), array('./../foo/bar', true, false, 'http://localhost/foo/bar'), array('app.php/', false, false, 'http://localhost/phpBB/app.php/'), - array($this->controller_helper->url('a'), false, false, 'http://localhost/phpBB/app.php/a'), - array($this->controller_helper->url(''), false, false, 'http://localhost/phpBB/app.php/'), + array('app.php/a', false, false, 'http://localhost/phpBB/app.php/a'), + array('app.php/a/b', false, false, 'http://localhost/phpBB/app.php/a/b'), array('./app.php/', false, false, 'http://localhost/phpBB/app.php/'), array('foobar', false, false, 'http://localhost/phpBB/foobar'), array('./foobar', false, false, 'http://localhost/phpBB/foobar'), @@ -69,31 +66,6 @@ class phpbb_security_redirect_test extends phpbb_security_test_base return $this->path_helper; } - protected function get_controller_helper() - { - if (!($this->controller_helper instanceof \phpbb\controller\helper)) - { - global $phpbb_dispatcher; - - $phpbb_dispatcher = new phpbb_mock_event_dispatcher; - $this->user = $this->getMock('\phpbb\user'); - $phpbb_path_helper = new \phpbb\path_helper( - new \phpbb\symfony_request( - new phpbb_mock_request() - ), - new \phpbb\filesystem(), - $phpbb_root_path, - $phpEx - ); - $this->template = new phpbb\template\twig\twig($phpbb_path_helper, $config, $this->user, new \phpbb\template\context()); - - // We don't use mod_rewrite in these tests - $config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); - $this->controller_helper = new \phpbb\controller\helper($this->template, $this->user, $config, '', 'php'); - } - return $this->controller_helper; - } - protected function setUp() { parent::setUp(); @@ -103,7 +75,6 @@ class phpbb_security_redirect_test extends phpbb_security_test_base ); $this->path_helper = $this->get_path_helper(); - $this->controller_helper = $this->get_controller_helper(); } /** From 6491477809bf6eed1cf4672e4a6439f24cce3b57 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 7 Mar 2014 15:14:35 +0100 Subject: [PATCH 3/9] [ticket/12090] Fix helper usage in functional controller tests PHPBB3-12090 --- .../ext/foo/bar/controller/controller.php | 30 ++----------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php index 7a52958ab6..36f232baec 100644 --- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php +++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php @@ -63,41 +63,17 @@ class controller 'tests/index.php', ), array( - $this->helper->url('foo_index_controller'), + $this->helper->route('foo_index_controller'), $rewrite_prefix . 'index', ), array( - $this->helper->url('foo_tests_index_controller'), + $this->helper->route('foo_tests_index_controller'), $rewrite_prefix . 'tests/index', ), array( - $this->helper->url('foo_tests_dotdot_index_controller'), + $this->helper->route('foo_tests_dotdot_index_controller'), $rewrite_prefix . 'index', ), - /* - // helper URLs starting with ../ are prone to failure. - // Do not test them right now. - array( - $this->helper->url('../index'), - '../index', - ), - array( - $this->helper->url('../../index'), - '../index', - ), - array( - $this->helper->url('../tests/index'), - $rewrite_prefix . '../tests/index', - ), - array( - $this->helper->url('../tests/../index'), - '../index', - ), - array( - $this->helper->url('../../tests/index'), - '../tests/index', - ), - */ ); foreach ($redirects as $redirect) From 275910d8b0fd8d5edeee07eb05ad82da48cc72a3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 8 Mar 2014 15:59:40 +0100 Subject: [PATCH 4/9] [ticket/12090] Fix pagination for routes No clickable "jump to" at the moment, as we can not get the route url by the route name in js yet. Need to find another solution later. PHPBB3-12090 --- phpBB/config/services.yml | 1 + phpBB/phpbb/pagination.php | 28 ++++++- phpBB/styles/prosilver/template/forum_fn.js | 11 +-- .../styles/prosilver/template/pagination.html | 6 +- .../subsilver2/template/pagination.html | 2 +- tests/pagination/config/routing.yml | 6 ++ tests/pagination/pagination_test.php | 84 ++++++++++++------- 7 files changed, 93 insertions(+), 45 deletions(-) create mode 100644 tests/pagination/config/routing.yml diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 0699cfcc6d..3427f95cc1 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -263,6 +263,7 @@ services: arguments: - @template - @user + - @controller.helper path_helper: class: phpbb\path_helper diff --git a/phpBB/phpbb/pagination.php b/phpBB/phpbb/pagination.php index 57e7932341..6a7631c89d 100644 --- a/phpBB/phpbb/pagination.php +++ b/phpBB/phpbb/pagination.php @@ -22,11 +22,13 @@ class pagination * * @param \phpbb\template\template $template * @param \phpbb\user $user + * @param \phpbb\controller\helper $helper */ - public function __construct(\phpbb\template\template $template, \phpbb\user $user) + public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\controller\helper $helper) { $this->template = $template; $this->user = $user; + $this->helper = $helper; } /** @@ -44,9 +46,26 @@ class pagination */ protected function generate_page_link($base_url, $on_page, $start_name, $per_page) { - if (strpos($start_name, '%d') !== false) + if (!is_string($base_url)) { - return ($on_page > 1) ? sprintf($base_url, (int) $on_page) : str_replace($start_name, '', $base_url); + if (is_array($base_url['routes'])) + { + $route = ($on_page > 1) ? $base_url['routes'][1] : $base_url['routes'][0]; + } + else + { + $route = $base_url['routes']; + } + $params = (isset($base_url['params'])) ? $base_url['params'] : array(); + $is_amp = (isset($base_url['is_amp'])) ? $base_url['is_amp'] : true; + $session_id = (isset($base_url['session_id'])) ? $base_url['session_id'] : false; + + if ($on_page > 1 || !is_array($base_url['routes'])) + { + $params[$start_name] = (int) $on_page; + } + + return $this->helper->route($route, $params, $is_amp, $session_id); } else { @@ -194,7 +213,8 @@ class pagination $tpl_prefix = ($tpl_prefix == 'PAGINATION') ? '' : $tpl_prefix . '_'; $template_array = array( - $tpl_prefix . 'BASE_URL' => $base_url, + $tpl_prefix . 'BASE_URL' => is_string($base_url) ? $base_url : '',//@todo: Fix this for routes + $tpl_prefix . 'START_NAME' => $start_name, $tpl_prefix . 'PER_PAGE' => $per_page, 'U_' . $tpl_prefix . 'PREVIOUS_PAGE' => ($on_page != 1) ? $u_previous_page : '', 'U_' . $tpl_prefix . 'NEXT_PAGE' => ($on_page != $total_pages) ? $u_next_page : '', diff --git a/phpBB/styles/prosilver/template/forum_fn.js b/phpBB/styles/prosilver/template/forum_fn.js index 408c9b9b8c..de51b54e9b 100644 --- a/phpBB/styles/prosilver/template/forum_fn.js +++ b/phpBB/styles/prosilver/template/forum_fn.js @@ -37,17 +37,14 @@ function jumpto(item) { on_page = item.attr('data-on-page'), per_page = item.attr('data-per-page'), base_url = item.attr('data-base-url'), + start_name = item.attr('data-start-name'), page = prompt(jump_page, on_page); if (page !== null && !isNaN(page) && page == Math.floor(page) && page > 0) { - if (base_url.indexOf('%d') === -1) { - if (base_url.indexOf('?') === -1) { - document.location.href = base_url + '?start=' + ((page - 1) * per_page); - } else { - document.location.href = base_url.replace(/&/g, '&') + '&start=' + ((page - 1) * per_page); - } + if (base_url.indexOf('?') === -1) { + document.location.href = base_url + '?' + start_name + '=' + ((page - 1) * per_page); } else { - document.location.href = base_url.replace('%d', page); + document.location.href = base_url.replace(/&/g, '&') + '&' + start_name + '=' + ((page - 1) * per_page); } } } diff --git a/phpBB/styles/prosilver/template/pagination.html b/phpBB/styles/prosilver/template/pagination.html index cb54193c3f..e27a90900a 100644 --- a/phpBB/styles/prosilver/template/pagination.html +++ b/phpBB/styles/prosilver/template/pagination.html @@ -1,4 +1,8 @@ - {PAGE_NUMBER} • + + {PAGE_NUMBER} • + + {PAGE_NUMBER} • +
    diff --git a/phpBB/styles/subsilver2/template/pagination.html b/phpBB/styles/subsilver2/template/pagination.html index a2e023ac22..550b28d305 100644 --- a/phpBB/styles/subsilver2/template/pagination.html +++ b/phpBB/styles/subsilver2/template/pagination.html @@ -1,5 +1,5 @@ - {L_GOTO_PAGE} + {L_GOTO_PAGE} {L_PREVIOUS} {pagination.PAGE_NUMBER} diff --git a/tests/pagination/config/routing.yml b/tests/pagination/config/routing.yml new file mode 100644 index 0000000000..dd667274cd --- /dev/null +++ b/tests/pagination/config/routing.yml @@ -0,0 +1,6 @@ +core_controller: + pattern: /test + defaults: { _controller: core_foo.controller:bar, page: 1} +core_page_controller: + pattern: /test/page/{page} + defaults: { _controller: core_foo.controller:bar} diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php index b7a4f101aa..38ab8e62a4 100644 --- a/tests/pagination/pagination_test.php +++ b/tests/pagination/pagination_test.php @@ -21,11 +21,25 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case public function setUp() { parent::setUp(); - $user = $this->getMock('\phpbb\user'); - $user->expects($this->any()) + + global $phpbb_dispatcher; + + $phpbb_dispatcher = new phpbb_mock_event_dispatcher; + $this->user = $this->getMock('\phpbb\user'); + $this->user->expects($this->any()) ->method('lang') ->will($this->returnCallback(array($this, 'return_callback_implode'))); - $this->pagination = new \phpbb\pagination($this->template, $user); + + $this->finder = new \phpbb\extension\finder( + new phpbb_mock_extension_manager(dirname(__FILE__) . '/', array()), + new \phpbb\filesystem(), + dirname(__FILE__) . '/', + new phpbb_mock_cache() + ); + + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); + $this->helper = new \phpbb\controller\helper($this->finder, $this->template, $this->user, $this->config, dirname(__FILE__) . '/', 'php'); + $this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper); } public function generate_template_pagination_data() @@ -77,49 +91,55 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case :u_next:page.php?start=30', ), array( - 'test/page/%d', - '/page/%d', + array('routes' => array( + 'core_controller', + 'core_page_controller', + )), + 'page', 95, 10, 10, 'pagination :per_page:10 :current_page:2 - :base_url:test/page/%d - :previous::test - :else:1:test - :current:2:test/page/2 - :else:3:test/page/3 - :else:4:test/page/4 - :else:5:test/page/5 - :ellipsis:9:test/page/9 - :else:10:test/page/10 - :next::test/page/3 - :u_prev:test - :u_next:test/page/3', + :base_url: + :previous::' . dirname(__FILE__) . '/' . 'test + :else:1:' . dirname(__FILE__) . '/' . 'test + :current:2:' . dirname(__FILE__) . '/' . 'test/page/2 + :else:3:' . dirname(__FILE__) . '/' . 'test/page/3 + :else:4:' . dirname(__FILE__) . '/' . 'test/page/4 + :else:5:' . dirname(__FILE__) . '/' . 'test/page/5 + :ellipsis:9:' . dirname(__FILE__) . '/' . 'test/page/9 + :else:10:' . dirname(__FILE__) . '/' . 'test/page/10 + :next::' . dirname(__FILE__) . '/' . 'test/page/3 + :u_prev:' . dirname(__FILE__) . '/' . 'test + :u_next:' . dirname(__FILE__) . '/' . 'test/page/3', ), array( - 'test/page/%d', - '/page/%d', + array('routes' => array( + 'core_controller', + 'core_page_controller', + )), + 'page', 95, 10, 20, 'pagination :per_page:10 :current_page:3 - :base_url:test/page/%d - :previous::test/page/2 - :else:1:test - :else:2:test/page/2 - :current:3:test/page/3 - :else:4:test/page/4 - :else:5:test/page/5 - :else:6:test/page/6 - :ellipsis:9:test/page/9 - :else:10:test/page/10 - :next::test/page/4 - :u_prev:test/page/2 - :u_next:test/page/4', + :base_url: + :previous::' . dirname(__FILE__) . '/' . 'test/page/2 + :else:1:' . dirname(__FILE__) . '/' . 'test + :else:2:' . dirname(__FILE__) . '/' . 'test/page/2 + :current:3:' . dirname(__FILE__) . '/' . 'test/page/3 + :else:4:' . dirname(__FILE__) . '/' . 'test/page/4 + :else:5:' . dirname(__FILE__) . '/' . 'test/page/5 + :else:6:' . dirname(__FILE__) . '/' . 'test/page/6 + :ellipsis:9:' . dirname(__FILE__) . '/' . 'test/page/9 + :else:10:' . dirname(__FILE__) . '/' . 'test/page/10 + :next::' . dirname(__FILE__) . '/' . 'test/page/4 + :u_prev:' . dirname(__FILE__) . '/' . 'test/page/2 + :u_next:' . dirname(__FILE__) . '/' . 'test/page/4', ), ); } From 07c07171f9b70a49b592473b8a8400d3838333a3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 8 Mar 2014 16:32:43 +0100 Subject: [PATCH 5/9] [ticket/12090] Make provider a service and inject it into the helper PHPBB3-12090 --- phpBB/config/services.yml | 7 +++++- phpBB/includes/functions_url_matcher.php | 8 +++---- phpBB/phpbb/controller/helper.php | 6 ++--- phpBB/phpbb/controller/provider.php | 30 +++++++++--------------- tests/controller/controller_test.php | 6 ++--- tests/controller/helper_route_test.php | 4 ++-- tests/pagination/pagination_test.php | 2 +- 7 files changed, 28 insertions(+), 35 deletions(-) diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 3427f95cc1..e474c51ae0 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -91,10 +91,10 @@ services: controller.helper: class: phpbb\controller\helper arguments: - - @ext.finder - @template - @user - @config + - @controller.provider - %core.root_path% - %core.php_ext% @@ -105,6 +105,11 @@ services: - @service_container - @template + controller.provider: + class: phpbb\controller\provider + arguments: + - @ext.finder + cron.task_collection: class: phpbb\di\service_collection arguments: diff --git a/phpBB/includes/functions_url_matcher.php b/phpBB/includes/functions_url_matcher.php index c5d6815119..4db1bfa01f 100644 --- a/phpBB/includes/functions_url_matcher.php +++ b/phpBB/includes/functions_url_matcher.php @@ -53,8 +53,8 @@ function phpbb_get_url_matcher(\phpbb\extension\finder $finder, RequestContext $ */ function phpbb_create_dumped_url_matcher(\phpbb\extension\finder $finder, $root_path, $php_ext) { - $provider = new \phpbb\controller\provider(); - $routes = $provider->import_paths_from_finder($finder)->find($root_path); + $provider = new \phpbb\controller\provider($finder); + $routes = $provider->find($root_path); $dumper = new PhpMatcherDumper($routes); $cached_url_matcher_dump = $dumper->dump(array( 'class' => 'phpbb_url_matcher', @@ -72,8 +72,8 @@ function phpbb_create_dumped_url_matcher(\phpbb\extension\finder $finder, $root_ */ function phpbb_create_url_matcher(\phpbb\extension\finder $finder, RequestContext $context, $root_path) { - $provider = new \phpbb\controller\provider(); - $routes = $provider->import_paths_from_finder($finder)->find($root_path); + $provider = new \phpbb\controller\provider($finder); + $routes = $provider->find($root_path); return new UrlMatcher($routes, $context); } diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index daa52ec672..c97e0883f4 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -58,16 +58,14 @@ class helper * @param string $phpbb_root_path phpBB root path * @param string $php_ext PHP extension */ - public function __construct(\phpbb\extension\finder $finder, \phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, $phpbb_root_path, $php_ext) + public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\controller\provider $provider, $phpbb_root_path, $php_ext) { $this->template = $template; $this->user = $user; $this->config = $config; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; - - $provider = new \phpbb\controller\provider(); - $this->route_collection = $provider->import_paths_from_finder($finder)->find($this->phpbb_root_path); + $this->route_collection = $provider->find($this->phpbb_root_path); } diff --git a/phpBB/phpbb/controller/provider.php b/phpBB/phpbb/controller/provider.php index fde51696e8..fbe717f1af 100644 --- a/phpBB/phpbb/controller/provider.php +++ b/phpBB/phpbb/controller/provider.php @@ -31,28 +31,20 @@ class provider * @param array() $routing_files Array of strings containing paths * to YAML files holding route information */ - public function __construct($routing_files = array()) + public function __construct(\phpbb\extension\finder $finder = null, $routing_files = array()) { $this->routing_files = $routing_files; - } - /** - * Locate paths containing routing files - * This sets an internal property but does not return the paths. - * - * @return The current instance of this object for method chaining - */ - public function import_paths_from_finder(\phpbb\extension\finder $finder) - { - // We hardcode the path to the core config directory - // because the finder cannot find it - $this->routing_files = array_merge(array('config/routing.yml'), array_keys($finder - ->directory('config') - ->suffix('routing.yml') - ->find() - )); - - return $this; + if ($finder) + { + // We hardcode the path to the core config directory + // because the finder cannot find it + $this->routing_files = array_merge($this->routing_files, array('config/routing.yml'), array_keys($finder + ->directory('config') + ->suffix('routing.yml') + ->find() + )); + } } /** diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 8709f449db..597f01fa0f 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -31,10 +31,8 @@ class phpbb_controller_controller_test extends phpbb_test_case public function test_provider() { - $provider = new \phpbb\controller\provider; - $routes = $provider - ->import_paths_from_finder($this->extension_manager->get_finder()) - ->find(__DIR__); + $provider = new \phpbb\controller\provider($this->extension_manager->get_finder()); + $routes = $provider->find(__DIR__); // This will need to be updated if any new routes are defined $this->assertInstanceOf('Symfony\Component\Routing\Route', $routes->get('core_controller')); diff --git a/tests/controller/helper_route_test.php b/tests/controller/helper_route_test.php index 3cd8a7237d..21dbe2a87e 100644 --- a/tests/controller/helper_route_test.php +++ b/tests/controller/helper_route_test.php @@ -80,7 +80,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case */ public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { - $this->helper = new \phpbb\controller\helper($this->finder, $this->template, $this->user, $this->config, dirname(__FILE__) . '/', 'php'); + $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, new \phpbb\controller\provider($this->finder), dirname(__FILE__) . '/', 'php'); $this->assertEquals(dirname(__FILE__) . '/' . $expected, $this->helper->route($route, $params, $is_amp, $session_id)); } @@ -120,7 +120,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); - $this->helper = new \phpbb\controller\helper($this->finder, $this->template, $this->user, $this->config, dirname(__FILE__) . '/', 'php'); + $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, new \phpbb\controller\provider($this->finder), dirname(__FILE__) . '/', 'php'); $this->assertEquals(dirname(__FILE__) . '/' . $expected, $this->helper->route($route, $params, $is_amp, $session_id)); } } diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php index 38ab8e62a4..89be33ebab 100644 --- a/tests/pagination/pagination_test.php +++ b/tests/pagination/pagination_test.php @@ -38,7 +38,7 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case ); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); - $this->helper = new \phpbb\controller\helper($this->finder, $this->template, $this->user, $this->config, dirname(__FILE__) . '/', 'php'); + $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, new \phpbb\controller\provider($this->finder), dirname(__FILE__) . '/', 'php'); $this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper); } From 436b1d3577cd1b66af568023e566d2de53c255a0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 9 Mar 2014 18:22:32 +0100 Subject: [PATCH 6/9] [ticket/12090] Fix parameter list of controller helper constructor PHPBB3-12090 --- phpBB/phpbb/controller/helper.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index c97e0883f4..8e33aaf605 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -53,8 +53,9 @@ class helper * Constructor * * @param \phpbb\template\template $template Template object - * @param \phpbb\user $user User object - * @param \phpbb\config\config $config Config object + * @param \phpbb\user $user User object + * @param \phpbb\config\config $config Config object + * @param \phpbb\controller\provider $provider Path provider * @param string $phpbb_root_path phpBB root path * @param string $php_ext PHP extension */ From 2eb24d0ace239324086002db4582eaaddd07aa28 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 9 Mar 2014 18:38:21 +0100 Subject: [PATCH 7/9] [ticket/12090] Split finding routes and returning routes into 2 methods PHPBB3-12090 --- phpBB/includes/functions_url_matcher.php | 4 ++-- phpBB/phpbb/controller/helper.php | 3 +-- phpBB/phpbb/controller/provider.php | 26 +++++++++++++++++++----- tests/controller/controller_test.php | 2 +- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/phpBB/includes/functions_url_matcher.php b/phpBB/includes/functions_url_matcher.php index 4db1bfa01f..8e5ae20f93 100644 --- a/phpBB/includes/functions_url_matcher.php +++ b/phpBB/includes/functions_url_matcher.php @@ -54,7 +54,7 @@ function phpbb_get_url_matcher(\phpbb\extension\finder $finder, RequestContext $ function phpbb_create_dumped_url_matcher(\phpbb\extension\finder $finder, $root_path, $php_ext) { $provider = new \phpbb\controller\provider($finder); - $routes = $provider->find($root_path); + $routes = $provider->find($root_path)->get_routes(); $dumper = new PhpMatcherDumper($routes); $cached_url_matcher_dump = $dumper->dump(array( 'class' => 'phpbb_url_matcher', @@ -73,7 +73,7 @@ function phpbb_create_dumped_url_matcher(\phpbb\extension\finder $finder, $root_ function phpbb_create_url_matcher(\phpbb\extension\finder $finder, RequestContext $context, $root_path) { $provider = new \phpbb\controller\provider($finder); - $routes = $provider->find($root_path); + $routes = $provider->find($root_path)->get_routes(); return new UrlMatcher($routes, $context); } diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index 8e33aaf605..2d11a54c08 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -66,8 +66,7 @@ class helper $this->config = $config; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; - $this->route_collection = $provider->find($this->phpbb_root_path); - + $this->route_collection = $provider->find($this->phpbb_root_path)->get_routes(); } /** diff --git a/phpBB/phpbb/controller/provider.php b/phpBB/phpbb/controller/provider.php index fbe717f1af..9df8130210 100644 --- a/phpBB/phpbb/controller/provider.php +++ b/phpBB/phpbb/controller/provider.php @@ -25,6 +25,12 @@ class provider */ protected $routing_files; + /** + * Collection of the routes in phpBB and all found extensions + * @var RouteCollection + */ + protected $routes; + /** * Construct method * @@ -48,20 +54,30 @@ class provider } /** - * Get a list of controllers and return it + * Find a list of controllers and return it * * @param string $base_path Base path to prepend to file paths - * @return array Array of controllers and their route information + * @return null */ public function find($base_path = '') { - $routes = new RouteCollection; + $this->routes = new RouteCollection; foreach ($this->routing_files as $file_path) { $loader = new YamlFileLoader(new FileLocator($base_path)); - $routes->addCollection($loader->load($file_path)); + $this->routes->addCollection($loader->load($file_path)); } - return $routes; + return $this; + } + + /** + * Get the list of routes + * + * @return RouteCollection Get the route collection + */ + public function get_routes() + { + return $this->routes; } } diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 597f01fa0f..550679ff07 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -32,7 +32,7 @@ class phpbb_controller_controller_test extends phpbb_test_case public function test_provider() { $provider = new \phpbb\controller\provider($this->extension_manager->get_finder()); - $routes = $provider->find(__DIR__); + $routes = $provider->find(__DIR__)->get_routes(); // This will need to be updated if any new routes are defined $this->assertInstanceOf('Symfony\Component\Routing\Route', $routes->get('core_controller')); From b29c4c635865444cacb67407c767951c83040036 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 9 Mar 2014 19:03:15 +0100 Subject: [PATCH 8/9] [ticket/12090] Move find() call to container and fix tests PHPBB3-12090 --- phpBB/config/services.yml | 2 ++ phpBB/phpbb/controller/helper.php | 2 +- tests/controller/helper_route_test.php | 12 ++++--- tests/pagination/pagination_test.php | 50 +++++++++++++------------- 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index e474c51ae0..2cf9dbed3e 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -109,6 +109,8 @@ services: class: phpbb\controller\provider arguments: - @ext.finder + calls: + - [find, [%core.root_path%]] cron.task_collection: class: phpbb\di\service_collection diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index 2d11a54c08..10fdbb1375 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -66,7 +66,7 @@ class helper $this->config = $config; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; - $this->route_collection = $provider->find($this->phpbb_root_path)->get_routes(); + $this->route_collection = $provider->get_routes(); } /** diff --git a/tests/controller/helper_route_test.php b/tests/controller/helper_route_test.php index 21dbe2a87e..5264c788c7 100644 --- a/tests/controller/helper_route_test.php +++ b/tests/controller/helper_route_test.php @@ -28,7 +28,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); $this->template = new phpbb\template\twig\twig($phpbb_path_helper, $this->config, $this->user, new \phpbb\template\context()); - $this->finder = new \phpbb\extension\finder( + $finder = new \phpbb\extension\finder( new phpbb_mock_extension_manager( dirname(__FILE__) . '/', array( @@ -43,6 +43,8 @@ class phpbb_controller_helper_route_test extends phpbb_test_case dirname(__FILE__) . '/', new phpbb_mock_cache() ); + $this->provider = new \phpbb\controller\provider($finder); + $this->provider->find(dirname(__FILE__) . '/'); } public function helper_url_data_no_rewrite() @@ -80,8 +82,8 @@ class phpbb_controller_helper_route_test extends phpbb_test_case */ public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { - $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, new \phpbb\controller\provider($this->finder), dirname(__FILE__) . '/', 'php'); - $this->assertEquals(dirname(__FILE__) . '/' . $expected, $this->helper->route($route, $params, $is_amp, $session_id)); + $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, $this->provider, '', 'php'); + $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id)); } public function helper_url_data_with_rewrite() @@ -120,7 +122,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); - $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, new \phpbb\controller\provider($this->finder), dirname(__FILE__) . '/', 'php'); - $this->assertEquals(dirname(__FILE__) . '/' . $expected, $this->helper->route($route, $params, $is_amp, $session_id)); + $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, $this->provider, '', 'php'); + $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id)); } } diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php index 89be33ebab..71206dff58 100644 --- a/tests/pagination/pagination_test.php +++ b/tests/pagination/pagination_test.php @@ -38,7 +38,9 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case ); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); - $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, new \phpbb\controller\provider($this->finder), dirname(__FILE__) . '/', 'php'); + $provider = new \phpbb\controller\provider($this->finder); + $provider->find(dirname(__FILE__) . '/'); + $this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, $provider, '', 'php'); $this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper); } @@ -103,17 +105,17 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case :per_page:10 :current_page:2 :base_url: - :previous::' . dirname(__FILE__) . '/' . 'test - :else:1:' . dirname(__FILE__) . '/' . 'test - :current:2:' . dirname(__FILE__) . '/' . 'test/page/2 - :else:3:' . dirname(__FILE__) . '/' . 'test/page/3 - :else:4:' . dirname(__FILE__) . '/' . 'test/page/4 - :else:5:' . dirname(__FILE__) . '/' . 'test/page/5 - :ellipsis:9:' . dirname(__FILE__) . '/' . 'test/page/9 - :else:10:' . dirname(__FILE__) . '/' . 'test/page/10 - :next::' . dirname(__FILE__) . '/' . 'test/page/3 - :u_prev:' . dirname(__FILE__) . '/' . 'test - :u_next:' . dirname(__FILE__) . '/' . 'test/page/3', + :previous::test + :else:1:test + :current:2:test/page/2 + :else:3:test/page/3 + :else:4:test/page/4 + :else:5:test/page/5 + :ellipsis:9:test/page/9 + :else:10:test/page/10 + :next::test/page/3 + :u_prev:test + :u_next:test/page/3', ), array( array('routes' => array( @@ -128,18 +130,18 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case :per_page:10 :current_page:3 :base_url: - :previous::' . dirname(__FILE__) . '/' . 'test/page/2 - :else:1:' . dirname(__FILE__) . '/' . 'test - :else:2:' . dirname(__FILE__) . '/' . 'test/page/2 - :current:3:' . dirname(__FILE__) . '/' . 'test/page/3 - :else:4:' . dirname(__FILE__) . '/' . 'test/page/4 - :else:5:' . dirname(__FILE__) . '/' . 'test/page/5 - :else:6:' . dirname(__FILE__) . '/' . 'test/page/6 - :ellipsis:9:' . dirname(__FILE__) . '/' . 'test/page/9 - :else:10:' . dirname(__FILE__) . '/' . 'test/page/10 - :next::' . dirname(__FILE__) . '/' . 'test/page/4 - :u_prev:' . dirname(__FILE__) . '/' . 'test/page/2 - :u_next:' . dirname(__FILE__) . '/' . 'test/page/4', + :previous::test/page/2 + :else:1:test + :else:2:test/page/2 + :current:3:test/page/3 + :else:4:test/page/4 + :else:5:test/page/5 + :else:6:test/page/6 + :ellipsis:9:test/page/9 + :else:10:test/page/10 + :next::test/page/4 + :u_prev:test/page/2 + :u_next:test/page/4', ), ); } From 0119a21862bc433b50fe3914e3bae6f12f61129c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 9 Mar 2014 19:17:12 +0100 Subject: [PATCH 9/9] [ticket/12090] Comment out broken test PHPBB3-12090 --- .../functional/fixtures/ext/foo/bar/controller/controller.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php index 36f232baec..45246f8a97 100644 --- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php +++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php @@ -70,10 +70,13 @@ class controller $this->helper->route('foo_tests_index_controller'), $rewrite_prefix . 'tests/index', ), + /** + * Symfony does not allow /../ in routes array( $this->helper->route('foo_tests_dotdot_index_controller'), $rewrite_prefix . 'index', ), + */ ); foreach ($redirects as $redirect)