Merge pull request #2676 from Nicofuma/ticket/12787

[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

phpBB/config/services.yml

@@ -96,6 +96,7 @@ services:
             - @config
             - @controller.provider
             - @ext.manager
+            - @symfony_request
             - %core.root_path%
             - %core.php_ext%
 
@@ -304,6 +305,7 @@ services:
         arguments:
             - @symfony_request
             - @filesystem
+            - @request
             - %core.root_path%
             - %core.php_ext%
             - %core.adm_relative_path%

phpBB/phpbb/controller/helper.php

@@ -40,6 +40,9 @@ class helper
 	*/
 	protected $config;
 
+	/* @var \phpbb\symfony_request */
+	protected $symfony_request;
+
 	/**
 	* phpBB root path
 	* @var string
@@ -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());
@@ -151,4 +156,14 @@ class helper
 
 		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();
+	}
 }

phpBB/phpbb/path_helper.php

@@ -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;
 
@@ -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;
@@ -98,7 +104,13 @@ class path_helper
 		{
 			$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;
@@ -170,6 +182,34 @@ class path_helper
 			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, '/');
 
@@ -190,6 +230,65 @@ class path_helper
 		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
 	*

tests/avatar/manager_test.php

@@ -38,6 +38,7 @@ class phpbb_avatar_manager_test extends \phpbb_test_case
 				new phpbb_mock_request()
 			),
 			new \phpbb\filesystem(),
+			$this->getMock('\phpbb\request\request'),
 			$phpbb_root_path,
 			$phpEx
 		);

tests/controller/helper_route_test.php

@@ -26,6 +26,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 				new phpbb_mock_request()
 			),
 			new \phpbb\filesystem(),
+			$this->getMock('\phpbb\request\request'),
 			$phpbb_root_path,
 			$phpEx
 		);

tests/extension/metadata_manager_test.php

@@ -53,6 +53,7 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
 					new phpbb_mock_request()
 				),
 				new \phpbb\filesystem(),
+				$this->getMock('\phpbb\request\request'),
 				$this->phpbb_root_path,
 				$this->phpEx
 			),

tests/functions/build_url_test.php

@@ -30,10 +30,11 @@ class phpbb_build_url_test extends phpbb_test_case
 				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()
 	{

tests/mock/controller_helper.php

@@ -23,4 +23,9 @@ class phpbb_mock_controller_helper extends \phpbb\controller\helper
 		$provider->find_routing_files($manager->get_finder());
 		$this->route_collection = $provider->find($phpbb_root_path_ext)->get_routes();
 	}
+
+	public function get_current_url()
+	{
+		return '';
+	}
 }

tests/path_helper/path_helper_test.php

@@ -29,6 +29,7 @@ class phpbb_path_helper_test extends phpbb_test_case
 				new phpbb_mock_request()
 			),
 			new \phpbb\filesystem(),
+			$this->getMock('\phpbb\request\request'),
 			$this->phpbb_root_path,
 			'php'
 		);
@@ -158,6 +159,7 @@ class phpbb_path_helper_test extends phpbb_test_case
 		$path_helper = new \phpbb\path_helper(
 			$symfony_request,
 			new \phpbb\filesystem(),
+			$this->getMock('\phpbb\request\request'),
 			$this->phpbb_root_path,
 			'php'
 		);
@@ -338,4 +340,58 @@ class phpbb_path_helper_test extends phpbb_test_case
 	{
 		$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));
+	}
 }

tests/security/redirect_test.php

@@ -63,6 +63,7 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 					new phpbb_mock_request()
 				),
 				new \phpbb\filesystem(),
+				$this->getMock('\phpbb\request\request'),
 				$this->phpbb_root_path,
 				'php'
 			);

tests/template/template_events_test.php

@@ -143,6 +143,7 @@ Zeta test event in all',
 				new phpbb_mock_request()
 			),
 			new \phpbb\filesystem(),
+			$this->getMock('\phpbb\request\request'),
 			$phpbb_root_path,
 			$phpEx
 		);

tests/template/template_test_case.php

@@ -72,6 +72,7 @@ class phpbb_template_template_test_case extends phpbb_test_case
 				new phpbb_mock_request()
 			),
 			new \phpbb\filesystem(),
+			$this->getMock('\phpbb\request\request'),
 			$phpbb_root_path,
 			$phpEx
 		);

tests/template/template_test_case_with_tree.php

@@ -27,6 +27,7 @@ class phpbb_template_template_test_case_with_tree extends phpbb_template_templat
 				new phpbb_mock_request()
 			),
 			new \phpbb\filesystem(),
+			$this->getMock('\phpbb\request\request'),
 			$phpbb_root_path,
 			$phpEx
 		);

tests/test_framework/phpbb_session_test_case.php

@@ -32,6 +32,7 @@ abstract class phpbb_session_test_case extends phpbb_database_test_case
 		$phpbb_path_helper = new \phpbb\path_helper(
 			$symfony_request,
 			$phpbb_filesystem,
+			$this->getMock('\phpbb\request\request'),
 			$phpbb_root_path,
 			$phpEx
 		);