From 8223a956dfe69a564764049a371b23c7d5b8584f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 26 Jun 2023 23:14:21 +0200 Subject: [PATCH 1/4] [ticket/17141] Ensure correction is newer below 0 PHPBB3-17141 --- phpBB/phpbb/path_helper.php | 4 +- tests/path_helper/path_helper_test.php | 90 ++++++++++++++++---------- 2 files changed, 58 insertions(+), 36 deletions(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index 5b6db35f23..281c12b375 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -236,7 +236,7 @@ class path_helper // Prepend ../ to the phpbb_root_path as many times as / exists in path_info $this->web_root_path = $this->filesystem->clean_path( - './' . str_repeat('../', $corrections) . $this->phpbb_root_path + './' . str_repeat('../', max(0, $corrections)) . $this->phpbb_root_path ); return $this->web_root_path; } @@ -264,7 +264,7 @@ class path_helper $relative_referer_path = substr($relative_referer_path, 0, $has_params); } $corrections = substr_count($relative_referer_path, '/'); - return $this->phpbb_root_path . str_repeat('../', $corrections - 1); + return $this->phpbb_root_path . str_repeat('../', max(0, $corrections - 1)); } // If not, it's a bit more complicated. We go to the parent directory diff --git a/tests/path_helper/path_helper_test.php b/tests/path_helper/path_helper_test.php index b9d043da28..ff0098cb5a 100644 --- a/tests/path_helper/path_helper_test.php +++ b/tests/path_helper/path_helper_test.php @@ -59,25 +59,25 @@ class phpbb_path_helper_test extends phpbb_test_case $filesystem = new \phpbb\filesystem\filesystem(); $this->set_phpbb_root_path($filesystem); - return array( - array( + return [ + [ 'http://www.test.com/test.php', 'http://www.test.com/test.php', '/', - ), - array( + ], + [ $this->phpbb_root_path . 'test.php', $this->phpbb_root_path . 'test.php', - ), - array( + ], + [ 'test.php', 'test.php', - ), - array( + ], + [ $this->phpbb_root_path . $this->phpbb_root_path . 'test.php', $filesystem->clean_path($this->phpbb_root_path . $this->phpbb_root_path . 'test.php'), - ), - ); + ], + ]; } /** @@ -158,6 +158,13 @@ class phpbb_path_helper_test extends phpbb_test_case '/phpbb3-fork/phpBB/app.php', '', ), + array( + './../'.$this->phpbb_root_path . 'test.php', + '', + '/phpbb3-fork/phpBB/foo', + '/phpbb3-fork/phpBB/app.php', + '', + ), ); } @@ -393,63 +400,78 @@ class phpbb_path_helper_test extends phpbb_test_case public function get_web_root_path_from_ajax_referer_data() { - return array( - array( + return [ + [ 'http://www.phpbb.com/community/route1/route2/', 'http://www.phpbb.com/community', '../../', - ), - array( + ], + [ + 'http://www.phpbb.com/community/route1/route2/?f=9', + 'http://www.phpbb.com/community', + '../../', + ], + [ 'http://www.phpbb.com/community/route1/route2', 'http://www.phpbb.com/community', '../', - ), - array( + ], + [ 'http://www.phpbb.com/community/route1', 'http://www.phpbb.com/community', '', - ), - array( + ], + [ 'http://www.phpbb.com/community/', 'http://www.phpbb.com/community', '', - ), - array( + ], + [ 'http://www.phpbb.com/notcommunity/route1/route2/', 'http://www.phpbb.com/community', '../../../community/', - ), - array( + ], + [ + 'http://www.phpbb.com/notcommunity/route1/route2/?f=9', + 'http://www.phpbb.com/community', + '../../../community/', + ], + [ 'http://www.phpbb.com/notcommunity/route1/route2', 'http://www.phpbb.com/community', '../../community/', - ), - array( + ], + [ 'http://www.phpbb.com/notcommunity/route1', 'http://www.phpbb.com/community', '../community/', - ), - array( + ], + [ 'http://www.phpbb.com/notcommunity/', 'http://www.phpbb.com/community', '../community/', - ), - array( + ], + [ 'http://www.phpbb.com/foobar', 'http://www.phpbb.com', '', - ), - array( + ], + [ 'http://www.foobar.com', 'http://www.phpbb.com', '/www.phpbb.com/', - ), - array( + ], + [ 'foobar', 'http://www.phpbb.com/community', '', - ) - ); + ], + [ + 'https://www.phpbb.com', + 'https://www.phpbb.com', + '' + ] + ]; } /** From 55ea8d603021546d19595250ba614a8013b19966 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 26 Jun 2023 23:14:58 +0200 Subject: [PATCH 2/4] [ticket/17141] Symfony request can never be null so don't check for it PHPBB3-17141 --- phpBB/phpbb/path_helper.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index 281c12b375..92635c95d0 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -151,11 +151,6 @@ class path_helper */ public function get_web_root_path() { - if ($this->symfony_request === null) - { - return $this->phpbb_root_path; - } - if (null !== $this->web_root_path) { return $this->web_root_path; From b0c008690a5d46d01719173b5969d0ae456fb0ac Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 26 Jun 2023 23:15:23 +0200 Subject: [PATCH 3/4] [ticket/17141] Remove duplicate prepending with phpbb_root_path PHPBB3-17141 --- 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 92635c95d0..167073fa25 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -213,7 +213,7 @@ class path_helper $this->symfony_request->get('_referer'), $absolute_board_url ); - return $this->web_root_path = $this->phpbb_root_path . $referer_web_root_path; + return $this->web_root_path = $referer_web_root_path; } // How many corrections might we need? From 91f216d9ce7dd5a7270918c9f6eb1cdf4af3e10d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 26 Jun 2023 23:15:55 +0200 Subject: [PATCH 4/4] [ticket/17141] Fully cover path_helper with unit tests PHPBB3-17141 --- tests/path_helper/path_helper_test.php | 151 ++++++++++++++++++++++++- 1 file changed, 149 insertions(+), 2 deletions(-) diff --git a/tests/path_helper/path_helper_test.php b/tests/path_helper/path_helper_test.php index ff0098cb5a..8d010c2f06 100644 --- a/tests/path_helper/path_helper_test.php +++ b/tests/path_helper/path_helper_test.php @@ -31,7 +31,8 @@ class phpbb_path_helper_test extends phpbb_test_case new \phpbb\filesystem\filesystem(), $this->createMock('\phpbb\request\request'), $this->phpbb_root_path, - 'php' + 'php', + 'adm/' ); } @@ -50,8 +51,20 @@ class phpbb_path_helper_test extends phpbb_test_case public function test_get_web_root_path() { - // Symfony Request = null, so always should return phpbb_root_path $this->assertEquals($this->phpbb_root_path, $this->path_helper->get_web_root_path()); + + // Second call will use class property + $this->assertEquals($this->phpbb_root_path, $this->path_helper->get_web_root_path()); + } + + public function test_get_adm_relative_path() + { + $this->assertEquals( 'adm/', $this->path_helper->get_adm_relative_path()); + } + + public function test_get_php_ext() + { + $this->assertSame('php', $this->path_helper->get_php_ext()); } public function basic_update_web_root_path_data() @@ -88,6 +101,26 @@ class phpbb_path_helper_test extends phpbb_test_case $this->assertEquals($expected, $this->path_helper->update_web_root_path($input)); } + public function test_update_web_root_path_app() + { + $path_helper = $this->getMockBuilder('\phpbb\path_helper') + ->setConstructorArgs([ + new \phpbb\symfony_request( + new phpbb_mock_request() + ), + new \phpbb\filesystem\filesystem(), + $this->createMock('\phpbb\request\request'), + $this->phpbb_root_path, + 'php', + 'adm/' + ]) + ->setMethods(['get_web_root_path']) + ->getMock(); + $path_helper->method('get_web_root_path') + ->willReturn('/var/www/phpbb/app.php/'); + $this->assertEquals('/var/www/phpbb/app.php/foo', $path_helper->update_web_root_path($this->phpbb_root_path . 'app.php/foo')); + } + public function update_web_root_path_data() { $this->set_phpbb_root_path(new \phpbb\filesystem\filesystem()); @@ -197,6 +230,47 @@ class phpbb_path_helper_test extends phpbb_test_case $this->assertEquals($correction . $input, $path_helper->update_web_root_path($input, $symfony_request)); } + public function remove_web_root_path_data() + { + $filesystem = new \phpbb\filesystem\filesystem(); + $this->set_phpbb_root_path($filesystem); + + return [ + [ + 'web/root/path/some_url', + 'web/root/path/some_url' + ], + [ + '/var/www/phpbb/test.php', + $this->phpbb_root_path . 'test.php' + ] + ]; + } + + /** + * @dataProvider remove_web_root_path_data + */ + public function test_remove_web_root_path($input, $expected) + { + $path_helper = $this->getMockBuilder('\phpbb\path_helper') + ->setConstructorArgs([ + new \phpbb\symfony_request( + new phpbb_mock_request() + ), + new \phpbb\filesystem\filesystem(), + $this->createMock('\phpbb\request\request'), + $this->phpbb_root_path, + 'php', + 'adm/' + ]) + ->setMethods(['get_web_root_path']) + ->getMock(); + $path_helper->method('get_web_root_path') + ->willReturn('/var/www/phpbb/'); + + $this->assertEquals($expected, $path_helper->remove_web_root_path($input)); + } + public function clean_url_data() { return array( @@ -390,6 +464,41 @@ class phpbb_path_helper_test extends phpbb_test_case ); } + public function test_get_web_root_path_ajax() + { + $symfony_request = $this->getMockBuilder('\phpbb\symfony_request') + ->setConstructorArgs([new phpbb_mock_request()]) + ->setMethods(['get', 'getSchemeAndHttpHost', 'getBasePath', 'getPathInfo']) + ->getMock(); + $symfony_request->method('get') + ->with('_referer') + ->willReturn('http://www.phpbb.com/community/route1/route2/'); + $symfony_request->method('getSchemeAndHttpHost') + ->willReturn('http://www.phpbb.com'); + $symfony_request->method('getBasePath') + ->willReturn('/community'); + $symfony_request->expects($this->any()) + ->method('getPathInfo') + ->will($this->returnValue('foo/bar')); + + $request = $this->createMock('phpbb\request\request'); + $request->method('is_ajax') + ->willReturn(true); + $request->method('escape') + ->willReturnArgument(0); + + $path_helper = new \phpbb\path_helper( + $symfony_request, + new \phpbb\filesystem\filesystem(), + $request, + $this->phpbb_root_path, + 'php', + 'adm/' + ); + + $this->assertEquals($this->phpbb_root_path . '../../', $path_helper->get_web_root_path()); + } + /** * @dataProvider append_url_params_data */ @@ -506,4 +615,42 @@ class phpbb_path_helper_test extends phpbb_test_case { $this->assertEquals($this->phpbb_root_path . $expected, $this->path_helper->get_valid_page($page, $mod_rewrite)); } + + public function is_router_used_data() + { + return [ + [ + 'index.php', + false, + ], + [ + 'app.php', + true, + ], + ]; + } + + /** + * @dataProvider is_router_used_data + */ + public function test_is_router_used($script_name, $expected) + { + $symfony_request = $this->getMockBuilder('\phpbb\symfony_request') + ->setConstructorArgs([new phpbb_mock_request()]) + ->setMethods(['getScriptName']) + ->getMock(); + $symfony_request->method('getScriptName') + ->willReturn($script_name); + + $path_helper = new \phpbb\path_helper( + $symfony_request, + new \phpbb\filesystem\filesystem(), + $this->createMock('\phpbb\request\request'), + $this->phpbb_root_path, + 'php', + 'adm/' + ); + + $this->assertSame($expected, $path_helper->is_router_used()); + } }