Skip to content

Commit

Permalink
Merge pull request phpbb#2676 from Nicofuma/ticket/12787
Browse files Browse the repository at this point in the history
[ticket/12787] Allow the route to say that the referer has to be used.

* Nicofuma/ticket/12787:
  [ticket/12787] Updates phpbb_mock_controller_helper
  [ticket/12787] Add controller_helper::get_current_url()
  [ticket/12787] Remove one app.php when it's both in $path and $web_root_path
  [ticket/12787] Fix the absolute board url
  [ticket/12787] Use a parameter (_referer) instead of the Referer header
  [ticket/12099] Add unit tests for get_web_root_path_from_ajax_referer()
  [ticket/12099] Remove config again
  [ticket/12099] Correctly fix the path when performing AJAX requests
  [ticket/12099] Add request argument to path_helper service
  • Loading branch information
nickvergessen committed Jul 8, 2014
2 parents dca8afc + b4d7192 commit 91f5d59
Show file tree
Hide file tree
Showing 14 changed files with 190 additions and 4 deletions.
2 changes: 2 additions & 0 deletions phpBB/config/services.yml
Expand Up @@ -96,6 +96,7 @@ services:
- @config
- @controller.provider
- @ext.manager
- @symfony_request
- %core.root_path%
- %core.php_ext%

Expand Down Expand Up @@ -304,6 +305,7 @@ services:
arguments:
- @symfony_request
- @filesystem
- @request
- %core.root_path%
- %core.php_ext%
- %core.adm_relative_path%
Expand Down
17 changes: 16 additions & 1 deletion phpBB/phpbb/controller/helper.php
Expand Up @@ -40,6 +40,9 @@ class helper
*/
protected $config;

/* @var \phpbb\symfony_request */
protected $symfony_request;

/**
* phpBB root path
* @var string
Expand All @@ -60,14 +63,16 @@ class helper
* @param \phpbb\config\config $config Config object
* @param \phpbb\controller\provider $provider Path provider
* @param \phpbb\extension\manager $manager Extension manager object
* @param \phpbb\symfony_request $symfony_request Symfony Request object
* @param string $phpbb_root_path phpBB root path
* @param string $php_ext PHP extension
*/
public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\controller\provider $provider, \phpbb\extension\manager $manager, $phpbb_root_path, $php_ext)
public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\controller\provider $provider, \phpbb\extension\manager $manager, \phpbb\symfony_request $symfony_request, $phpbb_root_path, $php_ext)
{
$this->template = $template;
$this->user = $user;
$this->config = $config;
$this->symfony_request = $symfony_request;
$this->phpbb_root_path = $phpbb_root_path;
$this->php_ext = $php_ext;
$provider->find_routing_files($manager->get_finder());
Expand Down Expand Up @@ -151,4 +156,14 @@ public function error($message, $code = 500)

return $this->render('message_body.html', $this->user->lang('INFORMATION'), $code);
}

/**
* Return the current url
*
* @return string
*/
public function get_current_url()
{
return generate_board_url(true) . $this->symfony_request->getRequestUri();
}
}
103 changes: 101 additions & 2 deletions phpBB/phpbb/path_helper.php
Expand Up @@ -24,6 +24,9 @@ class path_helper
/** @var \phpbb\filesystem */
protected $filesystem;

/** @var \phpbb\request\request_interface */
protected $request;

/** @var string */
protected $phpbb_root_path;

