Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ticket/17141] Improve stability of path_helper and increase test coverage #6498

Merged
merged 4 commits into from
Jun 28, 2023
Merged

[ticket/17141] Improve stability of path_helper and increase test coverage #6498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8223a95
[ticket/17141] Ensure correction is newer below 0
marc1706 Jun 26, 2023
55ea8d6
[ticket/17141] Symfony request can never be null so don't check for it
marc1706 Jun 26, 2023
b0c0086
[ticket/17141] Remove duplicate prepending with phpbb_root_path
marc1706 Jun 26, 2023
91f216d
[ticket/17141] Fully cover path_helper with unit tests
marc1706 Jun 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions phpBB/phpbb/path_helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,6 @@ public function remove_web_root_path($path)
*/
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;
Expand Down Expand Up @@ -218,7 +213,7 @@ public function get_web_root_path()
$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?
Expand All @@ -236,7 +231,7 @@ public function get_web_root_path()

// 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;
}
Expand Down Expand Up @@ -264,7 +259,7 @@ public function get_web_root_path_from_ajax_referer($absolute_referer_url, $abso
$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
Expand Down
241 changes: 205 additions & 36 deletions tests/path_helper/path_helper_test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ protected function setUp(): void
new \phpbb\filesystem\filesystem(),
$this->createMock('\phpbb\request\request'),
$this->phpbb_root_path,
'php'
'php',
'adm/'
);
}

Expand All @@ -50,34 +51,46 @@ public function set_phpbb_root_path($filesystem)

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()
{
$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'),
),
);
],
];
}

/**
Expand All @@ -88,6 +101,26 @@ public function test_basic_update_web_root_path($input, $expected)
$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());
Expand Down Expand Up @@ -158,6 +191,13 @@ public function update_web_root_path_data()
'/phpbb3-fork/phpBB/app.php',
'',
),
array(
'./../'.$this->phpbb_root_path . 'test.php',
'',
'/phpbb3-fork/phpBB/foo',
'/phpbb3-fork/phpBB/app.php',
'',
),
);
}

Expand Down Expand Up @@ -190,6 +230,47 @@ public function test_update_web_root_path($input, $getPathInfo, $getRequestUri,
$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(
Expand Down Expand Up @@ -383,6 +464,41 @@ public function append_url_params_data()
);
}

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
*/
Expand All @@ -393,63 +509,78 @@ public function test_append_url_params($url, $params, $is_amp, $expected)

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',
''
]
];
}

/**
Expand Down Expand Up @@ -484,4 +615,42 @@ 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));
}

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());
}
}