From 0a7db81426e966f9c44fcc85338615e1bb3c6f34 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 12 Nov 2013 22:25:23 +0100 Subject: [PATCH 1/3] [ticket/11997] Fix redirects from inside controllers The redirect url currently uses the web root path. However as we prepend the full board url later, we need to remove the relative web root path and prepend the normal root path again. Otherwise redirects from inside routes will not work as intended. PHPBB3-11997 --- phpBB/includes/functions.php | 10 ++++++++++ phpBB/phpbb/path_helper.php | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 0a10a9604c..c937b8f562 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2662,6 +2662,16 @@ function redirect($url, $return = false, $disable_cd_check = false) // Make sure no &'s are in, this will break the redirect $url = str_replace('&', '&', $url); + // The url currently uses the web root path. + // However as we prepend the full board url later, + // we need to remove the relative web root path and + // prepend the normal root path again. Otherwise redirects + // from inside routes will not work as intended. + if ($phpbb_path_helper instanceof \phpbb\path_helper) + { + $url = $phpbb_path_helper->remove_web_root_path($url); + } + // Determine which type of redirect we need to handle... $url_parts = @parse_url($url); diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index 8cd8808261..71252ac05b 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -101,6 +101,27 @@ class path_helper return $path; } + /** + * Strips away the web root path and prepends the normal root path + * + * This replaces get_web_root_path() . some_url with + * $phpbb_root_path . some_url + * + * @param string $path The path to be updated + * @return string + */ + public function remove_web_root_path($path) + { + if (strpos($path, $this->get_web_root_path()) === 0) + { + $path = substr($path, strlen($this->get_web_root_path())); + + return $this->phpbb_root_path . $path; + } + + return $path; + } + /** * Get a relative root path from the current URL * From 0aed2816761601ae902159665fdf58932209f772 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 13 Nov 2013 11:21:47 +0100 Subject: [PATCH 2/3] [ticket/11997] Fix missing global PHPBB3-11997 --- phpBB/includes/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index c937b8f562..a6c03098c2 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2645,7 +2645,7 @@ function generate_board_url($without_script_path = false) */ function redirect($url, $return = false, $disable_cd_check = false) { - global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem; + global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper; $failover_flag = false; From a0fca0acc24684615d123d71ce696e43ba4e2615 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 13 Nov 2013 12:03:06 +0100 Subject: [PATCH 3/3] [ticket/11997] Add functional test for redirects in controller PHPBB3-11997 --- .../functional/extension_controller_test.php | 32 +++++++++++++++++++ .../fixtures/ext/foo/bar/config/routing.yml | 4 +++ .../fixtures/ext/foo/bar/config/services.yml | 2 ++ .../ext/foo/bar/controller/controller.php | 30 ++++++++++++++++- .../prosilver/template/redirect_body.html | 5 +++ 5 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index 41bd48c204..b280f01638 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -109,4 +109,36 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c $this->assert_response_html(404); $this->assertContains('No route found for "GET /does/not/exist"', $crawler->filter('body')->text()); } + + /** + * Check the output of a controller using the template system + */ + public function test_redirect() + { + $this->phpbb_extension_manager->enable('foo/bar'); + $crawler = self::request('GET', 'app.php/foo/redirect'); + + $test_redirects = array( + 'index.php', + '../index.php', + 'tests/index.php', + '../tests/index.php', + 'app.php/index', + 'index', + '../index', + 'app.php/tests/index', + 'tests/index', + '../tests/index', + 'index', + ); + + $filesystem = new \phpbb\filesystem(); + + foreach ($test_redirects as $row_num => $redirect) + { + $this->assertContains($filesystem->clean_path(self::$root_url . $redirect), $crawler->filter('#redirect_' . $row_num)->text()); + } + + $this->phpbb_extension_manager->purge('foo/bar'); + } } diff --git a/tests/functional/fixtures/ext/foo/bar/config/routing.yml b/tests/functional/fixtures/ext/foo/bar/config/routing.yml index 09a30a8c67..9b1ce3cfd7 100644 --- a/tests/functional/fixtures/ext/foo/bar/config/routing.yml +++ b/tests/functional/fixtures/ext/foo/bar/config/routing.yml @@ -13,3 +13,7 @@ foo_template_controller: foo_exception_controller: pattern: /foo/exception defaults: { _controller: foo_bar.controller:exception } + +foo_redirect_controller: + pattern: /foo/redirect + defaults: { _controller: foo_bar.controller:redirect } diff --git a/tests/functional/fixtures/ext/foo/bar/config/services.yml b/tests/functional/fixtures/ext/foo/bar/config/services.yml index 3bca4c6567..d7012dee1c 100644 --- a/tests/functional/fixtures/ext/foo/bar/config/services.yml +++ b/tests/functional/fixtures/ext/foo/bar/config/services.yml @@ -4,3 +4,5 @@ services: arguments: - @controller.helper - @template + - %core.root_path% + - %core.php_ext% diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php index 259d548299..ebb259af2f 100644 --- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php +++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php @@ -8,10 +8,12 @@ class controller { protected $template; - public function __construct(\phpbb\controller\helper $helper, \phpbb\template\template $template) + public function __construct(\phpbb\controller\helper $helper, \phpbb\template\template $template, $root_path, $php_ext) { $this->template = $template; $this->helper = $helper; + $this->root_path = $root_path; + $this->php_ext = $php_ext; } public function handle() @@ -35,4 +37,30 @@ class controller { throw new \phpbb\controller\exception('Exception thrown from foo/exception route'); } + + public function redirect() + { + $redirects = array( + append_sid($this->root_path . 'index.' . $this->php_ext), + append_sid($this->root_path . '../index.' . $this->php_ext), + append_sid($this->root_path . 'tests/index.' . $this->php_ext), + append_sid($this->root_path . '../tests/index.' . $this->php_ext), + $this->helper->url('index'), + $this->helper->url('../index'), + $this->helper->url('../../index'), + $this->helper->url('tests/index'), + $this->helper->url('../tests/index'), + $this->helper->url('../../tests/index'), + $this->helper->url('../tests/../index'), + ); + + foreach ($redirects as $redirect) + { + $this->template->assign_block_vars('redirects', array( + 'URL' => redirect($redirect, true), + )); + } + + return $this->helper->render('redirect_body.html'); + } } diff --git a/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html new file mode 100644 index 0000000000..0c261f10ea --- /dev/null +++ b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html @@ -0,0 +1,5 @@ + + +
{redirects.URL}
+ +