Merge pull request #3055 from marc1706/ticket/13192

[ticket/13192] Add method for generating valid user page links based on mod_rewrite

* marc1706/ticket/13192:
  [ticket/13192] Add test for app.php in external subfolder
  [ticket/13192] Use ltrim() instead of preg_replace()
  [ticket/13192] Order test cases consistently
  [ticket/13192] Remove app.php on mod rewrite even if app.php is outside root
  [ticket/13192] Pass correct parameters and rename method to get_valid_page
  [ticket/13192] Use get_valid_user_page in confirm_box() and cleanup globals
  [ticket/13192] Use get_valid_user_page method in build_url function
  [ticket/13192] Add method for generating valid user page links

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 = $phpbb_path_helper->get_valid_page($user->page['page'], $config['enable_mod_rewrite']);
-	$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;
-	}
 
 	// Append SID
 	$redirect = append_sid($page, false, false);
@@ -2657,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 $config, $phpbb_path_helper;
 
 	if (isset($_POST['cancel']))
 	{
@@ -2719,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']);
+	$use_page = ($u_action) ? $u_action : str_replace('&', '&', $user->page['page']);
-	$u_action = reapply_sid($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(

phpBB/phpbb/path_helper.php

@@ -455,4 +455,38 @@ class path_helper
 
 		return $url_parts['base'] . (($params) ? '?' . $this->glue_url_params($params) : '');
 	}
+
+	/**
+	 * Get a valid page
+	 *
+	 * @param string $page The page to verify
+	 * @param bool $mod_rewrite Whether mod_rewrite is enabled, default: false
+	 *
+	 * @return string A valid page based on given page and mod_rewrite
+	 */
+	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($page);
+
+		// URL
+		if ($url_parts === false || empty($url_parts['scheme']) || empty($url_parts['host']))
+		{
+			// 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, 0, $app_position) . substr($page, $app_position + strlen('app.' . $this->php_ext . '/'));
+			}
+
+			// Remove preceding slashes from page name and prepend root path
+			$page = $this->get_phpbb_root_path() . ltrim($page, '/\\');
+		}
+
+		return $page;
+	}
 }

tests/path_helper/path_helper_test.php

@@ -436,4 +436,29 @@ 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_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', true, 'foo'),
+			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'),
+		);
+	}
+
+	/**
+	 * @dataProvider data_get_valid_page
+	 */
+	public function test_get_valid_page($page, $mod_rewrite, $expected)
+	{
+		$this->assertEquals($this->phpbb_root_path . $expected, $this->path_helper->get_valid_page($page, $mod_rewrite));
+	}
 }