Merge pull request #2969 from marc1706/ticket/13073

[ticket/13073] Correctly generate routes from subfolders like /adm

* marc1706/ticket/13073:
  [ticket/13073] Remove _test suffix from common test class
  [ticket/13073] Switch $input with $expected and add paths with letters
  [ticket/13073] Use abstract class for controller helper route tests
  [ticket/13073] Add path regex to get_preg_expression() and add unit tests
  [ticket/13073] Use just one regex in helper route()
  [ticket/13073] Properly place comments in helper
  [ticket/13073] Use correct class names in test files
  [ticket/13073] Rework route tests and add tests for more directory types
  [ticket/13073] Test that routes from subfolders like /adm work
  [ticket/13073] Add tests for routes from adm pages
  [ticket/13073] Add phpbb root path with mod rewrite enabled for proper routes

phpBB/includes/functions.php

@@ -3317,6 +3317,11 @@ function get_preg_expression($mode)
 		case 'table_prefix':
 			return '#^[a-zA-Z][a-zA-Z0-9_]*$#';
 		break;
+
+		// Matches the predecing dot
+		case 'path_remove_dot_trailing_slash':
+			return '#^(?:(\.)?)+(?:(.+)?)+(?:([\\/\\\])$)#';
+		break;
 	}
 
 	return '';

phpBB/phpbb/controller/helper.php

@@ -140,8 +140,15 @@ class helper
 		// If enable_mod_rewrite is false we need to replace the current front-end by app.php, otherwise we need to remove it.
 		$base_url = str_replace('/' . $page_name, empty($this->config['enable_mod_rewrite']) ? '/app.' . $this->php_ext : '', $base_url);
 
-		// We need to update the base url to move to the directory of the app.php file.
+		// We need to update the base url to move to the directory of the app.php file
+		if (empty($this->config['enable_mod_rewrite']))
+		{
 			$base_url = str_replace('/app.' . $this->php_ext, '/' . $this->phpbb_root_path . 'app.' . $this->php_ext, $base_url);
+		}
+		else
+		{
+			$base_url .= preg_replace(get_preg_expression('path_remove_dot_trailing_slash'), '$2', $this->phpbb_root_path);
+		}
 
 		$base_url = $this->filesystem->clean_path($base_url);
 

tests/controller/common_helper_route.php

@@ -15,35 +15,14 @@ require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
 
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
-class phpbb_controller_helper_route_test extends phpbb_test_case
+abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 {
+	protected $root_path;
+
 	public function setUp()
 	{
 		global $phpbb_dispatcher, $phpbb_root_path, $phpEx;
 
-		$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
-		$this->user = new \phpbb\user('\phpbb\datetime');
-
-		$request = new phpbb_mock_request();
-		$request->overwrite('SCRIPT_NAME', '/app.php', \phpbb\request\request_interface::SERVER);
-		$request->overwrite('SCRIPT_FILENAME', 'app.php', \phpbb\request\request_interface::SERVER);
-		$request->overwrite('REQUEST_URI', '/app.php', \phpbb\request\request_interface::SERVER);
-		$request->overwrite('SERVER_NAME', 'localhost', \phpbb\request\request_interface::SERVER);
-		$request->overwrite('SERVER_PORT', '80', \phpbb\request\request_interface::SERVER);
-
-		$this->symfony_request = new \phpbb\symfony_request(
-			$request
-		);
-		$this->filesystem = new \phpbb\filesystem();
-		$phpbb_path_helper = new \phpbb\path_helper(
-			$this->symfony_request,
-			$this->filesystem,
-			$this->getMock('\phpbb\request\request'),
-			$phpbb_root_path,
-			$phpEx
-		);
-		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
-		$this->template = new phpbb\template\twig\twig($phpbb_path_helper, $this->config, $this->user, new \phpbb\template\context());
 		$this->extension_manager = new phpbb_mock_extension_manager(
 			dirname(__FILE__) . '/',
 			array(
@@ -54,6 +33,49 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 				),
 			)
 		);
+		$this->generate_route_objects();
+		$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
+		$this->user = new \phpbb\user('\phpbb\datetime');
+
+		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
+		$this->template = new phpbb\template\twig\twig($this->phpbb_path_helper, $this->config, $this->user, new \phpbb\template\context());
+	}
+
+	protected function get_phpbb_root_path()
+	{
+		return '';
+	}
+
+	protected function get_uri()
+	{
+		return '/app.php';
+	}
+
+	protected function get_script_name()
+	{
+		return 'app.php';
+	}
+
+	protected function generate_route_objects()
+	{
+		$request = new phpbb_mock_request();
+		$request->overwrite('SCRIPT_NAME', $this->get_uri(), \phpbb\request\request_interface::SERVER);
+		$request->overwrite('SCRIPT_FILENAME', $this->get_script_name(), \phpbb\request\request_interface::SERVER);
+		$request->overwrite('REQUEST_URI', $this->get_uri(), \phpbb\request\request_interface::SERVER);
+		$request->overwrite('SERVER_NAME', 'localhost', \phpbb\request\request_interface::SERVER);
+		$request->overwrite('SERVER_PORT', '80', \phpbb\request\request_interface::SERVER);
+
+		$this->symfony_request = new \phpbb\symfony_request(
+			$request
+		);
+		$this->filesystem = new \phpbb\filesystem();
+		$this->phpbb_path_helper = new \phpbb\path_helper(
+			$this->symfony_request,
+			$this->filesystem,
+			$this->getMock('\phpbb\request\request'),
+			$phpbb_root_path,
+			$phpEx
+		);
 
 		$finder = new \phpbb\finder(
 			new \phpbb\filesystem(),
@@ -64,6 +86,8 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 		$this->provider = new \phpbb\controller\provider();
 		$this->provider->find_routing_files($finder);
 		$this->provider->find(dirname(__FILE__) . '/');
+		// Set correct current phpBB root path
+		$this->root_path = $this->get_phpbb_root_path();
 	}
 
 	public function helper_url_data_no_rewrite()
@@ -101,7 +125,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 	*/
 	public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
 	{
-		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, '', 'php', dirname(__FILE__) . '/');
+		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
 		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id));
 	}
 
