From 40497ec824344116143bc30b84fe8eb1c1971ebf Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 21 Oct 2014 22:16:53 -0500 Subject: [PATCH 1/8] [ticket/13192] Add method for generating valid user page links PHPBB3-13192 --- phpBB/phpbb/path_helper.php | 31 ++++++++++++++++++++++++++ tests/path_helper/path_helper_test.php | 21 +++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index 936564d8b6..77f123bf2c 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -445,4 +445,35 @@ class path_helper return $url_parts['base'] . (($params) ? '?' . $this->glue_url_params($params) : ''); } + + /** + * Get a valid user page + * + * @param string $user_page The current user page + * @param bool $mod_rewrite Whether mod_rewrite is enabled, default: false + * + * @return string A valid user page based on user page and mod_rewrite + */ + public function get_valid_user_page($user_page, $mod_rewrite = false) + { + // We need to be cautious here. + // On some situations, the redirect path is an absolute URL, sometimes a relative path + // For a relative path, let's prefix it with $phpbb_root_path to point to the correct location, + // else we use the URL directly. + $url_parts = parse_url($user_page); + + // URL + if ($url_parts === false || empty($url_parts['scheme']) || empty($url_parts['host'])) + { + // Remove 'app.php/' from the page, when rewrite is enabled + if ($mod_rewrite && strpos($user_page, 'app.' . $this->php_ext . '/') === 0) + { + $user_page = substr($user_page, strlen('app.' . $this->php_ext . '/')); + } + + $user_page = $this->get_phpbb_root_path() . $user_page; + } + + return $user_page; + } } diff --git a/tests/path_helper/path_helper_test.php b/tests/path_helper/path_helper_test.php index 3832307897..f2c2e0b102 100644 --- a/tests/path_helper/path_helper_test.php +++ b/tests/path_helper/path_helper_test.php @@ -421,4 +421,25 @@ class phpbb_path_helper_test extends phpbb_test_case { $this->assertEquals($this->phpbb_root_path . $expected, $this->path_helper->get_web_root_path_from_ajax_referer($referer_url, $board_url)); } + + public function data_get_valid_user_page() + { + return array( + // array( current page , mod_rewrite setting , expected output ) + array('index', true, 'index'), + array('index', false, 'index'), + array('foo/index', true, 'foo/index'), + array('foo/index', false, 'foo/index'), + array('app.php/foo', false, 'app.php/foo'), + array('app.php/foo', true, 'foo'), + ); + } + + /** + * @dataProvider data_get_valid_user_page + */ + public function test_get_valid_user_page($page, $mod_rewrite, $expected) + { + $this->assertEquals($this->phpbb_root_path . $expected, $this->path_helper->get_valid_user_page($page, $mod_rewrite)); + } } From c381ad2002546042de5a71dedbea1a7d45d1e2d8 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 21 Oct 2014 22:17:24 -0500 Subject: [PATCH 2/8] [ticket/13192] Use get_valid_user_page method in build_url function PHPBB3-13192 --- phpBB/includes/functions.php | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 7700dcfd27..d1d0f8f681 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2396,26 +2396,7 @@ function build_url($strip_vars = false) { global $config, $user, $phpbb_path_helper; - $php_ext = $phpbb_path_helper->get_php_ext(); - $page = $user->page['page']; - - // We need to be cautious here. - // On some situations, the redirect path is an absolute URL, sometimes a relative path - // For a relative path, let's prefix it with $phpbb_root_path to point to the correct location, - // else we use the URL directly. - $url_parts = parse_url($page); - - // URL - if ($url_parts === false || empty($url_parts['scheme']) || empty($url_parts['host'])) - { - // Remove 'app.php/' from the page, when rewrite is enabled - if ($config['enable_mod_rewrite'] && strpos($page, 'app.' . $php_ext . '/') === 0) - { - $page = substr($page, strlen('app.' . $php_ext . '/')); - } - - $page = $phpbb_path_helper->get_phpbb_root_path() . $page; - } + $page = $phpbb_path_helper->get_valid_user_page($user->page['page'], $config['enable_mod_rewrite']); // Append SID $redirect = append_sid($page, false, false); From ce8c09f51f47b5e6806d51da643fba82d2341372 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 21 Oct 2014 22:17:48 -0500 Subject: [PATCH 3/8] [ticket/13192] Use get_valid_user_page in confirm_box() and cleanup globals The $request global existed twice and the $phpEx global is not being used in confirm_box(). PHPBB3-13192 --- phpBB/includes/functions.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index d1d0f8f681..d11aadacfa 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2638,7 +2638,7 @@ function check_form_key($form_name, $timespan = false) function confirm_box($check, $title = '', $hidden = '', $html_body = 'confirm_body.html', $u_action = '') { global $user, $template, $db, $request; - global $phpEx, $phpbb_root_path, $request; + global $phpbb_path_helper, $phpbb_root_path; if (isset($_POST['cancel'])) { @@ -2700,8 +2700,8 @@ function confirm_box($check, $title = '', $hidden = '', $html_body = 'confirm_bo } // re-add sid / transform & to & for user->page (user->page is always using &) - $use_page = ($u_action) ? $phpbb_root_path . $u_action : $phpbb_root_path . str_replace('&', '&', $user->page['page']); - $u_action = reapply_sid($use_page); + $use_page = ($u_action) ? $u_action : str_replace('&', '&', $user->page['page']); + $u_action = reapply_sid($phpbb_path_helper->get_valid_user_page($use_page)); $u_action .= ((strpos($u_action, '?') === false) ? '?' : '&') . 'confirm_key=' . $confirm_key; $template->assign_vars(array( From a623868f20574e19d6840af11bce8836ad436e95 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 21 Oct 2014 22:38:03 -0500 Subject: [PATCH 4/8] [ticket/13192] Pass correct parameters and rename method to get_valid_page PHPBB3-13192 --- phpBB/includes/functions.php | 6 +++--- phpBB/phpbb/path_helper.php | 18 +++++++++--------- tests/path_helper/path_helper_test.php | 8 ++++---- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index d11aadacfa..169c741ecf 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2396,7 +2396,7 @@ function build_url($strip_vars = false) { global $config, $user, $phpbb_path_helper; - $page = $phpbb_path_helper->get_valid_user_page($user->page['page'], $config['enable_mod_rewrite']); + $page = $phpbb_path_helper->get_valid_page($user->page['page'], $config['enable_mod_rewrite']); // Append SID $redirect = append_sid($page, false, false); @@ -2638,7 +2638,7 @@ function check_form_key($form_name, $timespan = false) function confirm_box($check, $title = '', $hidden = '', $html_body = 'confirm_body.html', $u_action = '') { global $user, $template, $db, $request; - global $phpbb_path_helper, $phpbb_root_path; + global $config, $phpbb_path_helper; if (isset($_POST['cancel'])) { @@ -2701,7 +2701,7 @@ function confirm_box($check, $title = '', $hidden = '', $html_body = 'confirm_bo // re-add sid / transform & to & for user->page (user->page is always using &) $use_page = ($u_action) ? $u_action : str_replace('&', '&', $user->page['page']); - $u_action = reapply_sid($phpbb_path_helper->get_valid_user_page($use_page)); + $u_action = reapply_sid($phpbb_path_helper->get_valid_page($use_page, $config['enable_mod_rewrite'])); $u_action .= ((strpos($u_action, '?') === false) ? '?' : '&') . 'confirm_key=' . $confirm_key; $template->assign_vars(array( diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index 77f123bf2c..0a41efc128 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -447,33 +447,33 @@ class path_helper } /** - * Get a valid user page + * Get a valid page * - * @param string $user_page The current user page + * @param string $page The page to verify * @param bool $mod_rewrite Whether mod_rewrite is enabled, default: false * - * @return string A valid user page based on user page and mod_rewrite + * @return string A valid page based on given page and mod_rewrite */ - public function get_valid_user_page($user_page, $mod_rewrite = false) + public function get_valid_page($page, $mod_rewrite = false) { // We need to be cautious here. // On some situations, the redirect path is an absolute URL, sometimes a relative path // For a relative path, let's prefix it with $phpbb_root_path to point to the correct location, // else we use the URL directly. - $url_parts = parse_url($user_page); + $url_parts = parse_url($page); // URL if ($url_parts === false || empty($url_parts['scheme']) || empty($url_parts['host'])) { // Remove 'app.php/' from the page, when rewrite is enabled - if ($mod_rewrite && strpos($user_page, 'app.' . $this->php_ext . '/') === 0) + if ($mod_rewrite && strpos($page, 'app.' . $this->php_ext . '/') === 0) { - $user_page = substr($user_page, strlen('app.' . $this->php_ext . '/')); + $page = substr($page, strlen('app.' . $this->php_ext . '/')); } - $user_page = $this->get_phpbb_root_path() . $user_page; + $page = $this->get_phpbb_root_path() . $page; } - return $user_page; + return $page; } } diff --git a/tests/path_helper/path_helper_test.php b/tests/path_helper/path_helper_test.php index f2c2e0b102..26cb940b54 100644 --- a/tests/path_helper/path_helper_test.php +++ b/tests/path_helper/path_helper_test.php @@ -422,7 +422,7 @@ class phpbb_path_helper_test extends phpbb_test_case $this->assertEquals($this->phpbb_root_path . $expected, $this->path_helper->get_web_root_path_from_ajax_referer($referer_url, $board_url)); } - public function data_get_valid_user_page() + public function data_get_valid_page() { return array( // array( current page , mod_rewrite setting , expected output ) @@ -436,10 +436,10 @@ class phpbb_path_helper_test extends phpbb_test_case } /** - * @dataProvider data_get_valid_user_page + * @dataProvider data_get_valid_page */ - public function test_get_valid_user_page($page, $mod_rewrite, $expected) + public function test_get_valid_page($page, $mod_rewrite, $expected) { - $this->assertEquals($this->phpbb_root_path . $expected, $this->path_helper->get_valid_user_page($page, $mod_rewrite)); + $this->assertEquals($this->phpbb_root_path . $expected, $this->path_helper->get_valid_page($page, $mod_rewrite)); } } From 281cc5353208258e7f4a9032f720c5f1ae0fb8dc Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 2 Nov 2014 12:01:42 +0100 Subject: [PATCH 5/8] [ticket/13192] Remove app.php on mod rewrite even if app.php is outside root PHPBB3-13192 --- phpBB/phpbb/path_helper.php | 11 +++++++---- tests/path_helper/path_helper_test.php | 2 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index 0a41efc128..b2ec9d98e0 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -465,13 +465,16 @@ class path_helper // URL if ($url_parts === false || empty($url_parts['scheme']) || empty($url_parts['host'])) { - // Remove 'app.php/' from the page, when rewrite is enabled - if ($mod_rewrite && strpos($page, 'app.' . $this->php_ext . '/') === 0) + // Remove 'app.php/' from the page, when rewrite is enabled. + // Treat app.php as a reserved file name and remove on mod rewrite + // even if it might not be in the phpBB root. + if ($mod_rewrite && ($app_position = strpos($page, 'app.' . $this->php_ext . '/')) !== false) { - $page = substr($page, strlen('app.' . $this->php_ext . '/')); + $page = substr($page, 0, $app_position) . substr($page, $app_position + strlen('app.' . $this->php_ext . '/')); } - $page = $this->get_phpbb_root_path() . $page; + // Remove preceding slashes from page name and prepend root path + $page = $this->get_phpbb_root_path() . preg_replace('@^(?:([\\/\\\])?)@', '', $page); } return $page; diff --git a/tests/path_helper/path_helper_test.php b/tests/path_helper/path_helper_test.php index 26cb940b54..62c2a24b22 100644 --- a/tests/path_helper/path_helper_test.php +++ b/tests/path_helper/path_helper_test.php @@ -432,6 +432,8 @@ class phpbb_path_helper_test extends phpbb_test_case array('foo/index', false, 'foo/index'), array('app.php/foo', false, 'app.php/foo'), array('app.php/foo', true, 'foo'), + array('/../app.php/foo', false, '../app.php/foo'), + array('/../app.php/foo', true, '../foo'), ); } From 7ba1a9642715945f7e2641cfda7e1d188ef2a8d2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Jan 2015 13:38:52 +0100 Subject: [PATCH 6/8] [ticket/13192] Order test cases consistently PHPBB3-13192 --- tests/path_helper/path_helper_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/path_helper/path_helper_test.php b/tests/path_helper/path_helper_test.php index 62c2a24b22..3b25eba716 100644 --- a/tests/path_helper/path_helper_test.php +++ b/tests/path_helper/path_helper_test.php @@ -430,10 +430,10 @@ class phpbb_path_helper_test extends phpbb_test_case array('index', false, 'index'), array('foo/index', true, 'foo/index'), array('foo/index', false, 'foo/index'), - array('app.php/foo', false, 'app.php/foo'), array('app.php/foo', true, 'foo'), - array('/../app.php/foo', false, '../app.php/foo'), + array('app.php/foo', false, 'app.php/foo'), array('/../app.php/foo', true, '../foo'), + array('/../app.php/foo', false, '../app.php/foo'), ); } From e6509aaf606264414f30248afd1081ff05207328 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 10 Jan 2015 12:46:40 +0100 Subject: [PATCH 7/8] [ticket/13192] Use ltrim() instead of preg_replace() PHPBB3-13192 --- phpBB/phpbb/path_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index b2ec9d98e0..6feb64b07e 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -474,7 +474,7 @@ class path_helper } // Remove preceding slashes from page name and prepend root path - $page = $this->get_phpbb_root_path() . preg_replace('@^(?:([\\/\\\])?)@', '', $page); + $page = $this->get_phpbb_root_path() . ltrim($page, '/\\'); } return $page; From da189a105b41736f9e47c2f560f242d7844f9b43 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 19 Jan 2015 16:10:32 +0100 Subject: [PATCH 8/8] [ticket/13192] Add test for app.php in external subfolder PHPBB3-13192 --- tests/path_helper/path_helper_test.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/path_helper/path_helper_test.php b/tests/path_helper/path_helper_test.php index 3b25eba716..f33509c975 100644 --- a/tests/path_helper/path_helper_test.php +++ b/tests/path_helper/path_helper_test.php @@ -434,6 +434,8 @@ class phpbb_path_helper_test extends phpbb_test_case array('app.php/foo', false, 'app.php/foo'), array('/../app.php/foo', true, '../foo'), array('/../app.php/foo', false, '../app.php/foo'), + array('/../example/app.php/foo/bar', true, '../example/foo/bar'), + array('/../example/app.php/foo/bar', false, '../example/app.php/foo/bar'), ); }