Expand All @@ -41,13 +44,16 @@ class path_helper
*
* @param \phpbb\symfony_request $symfony_request
* @param \phpbb\filesystem $filesystem
* @param \phpbb\request\request_interface $request
* @param string $phpbb_root_path Relative path to phpBB root
* @param string $php_ext PHP extension (php)
* @param mixed $adm_relative_path Relative path admin path to adm/ root
*/
public function __construct(\phpbb\symfony_request $symfony_request, \phpbb\filesystem $filesystem, $phpbb_root_path, $php_ext, $adm_relative_path = null)
public function __construct(\phpbb\symfony_request $symfony_request, \phpbb\filesystem $filesystem, \phpbb\request\request_interface $request, $phpbb_root_path, $php_ext, $adm_relative_path = null)
{
$this->symfony_request = $symfony_request;
$this->filesystem = $filesystem;
$this->request = $request;
$this->phpbb_root_path = $phpbb_root_path;
$this->php_ext = $php_ext;
$this->adm_relative_path = $adm_relative_path;
Expand Down Expand Up @@ -98,7 +104,13 @@ public function update_web_root_path($path)
{
$path = substr($path, strlen($this->phpbb_root_path));

return $this->filesystem->clean_path($this->get_web_root_path() . $path);
$web_root_path = $this->get_web_root_path();
if (substr($web_root_path, -8) === 'app.php/' && substr($path, 0, 7) === 'app.php')
{
$path = substr($path, 8);
}

return $this->filesystem->clean_path($web_root_path . $path);
}

return $path;
Expand Down Expand Up @@ -170,6 +182,34 @@ public function get_web_root_path()
return $this->web_root_path = $this->phpbb_root_path;
}

/*
* Check AJAX request:
* If the current request is a AJAX we need to fix the paths.
* We need to get the root path based on the Referer, so we can use
* the generated URLs in the template of the Referer. If we do not
* generate the relative path based on the Referer, but based on the
* currently requested URL, the generated URLs will not point to the
* intended locations:
* Referer desired URL desired relative root path
* memberlist.php faq.php ./
* memberlist.php app.php/foo/bar ./
* app.php/foo memberlist.php ../
* app.php/foo app.php/fox ../
* app.php/foo/bar memberlist.php ../../
* ../page.php memberlist.php ./phpBB/
* ../sub/page.php memberlist.php ./../phpBB/
*
* The referer must be specified as a parameter in the query.
*/
if ($this->request->is_ajax() && $this->symfony_request->get('_referer'))
{
$referer_web_root_path = $this->get_web_root_path_from_ajax_referer(
$this->symfony_request->get('_referer'),
$this->symfony_request->getSchemeAndHttpHost() . $this->symfony_request->getBasePath()
);
return $this->web_root_path = $this->phpbb_root_path . $referer_web_root_path;
}

// How many corrections might we need?
$corrections = substr_count($path_info, '/');

Expand All @@ -190,6 +230,65 @@ public function get_web_root_path()
return $this->web_root_path;
}

/**
* Get the web root path of the referer form an ajax request
*
* @param string $absolute_referer_url
* @param string $absolute_board_url
* @return string
*/
public function get_web_root_path_from_ajax_referer($absolute_referer_url, $absolute_board_url)
{
// If the board URL is in the beginning of the referer, this means
// we the referer is in the board URL or a subdirectory of it.
// So we just need to count the / (slashes) in the left over part of
// the referer and prepend ../ the the current root_path, to get the
// web root path of the referer.
if (strpos($absolute_referer_url, $absolute_board_url) === 0)
{
$relative_referer_path = substr($absolute_referer_url, strlen($absolute_board_url));
$has_params = strpos($relative_referer_path, '?');
if ($has_params !== false)
{
$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);
}

// If not, it's a bit more complicated. We go to the parent directory
// of the referer until we find the remaining referer in the board URL.
// Foreach directory we need to add a ../ to the fixed root_path.
// When we finally found it, we need to remove the remaining referer
// from the board URL, to get the boards root path.
// If the then append these two strings, we get our fixed web root path.
$fixed_root_path = '';
$referer_dir = $absolute_referer_url;
$has_params = strpos($referer_dir, '?');
if ($has_params !== false)
{
$referer_dir = substr($referer_dir, 0, $has_params);
}

// If we do not find a slash at the end of the referer, we come
// from a file. So the first dirname() does not need a traversal
// path correction.
if (substr($referer_dir, -1) !== '/')
{
$referer_dir = dirname($referer_dir);
}

while (strpos($absolute_board_url, $referer_dir) !== 0)
{
$fixed_root_path .= '../';
$referer_dir = dirname($referer_dir);
}

$fixed_root_path .= substr($absolute_board_url, strlen($referer_dir) + 1);
// Add trailing slash
return $this->phpbb_root_path . $fixed_root_path . '/';
}

