diff options
| -rw-r--r-- | phpBB/phpbb/session.php | 20 | ||||
| -rw-r--r-- | tests/security/base.php | 18 | ||||
| -rw-r--r-- | tests/security/extract_current_page_test.php | 42 | ||||
| -rw-r--r-- | tests/session/extract_page_test.php | 74 | ||||
| -rw-r--r-- | tests/session/testable_facade.php | 16 | ||||
| -rw-r--r-- | tests/test_framework/phpbb_session_test_case.php | 7 |
6 files changed, 132 insertions, 45 deletions
diff --git a/phpBB/phpbb/session.php b/phpBB/phpbb/session.php index 2baf61043d..5e4380bfc8 100644 --- a/phpBB/phpbb/session.php +++ b/phpBB/phpbb/session.php @@ -42,13 +42,13 @@ class session */ static function extract_current_page($root_path) { - global $request; + global $request, $symfony_request, $phpbb_filesystem; $page_array = array(); // First of all, get the request uri... - $script_name = htmlspecialchars_decode($request->server('PHP_SELF')); - $args = explode('&', htmlspecialchars_decode($request->server('QUERY_STRING'))); + $script_name = $symfony_request->getScriptName(); + $args = explode('&', $symfony_request->getQueryString()); // If we are unable to get the script name we use REQUEST_URI as a failover and note it within the page array for easier support... if (!$script_name) @@ -89,6 +89,12 @@ class 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('./'))); @@ -105,10 +111,14 @@ class session } // Current page from phpBB root (for example: adm/index.php?i=10&b=2) - $page = (($page_dir) ? $page_dir . '/' : '') . $page_name . (($query_string) ? "?$query_string" : ''); + $page = (($page_dir) ? $page_dir . '/' : '') . $page_name; + if ($query_string) + { + $page .= '?' . $query_string; + } // The script path from the webroot to the current directory (for example: /phpBB3/adm/) : always prefixed with / and ends in / - $script_path = trim(str_replace('\\', '/', dirname($script_name))); + $script_path = $symfony_request->getBasePath(); // The script path from the webroot to the phpBB root (for example: /phpBB3/) $script_dirs = explode('/', $script_path); diff --git a/tests/security/base.php b/tests/security/base.php index 8cd24ff145..26f267745c 100644 --- a/tests/security/base.php +++ b/tests/security/base.php @@ -14,7 +14,7 @@ abstract class phpbb_security_test_base extends phpbb_test_case */ protected function setUp() { - global $user, $phpbb_root_path, $request; + global $user, $phpbb_root_path, $phpEx, $request, $symfony_request, $phpbb_filesystem; // Put this into a global function being run by every test to init a proper user session $server['HTTP_HOST'] = 'localhost'; @@ -37,6 +37,22 @@ abstract class phpbb_security_test_base extends phpbb_test_case */ $request = new phpbb_mock_request(array(), array(), array(), $server); + $symfony_request = $this->getMock("phpbb_symfony_request", array(), array( + $request, + )); + $symfony_request->expects($this->any()) + ->method('getScriptName') + ->will($this->returnValue($server['SCRIPT_NAME'])); + $symfony_request->expects($this->any()) + ->method('getQueryString') + ->will($this->returnValue($server['QUERY_STRING'])); + $symfony_request->expects($this->any()) + ->method('getBasePath') + ->will($this->returnValue($server['REQUEST_URI'])); + $symfony_request->expects($this->any()) + ->method('getPathInfo') + ->will($this->returnValue('/')); + $phpbb_filesystem = new phpbb_filesystem($symfony_request, $phpbb_root_path, $phpEx); // Set no user and trick a bit to circumvent errors $user = new \phpbb\user(); diff --git a/tests/security/extract_current_page_test.php b/tests/security/extract_current_page_test.php index e42f446b31..a7560f0d15 100644 --- a/tests/security/extract_current_page_test.php +++ b/tests/security/extract_current_page_test.php @@ -26,14 +26,24 @@ class phpbb_security_extract_current_page_test extends phpbb_security_test_base */ public function test_query_string_php_self($url, $query_string, $expected) { - global $request; + global $symfony_request, $request; - $request->merge(\phpbb\request\request_interface::SERVER, array( - 'PHP_SELF' => $url, - 'QUERY_STRING' => $query_string, + $symfony_request = $this->getMock("phpbb_symfony_request", array(), array( + $request, )); - - $result = \phpbb\session::extract_current_page('./'); + $symfony_request->expects($this->any()) + ->method('getScriptName') + ->will($this->returnValue($url)); + $symfony_request->expects($this->any()) + ->method('getQueryString') + ->will($this->returnValue($query_string)); + $symfony_request->expects($this->any()) + ->method('getBasePath') + ->will($this->returnValue($server['REQUEST_URI'])); + $symfony_request->expects($this->any()) + ->method('getPathInfo') + ->will($this->returnValue('/')); + $result = phpbb_session::extract_current_page('./'); $label = 'Running extract_current_page on ' . $query_string . ' with PHP_SELF filled.'; $this->assertEquals($expected, $result['query_string'], $label); @@ -44,12 +54,23 @@ class phpbb_security_extract_current_page_test extends phpbb_security_test_base */ public function test_query_string_request_uri($url, $query_string, $expected) { - global $request; + global $symfony_request, $request; - $request->merge(\phpbb\request\request_interface::SERVER, array( - 'PHP_SELF' => $url, - 'QUERY_STRING' => $query_string, + $symfony_request = $this->getMock("phpbb_symfony_request", array(), array( + $request, )); + $symfony_request->expects($this->any()) + ->method('getScriptName') + ->will($this->returnValue($url)); + $symfony_request->expects($this->any()) + ->method('getQueryString') + ->will($this->returnValue($query_string)); + $symfony_request->expects($this->any()) + ->method('getBasePath') + ->will($this->returnValue($server['REQUEST_URI'])); + $symfony_request->expects($this->any()) + ->method('getPathInfo') + ->will($this->returnValue('/')); $result = \phpbb\session::extract_current_page('./'); @@ -57,4 +78,3 @@ class phpbb_security_extract_current_page_test extends phpbb_security_test_base $this->assertEquals($expected, $result['query_string'], $label); } } - diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index f4ae8de021..1834eddebc 100644 --- a/tests/session/extract_page_test.php +++ b/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,76 @@ 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, + ), + ), + array( + './../phpBB/', + '/test/test.php', + 'page=1&test=2', + '/test/', + '', + array( + 'page_name' => 'test.php', + //'page_dir' => '', + 'query_string' => 'page=1&test=2', + 'script_path' => '/test/', + //'root_script_path' => '../phpBB/', + //'page' => '../test/test.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). diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index f289c48f69..2600a46cc4 100644 --- a/tests/session/testable_facade.php +++ b/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\request_interface::SERVER); - $request->overwrite('QUERY_STRING', $query_string, \phpbb\request\request_interface::SERVER); - $request->overwrite('REQUEST_URI', $request_uri, \phpbb\request\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); } } - diff --git a/tests/test_framework/phpbb_session_test_case.php b/tests/test_framework/phpbb_session_test_case.php index e6a2b03bba..cfa8399c96 100644 --- a/tests/test_framework/phpbb_session_test_case.php +++ b/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 = |