Merge pull request #4 from nickvergessen/ticket/marc/11997

Ticket/marc/11997

phpBB/includes/functions.php

@@ -2645,7 +2645,7 @@ function generate_board_url($without_script_path = false)
 */
 function redirect($url, $return = false, $disable_cd_check = false)
 {
-	global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem;
+	global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper;
 
 	$failover_flag = false;
 
@@ -2662,6 +2662,16 @@ function redirect($url, $return = false, $disable_cd_check = false)
 	// Make sure no &'s are in, this will break the redirect
 	$url = str_replace('&', '&', $url);
 
+	// The url currently uses the web root path.
+	// However as we prepend the full board url later,
+	// we need to remove the relative web root path and
+	// prepend the normal root path again. Otherwise redirects
+	// from inside routes will not work as intended.
+	if ($phpbb_path_helper instanceof \phpbb\path_helper)
+	{
+		$url = $phpbb_path_helper->remove_web_root_path($url);
+	}
+
 	// Determine which type of redirect we need to handle...
 	$url_parts = @parse_url($url);
 

phpBB/phpbb/path_helper.php

@@ -101,6 +101,27 @@ class path_helper
 		return $path;
 	}
 
+	/**
+	* Strips away the web root path and prepends the normal root path
+	*
+	* This replaces get_web_root_path() . some_url with
+	*	$phpbb_root_path . some_url
+	*
+	* @param string $path The path to be updated
+	* @return string
+	*/
+	public function remove_web_root_path($path)
+	{
+		if (strpos($path, $this->get_web_root_path()) === 0)
+		{
+			$path = substr($path, strlen($this->get_web_root_path()));
+
+			return $this->phpbb_root_path . $path;
+		}
+
+		return $path;
+	}
+
 	/**
 	* Get a relative root path from the current URL
 	*

tests/functional/extension_controller_test.php

@@ -109,4 +109,36 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c
 		$this->assert_response_html(404);
 		$this->assertContains('No route found for "GET /does/not/exist"', $crawler->filter('body')->text());
 	}
+
+	/**
+	* Check the output of a controller using the template system
+	*/
+	public function test_redirect()
+	{
+		$this->phpbb_extension_manager->enable('foo/bar');
+		$crawler = self::request('GET', 'app.php/foo/redirect');
+
+		$test_redirects = array(
+			'index.php',
+			'../index.php',
+			'tests/index.php',
+			'../tests/index.php',
+			'app.php/index',
+			'index',
+			'../index',
+			'app.php/tests/index',
+			'tests/index',
+			'../tests/index',
+			'index',
+		);
+
+		$filesystem = new \phpbb\filesystem();
+
+		foreach ($test_redirects as $row_num => $redirect)
+		{
+			$this->assertContains($filesystem->clean_path(self::$root_url . $redirect), $crawler->filter('#redirect_' . $row_num)->text());
+		}
+
+		$this->phpbb_extension_manager->purge('foo/bar');
+	}
 }

tests/functional/fixtures/ext/foo/bar/config/routing.yml

@@ -13,3 +13,7 @@ foo_template_controller:
 foo_exception_controller:
     pattern: /foo/exception
     defaults: { _controller: foo_bar.controller:exception }
+
+foo_redirect_controller:
+    pattern: /foo/redirect
+    defaults: { _controller: foo_bar.controller:redirect }

tests/functional/fixtures/ext/foo/bar/config/services.yml

@@ -4,3 +4,5 @@ services:
         arguments:
             - @controller.helper
             - @template
+            - %core.root_path%
+            - %core.php_ext%

tests/functional/fixtures/ext/foo/bar/controller/controller.php

@@ -8,10 +8,12 @@ class controller
 {
 	protected $template;
 
-	public function __construct(\phpbb\controller\helper $helper, \phpbb\template\template $template)
+	public function __construct(\phpbb\controller\helper $helper, \phpbb\template\template $template, $root_path, $php_ext)
 	{
 		$this->template = $template;
 		$this->helper = $helper;
+		$this->root_path = $root_path;
+		$this->php_ext = $php_ext;
 	}
 
 	public function handle()
@@ -35,4 +37,30 @@ class controller
 	{
 		throw new \phpbb\controller\exception('Exception thrown from foo/exception route');
 	}
+
+	public function redirect()
+	{
+		$redirects = array(
+			append_sid($this->root_path . 'index.' . $this->php_ext),
+			append_sid($this->root_path . '../index.' . $this->php_ext),
+			append_sid($this->root_path . 'tests/index.' . $this->php_ext),
+			append_sid($this->root_path . '../tests/index.' . $this->php_ext),
+			$this->helper->url('index'),
+			$this->helper->url('../index'),
+			$this->helper->url('../../index'),
+			$this->helper->url('tests/index'),
+			$this->helper->url('../tests/index'),
+			$this->helper->url('../../tests/index'),
+			$this->helper->url('../tests/../index'),
+		);
+
+		foreach ($redirects as $redirect)
+		{
+			$this->template->assign_block_vars('redirects', array(
+				'URL'		=> redirect($redirect, true),
+			));
+		}
+
+		return $this->helper->render('redirect_body.html');
+	}
 }

tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html

@@ -0,0 +1,5 @@
+<!-- INCLUDE overall_header.html -->
+<!-- BEGIN redirects -->
+<div id="redirect_{redirects.S_ROW_COUNT}">{redirects.URL}</div>
+<!-- END redirects -->
+<!-- INCLUDE overall_footer.html -->