[ticket/11850] page_name contains controller request rather than query string

Fixing tests

PHPBB3-11850

phpBB/phpbb/session.php

@@ -87,6 +87,12 @@ class phpbb_session
 		$page_name = (substr($script_name, -1, 1) == '/') ? '' : basename($script_name);
 		$page_name = urlencode(htmlspecialchars($page_name));
 
+		$symfony_request_path = $phpbb_filesystem->clean_path($symfony_request->getPathInfo());
+		if ($symfony_request_path !== '/')
+		{
+			$page_name .= $symfony_request_path;
+		}
+
 		// current directory within the phpBB root (for example: adm)
 		$root_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($root_path)));
 		$page_dirs = explode('/', str_replace('\\', '/', phpbb_realpath('./')));
@@ -103,12 +109,7 @@ class phpbb_session
 		}
 
 		// Current page from phpBB root (for example: adm/index.php?i=10&b=2)
-		$symfony_request_path = $phpbb_filesystem->clean_path($symfony_request->getPathInfo());
 		$page = (($page_dir) ? $page_dir . '/' : '') . $page_name;
-		if ($symfony_request_path !== '/')
-		{
-			$page .= $symfony_request_path;
-		}
 		if ($query_string)
 		{
 			$page .= '?' . $query_string;

tests/session/extract_page_test.php

@@ -24,6 +24,7 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case
 				'/phpBB/index.php',
 				'',
 				'/phpBB/',
+				'/',
 				array(
 					'page_name' => 'index.php',
 					'page_dir' => '',
@@ -38,7 +39,8 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case
 				'./',
 				'/phpBB/ucp.php',
 				'mode=login',
-				'/phpBB/ucp.php?mode=login',
+				'/phpBB/',
+				'/',
 				array(
 					'page_name' => 'ucp.php',
 					'page_dir' => '',
@@ -53,7 +55,8 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case
 				'./',
 				'/phpBB/ucp.php',
 				'mode=register',
-				'/phpBB/ucp.php?mode=register',
+				'/phpBB/',
+				'/',
 				array(
 					'page_name' => 'ucp.php',
 					'page_dir' => '',
@@ -68,7 +71,8 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case
 				'./',
 				'/phpBB/ucp.php',
 				'mode=register',
-				'/phpBB/ucp.php?mode=register',
+				'/phpBB/',
+				'/',
 				array(
 					'page_name' => 'ucp.php',
 					'page_dir' => '',
@@ -83,30 +87,60 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case
 				'./../',
 				'/phpBB/adm/index.php',
 				'sid=e7215d958cdd41a6fc13509bebe53e42',
-				'/phpBB/adm/index.php?sid=e7215d958cdd41a6fc13509bebe53e42',
+				'/phpBB/adm/',
+				'/',
 				array(
 					'page_name' => 'index.php',
 					//'page_dir' => 'adm',
 					// ^-- Ignored because .. returns different directory in live vs testing
 					'query_string' => '',
 					'script_path' => '/phpBB/adm/',
-					'root_script_path' => '/phpBB/',
+					//'root_script_path' => '/phpBB/',
 					//'page' => 'adm/index.php',
 					'forum' => 0,
 				),
 			),
+			array(
+				'./',
+				'/phpBB/adm/app.php',
+				'page=1&test=2',
+				'/phpBB/',
+				'/foo/bar',
+				array(
+					'page_name' => 'app.php/foo/bar',
+					'page_dir' => '',
+					'query_string' => 'page=1&test=2',
+					'script_path' => '/phpBB/',
+					'root_script_path' => '/phpBB/',
+					'page' => 'app.php/foo/bar?page=1&test=2',
+					'forum' => 0,
+				),
+			),
 		);
 	}
 
 	/** @dataProvider extract_current_page_data */
-	function test_extract_current_page($root_path, $php_self, $query_string, $request_uri, $expected)
+	function test_extract_current_page($root_path, $getScriptName, $getQueryString, $getBasePath, $getPathInfo, $expected)
 	{
-		$output = $this->session_facade->extract_current_page(
-			$root_path,
-			$php_self,
-			$query_string,
-			$request_uri
-		);
+		global $symfony_request;
+
+		$symfony_request = $this->getMock("phpbb_symfony_request", array(), array(
+			new phpbb_mock_request(),
+		));
+		$symfony_request->expects($this->any())
+			->method('getScriptName')
+			->will($this->returnValue($getScriptName));
+		$symfony_request->expects($this->any())
+			->method('getQueryString')
+			->will($this->returnValue($getQueryString));
+		$symfony_request->expects($this->any())
+			->method('getBasePath')
+			->will($this->returnValue($getBasePath));
+		$symfony_request->expects($this->any())
+			->method('getPathInfo')
+			->will($this->returnValue($getPathInfo));
+
+		$output = phpbb_session::extract_current_page($root_path);
 
 		// This compares the result of the output.
 		// Any keys that are not in the expected array are overwritten by the output (aka not checked).

tests/session/testable_facade.php

@@ -33,21 +33,6 @@ class phpbb_session_testable_facade
 		$this->session_factory = $session_factory;
 	}
 
-	function extract_current_page(
-		$root_path,
-		$php_self, 
-		$query_string,
-		$request_uri
-	) 
-	{
-		$this->session_factory->get_session($this->db);
-		global $request;
-		$request->overwrite('PHP_SELF', $php_self, phpbb_request_interface::SERVER);
-		$request->overwrite('QUERY_STRING', $query_string, phpbb_request_interface::SERVER);
-		$request->overwrite('REQUEST_URI', $request_uri, phpbb_request_interface::SERVER);
-		return phpbb_session::extract_current_page($root_path);
-	}
-
 	function extract_current_hostname(
 		$host,
 		$server_name_config,
@@ -139,4 +124,3 @@ class phpbb_session_testable_facade
 		return $session->validate_referer($check_script_path);
 	}
 }
-

tests/test_framework/phpbb_session_test_case.php

@@ -19,6 +19,13 @@ abstract class phpbb_session_test_case extends phpbb_database_test_case
 	function setUp()
 	{
 		parent::setUp();
+
+		global $symfony_request, $phpbb_filesystem, $request, $phpbb_root_path, $phpEx;
+		$symfony_request = new phpbb_symfony_request(
+			new phpbb_mock_request()
+		);
+		$phpbb_filesystem = new phpbb_filesystem($symfony_request, $phpbb_root_path, $phpEx);
+
 		$this->session_factory = new phpbb_session_testable_factory;
 		$this->db = $this->new_dbal();
 		$this->session_facade =