/**
* Eliminates useless . and .. components from specified URL
*
Expand Down
1 change: 1 addition & 0 deletions tests/avatar/manager_test.php
Expand Up @@ -38,6 +38,7 @@ public function setUp()
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$phpbb_root_path,
$phpEx
);
Expand Down
1 change: 1 addition & 0 deletions tests/controller/helper_route_test.php
Expand Up @@ -26,6 +26,7 @@ public function setUp()
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$phpbb_root_path,
$phpEx
);
Expand Down
1 change: 1 addition & 0 deletions tests/extension/metadata_manager_test.php
Expand Up @@ -53,6 +53,7 @@ protected function setUp()
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$this->phpbb_root_path,
$this->phpEx
),
Expand Down
3 changes: 2 additions & 1 deletion tests/functions/build_url_test.php
Expand Up @@ -30,10 +30,11 @@ protected function setUp()
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$phpbb_root_path,
'php'
);
$phpbb_container->set('path_helper', $path_helper);
$phpbb_container->set('path_helper', $phpbb_path_helper);
}
public function build_url_test_data()
{
Expand Down
5 changes: 5 additions & 0 deletions tests/mock/controller_helper.php
Expand Up @@ -23,4 +23,9 @@ public function __construct(\phpbb\template\template $template, \phpbb\user $use
$provider->find_routing_files($manager->get_finder());
$this->route_collection = $provider->find($phpbb_root_path_ext)->get_routes();
}

public function get_current_url()
{
return '';
}
}
56 changes: 56 additions & 0 deletions tests/path_helper/path_helper_test.php
Expand Up @@ -29,6 +29,7 @@ public function setUp()
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$this->phpbb_root_path,
'php'
);
Expand Down Expand Up @@ -158,6 +159,7 @@ public function test_update_web_root_path($input, $getPathInfo, $getRequestUri,
$path_helper = new \phpbb\path_helper(
$symfony_request,
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$this->phpbb_root_path,
'php'
);
Expand Down Expand Up @@ -338,4 +340,58 @@ public function test_append_url_params($url, $params, $is_amp, $expected)
{
$this->assertEquals($expected, $this->path_helper->append_url_params($url, $params, $is_amp));
}

public function get_web_root_path_from_ajax_referer_data()
{
return array(
array(
'http://www.phpbb.com/community/route1/route2/',
'http://www.phpbb.com/community',
'../../',
),
array(
'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',
'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/',
),
);
}

/**
* @dataProvider get_web_root_path_from_ajax_referer_data
*/
public function test_get_web_root_path_from_ajax_referer($referer_url, $board_url, $expected)
{
$this->assertEquals($this->phpbb_root_path . $expected, $this->path_helper->get_web_root_path_from_ajax_referer($referer_url, $board_url));
}
}
1 change: 1 addition & 0 deletions tests/security/redirect_test.php
Expand Up @@ -63,6 +63,7 @@ protected function get_path_helper()
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$this->phpbb_root_path,
'php'
);
Expand Down
1 change: 1 addition & 0 deletions tests/template/template_events_test.php
Expand Up @@ -143,6 +143,7 @@ protected function setup_engine_for_events($dataset, $style_names, array $new_co
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$phpbb_root_path,
$phpEx
);
Expand Down
1 change: 1 addition & 0 deletions tests/template/template_test_case.php
Expand Up @@ -72,6 +72,7 @@ protected function setup_engine(array $new_config = array())
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$phpbb_root_path,
$phpEx
);
Expand Down
1 change: 1 addition & 0 deletions tests/template/template_test_case_with_tree.php
Expand Up @@ -27,6 +27,7 @@ protected function setup_engine(array $new_config = array())
new phpbb_mock_request()
),
new \phpbb\filesystem(),
$this->getMock('\phpbb\request\request'),
$phpbb_root_path,
$phpEx
);
Expand Down
1 change: 1 addition & 0 deletions tests/test_framework/phpbb_session_test_case.php
Expand Up @@ -32,6 +32,7 @@ function setUp()
$phpbb_path_helper = new \phpbb\path_helper(
$symfony_request,
$phpbb_filesystem,
$this->getMock('\phpbb\request\request'),
$phpbb_root_path,
$phpEx
);
Expand Down

0 comments on commit 91f5d59

Please sign in to comment.