@@ -141,7 +165,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 	public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
 	{
 		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
-		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, '', 'php', dirname(__FILE__) . '/');
+		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
 		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id));
 	}
 
@@ -181,7 +205,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 	public function test_helper_url_absolute($route, $params, $is_amp, $session_id, $expected, $description)
 	{
 		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
-		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, '', 'php', dirname(__FILE__) . '/');
+		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
 		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::ABSOLUTE_URL));
 	}
 
@@ -221,7 +245,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 	public function test_helper_url_relative_path($route, $params, $is_amp, $session_id, $expected, $description)
 	{
 		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
-		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, '', 'php', dirname(__FILE__) . '/');
+		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
 		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::RELATIVE_PATH));
 	}
 
@@ -261,7 +285,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 	public function test_helper_url_network($route, $params, $is_amp, $session_id, $expected, $description)
 	{
 		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
-		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, '', 'php', dirname(__FILE__) . '/');
+		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
 		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::NETWORK_PATH));
 	}
 //TODO
@@ -301,7 +325,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 	public function test_helper_url_absolute_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
 	{
 		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
-		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, '', 'php', dirname(__FILE__) . '/');
+		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
 		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::ABSOLUTE_URL));
 	}
 
@@ -341,7 +365,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 	public function test_helper_url_relative_path_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
 	{
 		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
-		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, '', 'php', dirname(__FILE__) . '/');
+		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
 		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::RELATIVE_PATH));
 	}
 
@@ -381,7 +405,7 @@ class phpbb_controller_helper_route_test extends phpbb_test_case
 	public function test_helper_url_network_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
 	{
 		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
-		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, '', 'php', dirname(__FILE__) . '/');
+		$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
 		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::NETWORK_PATH));
 	}
 }

tests/controller/helper_route_adm_subdir_test.php

@@ -0,0 +1,33 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
+require_once dirname(__FILE__) . '/common_helper_route.php';
+
+class phpbb_controller_helper_route_adm_subdir_test extends phpbb_controller_common_helper_route
+{
+	protected function get_phpbb_root_path()
+	{
+		return './../../';
+	}
+
+	protected function get_uri()
+	{
+		return '/adm/subdir/index.php';
+	}
+
+	protected function get_script_name()
+	{
+		return 'index.php';
+	}
+}

tests/controller/helper_route_adm_test.php

@@ -0,0 +1,33 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
+require_once dirname(__FILE__) . '/common_helper_route.php';
+
+class phpbb_controller_helper_route_adm_test extends phpbb_controller_common_helper_route
+{
+	protected function get_phpbb_root_path()
+	{
+		return './../';
+	}
+
+	protected function get_uri()
+	{
+		return '/adm/index.php';
+	}
+
+	protected function get_script_name()
+	{
+		return 'index.php';
+	}
+}

tests/controller/helper_route_root_test.php

@@ -0,0 +1,33 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
+require_once dirname(__FILE__) . '/common_helper_route.php';
+
+class phpbb_controller_helper_route_test extends phpbb_controller_common_helper_route
+{
+	protected function get_phpbb_root_path()
+	{
+		return '';
+	}
+
+	protected function get_uri()
+	{
+		return '/app.php';
+	}
+
+	protected function get_script_name()
+	{
+		return 'app.php';
+	}
+}

tests/controller/helper_route_unclean_path_test.php

@@ -0,0 +1,33 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
+require_once dirname(__FILE__) . '/common_helper_route.php';
+
+class phpbb_controller_helper_route_unclean_path_test extends phpbb_controller_common_helper_route
+{
+	protected function get_phpbb_root_path()
+	{
+		return './../';
+	}
+
+	protected function get_uri()
+	{
+		return '/adm/../bertie/index.php';
+	}
+
+	protected function get_script_name()
+	{
+		return 'index.php';
+	}
+}

tests/functions/get_preg_expression_test.php

@@ -0,0 +1,40 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
+
+class phpbb_functions_get_preg_expression_test extends phpbb_test_case
+{
+	public function data_path_remove_dot_trailing_slash()
+	{
+		return array(
+			array('./../', '$2', '/..'),
+			array('/../', '$2', '/..'),
+			array('', '$2', ''),
+			array('./', '$2', ''),
+			array('/', '$2', ''),
+			array('./../../', '$2', '/../..'),
+			array('/../../', '$2', '/../..'),
+			array('./dir/', '$2', '/dir'),
+			array('./../dir/', '$2', '/../dir'),
+		);
+	}
+
+	/**
+	 * @dataProvider data_path_remove_dot_trailing_slash
+	 */
+	public function test_path_remove_dot_trailing_slash($input, $replace, $expected)
+	{
+		$this->assertSame($expected, preg_replace(get_preg_expression('path_remove_dot_trailing_slash'), $replace, $input));
+	}
+}