From 60dda5577dfcbb3c5aac0fdd2a052e12293061a5 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Fri, 8 Nov 2013 23:12:42 +0100
Subject: [PATCH 01/23] [ticket/11997] Correctly redirect to front controllers

We currently do a lot of checks in order to prevent users from getting to
a 404 page. However, this logic relies on checking if a file or folder exists.
Due to the front controllers and the URL rewriting in 3.1, it is no longer
possible to rely on existing files for redirecting. This patch will take
care of properly redirecting users to front controller files. An incorrect
link will cause users to get a 404 error though.

PHPBB3-11997
---
 phpBB/includes/functions.php     | 30 ++++++------------------------
 tests/security/redirect_test.php | 16 +++++++++++++++-
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index 35fa785616..744e610f32 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -999,7 +999,9 @@ function phpbb_own_realpath($path)
 	// @internal The slash in is_dir() gets around an open_basedir restriction
 	if (!@file_exists($resolved) || (!@is_dir($resolved . '/') && !is_file($resolved)))
 	{
-		return false;
+		// In order to allow proper URL rewriting we need to allow
+		// paths that are non-existent
+		//return false;
 	}
 
 	// Put the slashes back to the native operating systems slashes
@@ -2696,20 +2698,6 @@ function redirect($url, $return = false, $disable_cd_check = false)
 		// Relative uri
 		$pathinfo = pathinfo($url);
 
-		if (!$disable_cd_check && !file_exists($pathinfo['dirname'] . '/'))
-		{
-			$url = str_replace('../', '', $url);
-			$pathinfo = pathinfo($url);
-
-			if (!file_exists($pathinfo['dirname'] . '/'))
-			{
-				// fallback to "last known user page"
-				// at least this way we know the user does not leave the phpBB root
-				$url = generate_board_url() . '/' . $user->page['page'];
-				$failover_flag = true;
-			}
-		}
-
 		if (!$failover_flag)
 		{
 			// Is the uri pointing to the current directory?
@@ -2723,14 +2711,7 @@ function redirect($url, $return = false, $disable_cd_check = false)
 					$url = substr($url, 1);
 				}
 
-				if ($user->page['page_dir'])
-				{
-					$url = generate_board_url() . '/' . $user->page['page_dir'] . '/' . $url;
-				}
-				else
-				{
-					$url = generate_board_url() . '/' . $url;
-				}
+				$url = generate_board_url() . '/' . $url;
 			}
 			else
 			{
@@ -2741,8 +2722,9 @@ function redirect($url, $return = false, $disable_cd_check = false)
 
 				$root_dirs = array_diff_assoc($root_dirs, $intersection);
 				$page_dirs = array_diff_assoc($page_dirs, $intersection);
+				$root_dirs_size = sizeof($root_dirs) - 2;
 
-				$dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
+				$dir = (($root_dirs_size > 0) ? str_repeat('../', $root_dirs_size) : '') . implode('/', $page_dirs);
 
 				// Strip / from the end
 				if ($dir && substr($dir, -1, 1) == '/')
diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php
index 8e36780ca4..e934a4ab1b 100644
--- a/tests/security/redirect_test.php
+++ b/tests/security/redirect_test.php
@@ -21,8 +21,22 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 			array('bad://localhost/phpBB/index.php', 'INSECURE_REDIRECT', false),
 			array('http://www.otherdomain.com/somescript.php', false, 'http://localhost/phpBB'),
 			array("http://localhost/phpBB/memberlist.php\n\rConnection: close", 'INSECURE_REDIRECT', false),
-			array('javascript:test', false, 'http://localhost/phpBB/../javascript:test'),
+			array('javascript:test', false, 'http://localhost/phpBB/javascript:test'),
 			array('http://localhost/phpBB/index.php;url=', 'INSECURE_REDIRECT', false),
+			array('http://localhost/phpBB/app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'),
+			array('./app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'),
+			array('app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'),
+			array('./../app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'),
+			array('./../app.php/foo/bar', false, 'http://localhost/phpBB/app.php/foo/bar'),
+			array('./../foo/bar', false, 'http://localhost/phpBB/foo/bar'),
+			array('app.php/', false, 'http://localhost/phpBB/app.php/'),
+			array('./app.php/', false, 'http://localhost/phpBB/app.php/'),
+			array('foobar', false, 'http://localhost/phpBB/foobar'),
+			array('./foobar', false, 'http://localhost/phpBB/foobar'),
+			array('foo/bar', false, 'http://localhost/phpBB/foo/bar'),
+			array('./foo/bar', false, 'http://localhost/phpBB/foo/bar'),
+			array('./../index.php', false, 'http://localhost/phpBB/index.php'),
+			array('../index.php', false, 'http://localhost/phpBB/index.php'),
 		);
 	}
 

From 6bb9bce40e0831e449d1843901a485eef66206d6 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sun, 10 Nov 2013 12:56:09 +0100
Subject: [PATCH 02/23] [ticket/11997] Remove obsolete if statement in
 phpbb_own_realpath()

PHPBB3-11997
---
 phpBB/includes/functions.php | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index 744e610f32..229e856d05 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -994,16 +994,6 @@ function phpbb_own_realpath($path)
 		$resolved .= $bit . (($i == $max) ? '' : '/');
 	}
 
-	// @todo If the file exists fine and open_basedir only has one path we should be able to prepend it
-	// because we must be inside that basedir, the question is where...
-	// @internal The slash in is_dir() gets around an open_basedir restriction
-	if (!@file_exists($resolved) || (!@is_dir($resolved . '/') && !is_file($resolved)))
-	{
-		// In order to allow proper URL rewriting we need to allow
-		// paths that are non-existent
-		//return false;
-	}
-
 	// Put the slashes back to the native operating systems slashes
 	$resolved = str_replace('/', DIRECTORY_SEPARATOR, $resolved);
 

From 2d0fb4d166d0f6371b6c9c6a9e1dce2b34992c9e Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Mon, 11 Nov 2013 21:55:21 +0100
Subject: [PATCH 03/23] [ticket/11997] Rename root_dirs_size to
 superordinate_dirs_count

This variable name better fits what is done with it. A comment explaining
why 2 has to be subtracted from the size of root_dirs has also been added

PHPBB3-11997
---
 phpBB/includes/functions.php | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index 229e856d05..da90dfea10 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2712,9 +2712,17 @@ function redirect($url, $return = false, $disable_cd_check = false)
 
 				$root_dirs = array_diff_assoc($root_dirs, $intersection);
 				$page_dirs = array_diff_assoc($page_dirs, $intersection);
-				$root_dirs_size = sizeof($root_dirs) - 2;
 
-				$dir = (($root_dirs_size > 0) ? str_repeat('../', $root_dirs_size) : '') . implode('/', $page_dirs);
+				// Due to the dirname returned by pathinfo, the
+				// realpath for the $page_dirs array will be 2
+				// superordinate folders up from the phpBB root
+				// path even if the supplied path is in the
+				// phpBB root path. We need to subtract these 2
+				// folders in order to be able to correctly
+				// prepend '../' to the supplied path.
+				$superordinate_dirs_count = sizeof($root_dirs) - 2;
+
+				$dir = (($superordinate_dirs_count > 0) ? str_repeat('../', $superordinate_dirs_count) : '') . implode('/', $page_dirs);
 
 				// Strip / from the end
 				if ($dir && substr($dir, -1, 1) == '/')

From d43542a434c6a214c7533f76f3b1dc7afe84e5cf Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Tue, 12 Nov 2013 00:46:43 +0100
Subject: [PATCH 04/23] [ticket/11997] Use $phpbb_filesystem->clean_path() for
 proper redirect paths

PHPBB3-11997
---
 phpBB/includes/functions.php     | 15 +++------
 tests/security/redirect_test.php | 58 +++++++++++++++++++-------------
 2 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index da90dfea10..0a10a9604c 100644
--- a/phpBB/includes/functions.php
+++ b/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;
+	global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem;
 
 	$failover_flag = false;
 
@@ -2713,16 +2713,7 @@ function redirect($url, $return = false, $disable_cd_check = false)
 				$root_dirs = array_diff_assoc($root_dirs, $intersection);
 				$page_dirs = array_diff_assoc($page_dirs, $intersection);
 
-				// Due to the dirname returned by pathinfo, the
-				// realpath for the $page_dirs array will be 2
-				// superordinate folders up from the phpBB root
-				// path even if the supplied path is in the
-				// phpBB root path. We need to subtract these 2
-				// folders in order to be able to correctly
-				// prepend '../' to the supplied path.
-				$superordinate_dirs_count = sizeof($root_dirs) - 2;
-
-				$dir = (($superordinate_dirs_count > 0) ? str_repeat('../', $superordinate_dirs_count) : '') . implode('/', $page_dirs);
+				$dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
 
 				// Strip / from the end
 				if ($dir && substr($dir, -1, 1) == '/')
@@ -2765,6 +2756,8 @@ function redirect($url, $return = false, $disable_cd_check = false)
 		trigger_error('INSECURE_REDIRECT', E_USER_ERROR);
 	}
 
+	$url = $phpbb_filesystem->clean_path($url);
+
 	if ($return)
 	{
 		return $url;
diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php
index e934a4ab1b..6ea94d33be 100644
--- a/tests/security/redirect_test.php
+++ b/tests/security/redirect_test.php
@@ -17,26 +17,32 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 	{
 		// array(Input -> redirect(), expected triggered error (else false), expected returned result url (else false))
 		return array(
-			array('data://x', false, 'http://localhost/phpBB'),
-			array('bad://localhost/phpBB/index.php', 'INSECURE_REDIRECT', false),
-			array('http://www.otherdomain.com/somescript.php', false, 'http://localhost/phpBB'),
-			array("http://localhost/phpBB/memberlist.php\n\rConnection: close", 'INSECURE_REDIRECT', false),
-			array('javascript:test', false, 'http://localhost/phpBB/javascript:test'),
-			array('http://localhost/phpBB/index.php;url=', 'INSECURE_REDIRECT', false),
-			array('http://localhost/phpBB/app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'),
-			array('./app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'),
-			array('app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'),
-			array('./../app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'),
-			array('./../app.php/foo/bar', false, 'http://localhost/phpBB/app.php/foo/bar'),
-			array('./../foo/bar', false, 'http://localhost/phpBB/foo/bar'),
-			array('app.php/', false, 'http://localhost/phpBB/app.php/'),
-			array('./app.php/', false, 'http://localhost/phpBB/app.php/'),
-			array('foobar', false, 'http://localhost/phpBB/foobar'),
-			array('./foobar', false, 'http://localhost/phpBB/foobar'),
-			array('foo/bar', false, 'http://localhost/phpBB/foo/bar'),
-			array('./foo/bar', false, 'http://localhost/phpBB/foo/bar'),
-			array('./../index.php', false, 'http://localhost/phpBB/index.php'),
-			array('../index.php', false, 'http://localhost/phpBB/index.php'),
+			array('data://x', false, false, 'http://localhost/phpBB'),
+			array('bad://localhost/phpBB/index.php', false, 'INSECURE_REDIRECT', false),
+			array('http://www.otherdomain.com/somescript.php', false, false, 'http://localhost/phpBB'),
+			array("http://localhost/phpBB/memberlist.php\n\rConnection: close", false, 'INSECURE_REDIRECT', false),
+			array('javascript:test', false, false, 'http://localhost/phpBB/javascript:test'),
+			array('http://localhost/phpBB/index.php;url=', false, 'INSECURE_REDIRECT', false),
+			array('http://localhost/phpBB/app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
+			array('./app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
+			array('app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
+			array('./../app.php/foobar', false, false, 'http://localhost/app.php/foobar'),
+			array('./../app.php/foobar', true, false, 'http://localhost/app.php/foobar'),
+			array('./../app.php/foo/bar', false, false, 'http://localhost/app.php/foo/bar'),
+			array('./../app.php/foo/bar', true, false, 'http://localhost/app.php/foo/bar'),
+			array('./../foo/bar', false, false, 'http://localhost/foo/bar'),
+			array('./../foo/bar', true, false, 'http://localhost/foo/bar'),
+			array('app.php/', false, false, 'http://localhost/phpBB/app.php/'),
+			array('./app.php/', false, false, 'http://localhost/phpBB/app.php/'),
+			array('foobar', false, false, 'http://localhost/phpBB/foobar'),
+			array('./foobar', false, false, 'http://localhost/phpBB/foobar'),
+			array('foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
+			array('./foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
+			array('./../index.php', false, false, 'http://localhost/index.php'),
+			array('./../index.php', true, false, 'http://localhost/index.php'),
+			array('../index.php', false, false, 'http://localhost/index.php'),
+			array('../index.php', true, false, 'http://localhost/index.php'),
+			array('./index.php', false, false, 'http://localhost/phpBB/index.php'),
 		);
 	}
 
@@ -52,21 +58,27 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 	/**
 	* @dataProvider provider
 	*/
-	public function test_redirect($test, $expected_error, $expected_result)
+	public function test_redirect($test, $disable_cd_check, $expected_error, $expected_result)
 	{
-		global $user;
+		global $user, $phpbb_root_path;
+
+		$temp_phpbb_root_path = $phpbb_root_path;
+		// We need to hack phpbb_root_path here, so it matches the actual fileinfo of the testing script.
+		// Otherwise the paths are returned incorrectly.
+		$phpbb_root_path = '';
 
 		if ($expected_error !== false)
 		{
 			$this->setExpectedTriggerError(E_USER_ERROR, $expected_error);
 		}
 
-		$result = redirect($test, true);
+		$result = redirect($test, true, $disable_cd_check);
 
 		// only verify result if we did not expect an error
 		if ($expected_error === false)
 		{
 			$this->assertEquals($expected_result, $result);
 		}
+		$phpbb_root_path = $temp_phpbb_root_path;
 	}
 }

From 0a7db81426e966f9c44fcc85338615e1bb3c6f34 Mon Sep 17 00:00:00 2001
From: Joas Schilling <nickvergessen@gmx.de>
Date: Tue, 12 Nov 2013 22:25:23 +0100
Subject: [PATCH 05/23] [ticket/11997] Fix redirects from inside controllers

The redirect 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.

PHPBB3-11997
---
 phpBB/includes/functions.php | 10 ++++++++++
 phpBB/phpbb/path_helper.php  | 21 +++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index 0a10a9604c..c937b8f562 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2662,6 +2662,16 @@ function redirect($url, $return = false, $disable_cd_check = false)
 	// Make sure no &amp;'s are in, this will break the redirect
 	$url = str_replace('&amp;', '&', $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);
 
diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php
index 8cd8808261..71252ac05b 100644
--- a/phpBB/phpbb/path_helper.php
+++ b/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
 	*

From 0aed2816761601ae902159665fdf58932209f772 Mon Sep 17 00:00:00 2001
From: Joas Schilling <nickvergessen@gmx.de>
Date: Wed, 13 Nov 2013 11:21:47 +0100
Subject: [PATCH 06/23] [ticket/11997] Fix missing global

PHPBB3-11997
---
 phpBB/includes/functions.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index c937b8f562..a6c03098c2 100644
--- a/phpBB/includes/functions.php
+++ b/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;
 

From a0fca0acc24684615d123d71ce696e43ba4e2615 Mon Sep 17 00:00:00 2001
From: Joas Schilling <nickvergessen@gmx.de>
Date: Wed, 13 Nov 2013 12:03:06 +0100
Subject: [PATCH 07/23] [ticket/11997] Add functional test for redirects in
 controller

PHPBB3-11997
---
 .../functional/extension_controller_test.php  | 32 +++++++++++++++++++
 .../fixtures/ext/foo/bar/config/routing.yml   |  4 +++
 .../fixtures/ext/foo/bar/config/services.yml  |  2 ++
 .../ext/foo/bar/controller/controller.php     | 30 ++++++++++++++++-
 .../prosilver/template/redirect_body.html     |  5 +++
 5 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html

diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php
index 41bd48c204..b280f01638 100644
--- a/tests/functional/extension_controller_test.php
+++ b/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');
+	}
 }
diff --git a/tests/functional/fixtures/ext/foo/bar/config/routing.yml b/tests/functional/fixtures/ext/foo/bar/config/routing.yml
index 09a30a8c67..9b1ce3cfd7 100644
--- a/tests/functional/fixtures/ext/foo/bar/config/routing.yml
+++ b/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 }
diff --git a/tests/functional/fixtures/ext/foo/bar/config/services.yml b/tests/functional/fixtures/ext/foo/bar/config/services.yml
index 3bca4c6567..d7012dee1c 100644
--- a/tests/functional/fixtures/ext/foo/bar/config/services.yml
+++ b/tests/functional/fixtures/ext/foo/bar/config/services.yml
@@ -4,3 +4,5 @@ services:
         arguments:
             - @controller.helper
             - @template
+            - %core.root_path%
+            - %core.php_ext%
diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
index 259d548299..ebb259af2f 100644
--- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php
+++ b/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');
+	}
 }
diff --git a/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html
new file mode 100644
index 0000000000..0c261f10ea
--- /dev/null
+++ b/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 -->

From 68db335468c0395d8500d1e4e54ed28aca875a30 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sun, 17 Nov 2013 20:16:02 +0100
Subject: [PATCH 08/23] [ticket/11997] Remove obsolete failover_flag in
 function redirect()

This flag is no longer needed as the failover is no longer needed.

PHPBB3-11997
---
 phpBB/includes/functions.php | 89 +++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 47 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index a6c03098c2..0663d0cf85 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2647,8 +2647,6 @@ function redirect($url, $return = false, $disable_cd_check = false)
 {
 	global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper;
 
-	$failover_flag = false;
-
 	if (empty($user->lang))
 	{
 		$user->add_lang('common');
@@ -2698,56 +2696,53 @@ function redirect($url, $return = false, $disable_cd_check = false)
 		// Relative uri
 		$pathinfo = pathinfo($url);
 
-		if (!$failover_flag)
+		// Is the uri pointing to the current directory?
+		if ($pathinfo['dirname'] == '.')
 		{
-			// Is the uri pointing to the current directory?
-			if ($pathinfo['dirname'] == '.')
+			$url = str_replace('./', '', $url);
+
+			// Strip / from the beginning
+			if ($url && substr($url, 0, 1) == '/')
 			{
-				$url = str_replace('./', '', $url);
-
-				// Strip / from the beginning
-				if ($url && substr($url, 0, 1) == '/')
-				{
-					$url = substr($url, 1);
-				}
-
-				$url = generate_board_url() . '/' . $url;
+				$url = substr($url, 1);
 			}
-			else
+
+			$url = generate_board_url() . '/' . $url;
+		}
+		else
+		{
+			// Used ./ before, but $phpbb_root_path is working better with urls within another root path
+			$root_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($phpbb_root_path)));
+			$page_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($pathinfo['dirname'])));
+			$intersection = array_intersect_assoc($root_dirs, $page_dirs);
+
+			$root_dirs = array_diff_assoc($root_dirs, $intersection);
+			$page_dirs = array_diff_assoc($page_dirs, $intersection);
+
+			$dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
+
+			// Strip / from the end
+			if ($dir && substr($dir, -1, 1) == '/')
 			{
-				// Used ./ before, but $phpbb_root_path is working better with urls within another root path
-				$root_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($phpbb_root_path)));
-				$page_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($pathinfo['dirname'])));
-				$intersection = array_intersect_assoc($root_dirs, $page_dirs);
-
-				$root_dirs = array_diff_assoc($root_dirs, $intersection);
-				$page_dirs = array_diff_assoc($page_dirs, $intersection);
-
-				$dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
-
-				// Strip / from the end
-				if ($dir && substr($dir, -1, 1) == '/')
-				{
-					$dir = substr($dir, 0, -1);
-				}
-
-				// Strip / from the beginning
-				if ($dir && substr($dir, 0, 1) == '/')
-				{
-					$dir = substr($dir, 1);
-				}
-
-				$url = str_replace($pathinfo['dirname'] . '/', '', $url);
-
-				// Strip / from the beginning
-				if (substr($url, 0, 1) == '/')
-				{
-					$url = substr($url, 1);
-				}
-
-				$url = (!empty($dir) ? $dir . '/' : '') . $url;
-				$url = generate_board_url() . '/' . $url;
+				$dir = substr($dir, 0, -1);
 			}
+
+			// Strip / from the beginning
+			if ($dir && substr($dir, 0, 1) == '/')
+			{
+				$dir = substr($dir, 1);
+			}
+
+			$url = str_replace($pathinfo['dirname'] . '/', '', $url);
+
+			// Strip / from the beginning
+			if (substr($url, 0, 1) == '/')
+			{
+				$url = substr($url, 1);
+			}
+
+			$url = (!empty($dir) ? $dir . '/' : '') . $url;
+			$url = generate_board_url() . '/' . $url;
 		}
 	}
 

From 8370857f0408b9610ba80e9bc06cde19c8e58983 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 7 Dec 2013 13:20:40 +0100
Subject: [PATCH 09/23] [ticket/11997] Undo changes to phpbb_own_realpath()

PHPBB3-11997
---
 phpBB/includes/functions.php | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index 0663d0cf85..588a060630 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -994,6 +994,14 @@ function phpbb_own_realpath($path)
 		$resolved .= $bit . (($i == $max) ? '' : '/');
 	}
 
+	// @todo If the file exists fine and open_basedir only has one path we should be able to prepend it
+	// because we must be inside that basedir, the question is where...
+	// @internal The slash in is_dir() gets around an open_basedir restriction
+	if (!@file_exists($resolved) || (!@is_dir($resolved . '/') && !is_file($resolved)))
+	{
+		return false;
+	}
+
 	// Put the slashes back to the native operating systems slashes
 	$resolved = str_replace('/', DIRECTORY_SEPARATOR, $resolved);
 

From 8bbede425193caa57be81638b8377c2c9a21e022 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 7 Dec 2013 13:23:57 +0100
Subject: [PATCH 10/23] [ticket/11997] Add method for controller redirect URLs
 to path helper

This method will allow us to get proper redirect URLs for controllers.

PHPBB3-11997
---
 phpBB/phpbb/path_helper.php | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php
index 71252ac05b..f6587fa101 100644
--- a/phpBB/phpbb/path_helper.php
+++ b/phpBB/phpbb/path_helper.php
@@ -183,4 +183,28 @@ class path_helper
 		*/
 		return $this->web_root_path = $this->phpbb_root_path . str_repeat('../', $corrections - 1);
 	}
+
+	/**
+	* Get the redirect URL for controllers
+	*
+	* @param string $url URL to the controller
+	*
+	* @param string Redirect URL for controller
+	*/
+	public function get_controller_redirect_url($url)
+	{
+		// Remove predecing dots
+		$url = ltrim($this->remove_web_root_path($url), '.');
+
+		// Get position of URL delimiter
+		$delimiter_position = strpos($url, '/');
+
+		// Add URL delimiter in front of path if it doesn't exist
+		if ($delimiter_position === false || $delimiter_position > 1)
+		{
+			$url = '/' . $url;
+		}
+
+		return generate_board_url() . $url;
+	}
 }

From a7f2788c72dd45b65de494ca72d13aaee3b140d6 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 7 Dec 2013 13:25:04 +0100
Subject: [PATCH 11/23] [ticket/11997] Use get_controller_redirect_url() in
 redirect() function

This method of path_helper will now be used instead of the previous hack
of the phpbb_own_realpath() function.

PHPBB3-11997
---
 phpBB/includes/functions.php | 122 +++++++++++++++++++++--------------
 1 file changed, 73 insertions(+), 49 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index 588a060630..9569a6de82 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2655,6 +2655,8 @@ function redirect($url, $return = false, $disable_cd_check = false)
 {
 	global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper;
 
+	$failover_flag = false;
+
 	if (empty($user->lang))
 	{
 		$user->add_lang('common');
@@ -2668,16 +2670,6 @@ function redirect($url, $return = false, $disable_cd_check = false)
 	// Make sure no &amp;'s are in, this will break the redirect
 	$url = str_replace('&amp;', '&', $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);
 
@@ -2704,53 +2696,87 @@ function redirect($url, $return = false, $disable_cd_check = false)
 		// Relative uri
 		$pathinfo = pathinfo($url);
 
-		// Is the uri pointing to the current directory?
-		if ($pathinfo['dirname'] == '.')
+		// Also treat URLs that have a non-existing basename
+		if (!$disable_cd_check && (!file_exists($pathinfo['dirname'] . '/') || !file_exists($pathinfo['basename'])))
 		{
-			$url = str_replace('./', '', $url);
+			$url = str_replace('../', '', $url);
+			$pathinfo = pathinfo($url);
 
-			// Strip / from the beginning
-			if ($url && substr($url, 0, 1) == '/')
+			// Also treat URLs that have a non-existing basename
+			if (!file_exists($pathinfo['dirname'] . '/') || !file_exists($pathinfo['basename']))
 			{
-				$url = substr($url, 1);
+				// fallback to "last known user page"
+				// at least this way we know the user does not leave the phpBB root
+				if ($phpbb_path_helper instanceof \phpbb\path_helper)
+				{
+					$url = $phpbb_path_helper->get_controller_redirect_url($url);
+				}
+				else
+				{
+					$url = generate_board_url() . '/' . $user->page['page'];
+				}
+				$failover_flag = true;
 			}
-
-			$url = generate_board_url() . '/' . $url;
 		}
-		else
+
+		if (!$failover_flag)
 		{
-			// Used ./ before, but $phpbb_root_path is working better with urls within another root path
-			$root_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($phpbb_root_path)));
-			$page_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($pathinfo['dirname'])));
-			$intersection = array_intersect_assoc($root_dirs, $page_dirs);
-
-			$root_dirs = array_diff_assoc($root_dirs, $intersection);
-			$page_dirs = array_diff_assoc($page_dirs, $intersection);
-
-			$dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
-
-			// Strip / from the end
-			if ($dir && substr($dir, -1, 1) == '/')
+			// Is the uri pointing to the current directory?
+			if ($pathinfo['dirname'] == '.')
 			{
-				$dir = substr($dir, 0, -1);
-			}
+				$url = str_replace('./', '', $url);
 
-			// Strip / from the beginning
-			if ($dir && substr($dir, 0, 1) == '/')
+				// Strip / from the beginning
+				if ($url && substr($url, 0, 1) == '/')
+				{
+					$url = substr($url, 1);
+				}
+
+				if ($user->page['page_dir'])
+				{
+					$url = generate_board_url() . '/' . $user->page['page_dir'] . '/' . $url;
+				}
+				else
+				{
+					$url = generate_board_url() . '/' . $url;
+				}
+			}
+			else
 			{
-				$dir = substr($dir, 1);
+				// Used ./ before, but $phpbb_root_path is working better with urls within another root path
+				$root_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($phpbb_root_path)));
+				$page_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($pathinfo['dirname'])));
+				$intersection = array_intersect_assoc($root_dirs, $page_dirs);
+
+				$root_dirs = array_diff_assoc($root_dirs, $intersection);
+				$page_dirs = array_diff_assoc($page_dirs, $intersection);
+
+				$dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
+
+				// Strip / from the end
+				if ($dir && substr($dir, -1, 1) == '/')
+				{
+					$dir = substr($dir, 0, -1);
+				}
+
+				// Strip / from the beginning
+				if ($dir && substr($dir, 0, 1) == '/')
+				{
+					$dir = substr($dir, 1);
+				}
+
+				$url = str_replace($pathinfo['dirname'] . '/', '', $url);
+
+				// Strip / from the beginning
+				if (substr($url, 0, 1) == '/')
+				{
+					$url = substr($url, 1);
+				}
+
+				$url = (!empty($dir) ? $dir . '/' : '') . $url;
+				$url = generate_board_url() . '/' . $url;
 			}
-
-			$url = str_replace($pathinfo['dirname'] . '/', '', $url);
-
-			// Strip / from the beginning
-			if (substr($url, 0, 1) == '/')
-			{
-				$url = substr($url, 1);
-			}
-
-			$url = (!empty($dir) ? $dir . '/' : '') . $url;
-			$url = generate_board_url() . '/' . $url;
+			$url = $phpbb_filesystem->clean_path($url);
 		}
 	}
 
@@ -2769,8 +2795,6 @@ function redirect($url, $return = false, $disable_cd_check = false)
 		trigger_error('INSECURE_REDIRECT', E_USER_ERROR);
 	}
 
-	$url = $phpbb_filesystem->clean_path($url);
-
 	if ($return)
 	{
 		return $url;

From f32a30eecacba212850a11b7b4740d0a69bd49de Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 7 Dec 2013 13:28:44 +0100
Subject: [PATCH 12/23] [ticket/11997] Fix tests for path_helper's
 get_controller_redirect_url()

PHPBB3-11997
---
 .../functional/extension_controller_test.php  | 19 ++++++------
 tests/security/redirect_test.php              | 29 +++++++++++++------
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php
index b280f01638..fe21ec5bd8 100644
--- a/tests/functional/extension_controller_test.php
+++ b/tests/functional/extension_controller_test.php
@@ -115,28 +115,27 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c
 	*/
 	public function test_redirect()
 	{
+		$filesystem = new \phpbb\filesystem();
 		$this->phpbb_extension_manager->enable('foo/bar');
 		$crawler = self::request('GET', 'app.php/foo/redirect');
 
 		$test_redirects = array(
 			'index.php',
-			'../index.php',
+			'index.php',
+			'tests/index.php',
 			'tests/index.php',
-			'../tests/index.php',
 			'app.php/index',
-			'index',
-			'../index',
+			'app.php/index',
+			'app.php/index',
+			'app.php/tests/index',
+			'app.php/tests/index',
+			'app.php/tests/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->assertContains($filesystem->clean_path(self::$root_url) . $redirect, $crawler->filter('#redirect_' . $row_num)->text());
 		}
 
 		$this->phpbb_extension_manager->purge('foo/bar');
diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php
index 6ea94d33be..48360e3034 100644
--- a/tests/security/redirect_test.php
+++ b/tests/security/redirect_test.php
@@ -26,21 +26,21 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 			array('http://localhost/phpBB/app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
 			array('./app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
 			array('app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
-			array('./../app.php/foobar', false, false, 'http://localhost/app.php/foobar'),
-			array('./../app.php/foobar', true, false, 'http://localhost/app.php/foobar'),
-			array('./../app.php/foo/bar', false, false, 'http://localhost/app.php/foo/bar'),
-			array('./../app.php/foo/bar', true, false, 'http://localhost/app.php/foo/bar'),
-			array('./../foo/bar', false, false, 'http://localhost/foo/bar'),
-			array('./../foo/bar', true, false, 'http://localhost/foo/bar'),
+			array('./../app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
+			array('./../app.php/foobar', true, 'INSECURE_REDIRECT', false),
+			array('./../app.php/foo/bar', false, false, 'http://localhost/phpBB/app.php/foo/bar'),
+			array('./../app.php/foo/bar', true, 'INSECURE_REDIRECT', false),
+			array('./../foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
+			array('./../foo/bar', true, 'INSECURE_REDIRECT', false),
 			array('app.php/', false, false, 'http://localhost/phpBB/app.php/'),
 			array('./app.php/', false, false, 'http://localhost/phpBB/app.php/'),
 			array('foobar', false, false, 'http://localhost/phpBB/foobar'),
 			array('./foobar', false, false, 'http://localhost/phpBB/foobar'),
 			array('foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
 			array('./foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
-			array('./../index.php', false, false, 'http://localhost/index.php'),
+			array('./../index.php', false, false, 'http://localhost/phpBB/index.php'),
 			array('./../index.php', true, false, 'http://localhost/index.php'),
-			array('../index.php', false, false, 'http://localhost/index.php'),
+			array('../index.php', false, false, 'http://localhost/phpBB/index.php'),
 			array('../index.php', true, false, 'http://localhost/index.php'),
 			array('./index.php', false, false, 'http://localhost/phpBB/index.php'),
 		);
@@ -53,6 +53,15 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 		$GLOBALS['config'] = array(
 			'force_server_vars'	=> '0',
 		);
+
+		$this->path_helper = new \phpbb\path_helper(
+			new \phpbb\symfony_request(
+				new phpbb_mock_request()
+			),
+			new \phpbb\filesystem(),
+			$this->phpbb_root_path,
+			'php'
+		);
 	}
 
 	/**
@@ -60,7 +69,9 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 	*/
 	public function test_redirect($test, $disable_cd_check, $expected_error, $expected_result)
 	{
-		global $user, $phpbb_root_path;
+		global $user, $phpbb_root_path, $phpbb_path_helper;
+
+		$phpbb_path_helper = $this->path_helper;
 
 		$temp_phpbb_root_path = $phpbb_root_path;
 		// We need to hack phpbb_root_path here, so it matches the actual fileinfo of the testing script.

From 15913fdf79b8e41049e3263e5e27e6690effc65e Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Fri, 20 Dec 2013 18:13:53 +0100
Subject: [PATCH 13/23] [ticket/11997] Move expected redirect returns to
 controller and output to HTML

The controller will now output the expected redirect returns the same way the
redirect returns are output. The extension controller test will compare those
2 outputs.

PHPBB3-11997
---
 .../functional/extension_controller_test.php  | 25 +++-----
 .../ext/foo/bar/controller/controller.php     | 61 +++++++++++++++----
 .../prosilver/template/redirect_body.html     |  3 +
 3 files changed, 62 insertions(+), 27 deletions(-)

diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php
index ab90223c48..5127aa7f47 100644
--- a/tests/functional/extension_controller_test.php
+++ b/tests/functional/extension_controller_test.php
@@ -121,23 +121,18 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c
 		$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',
-			'app.php/index',
-			'app.php/index',
-			'app.php/tests/index',
-			'app.php/tests/index',
-			'app.php/tests/index',
-			'app.php/tests/index',
-		);
+		$nodes = $crawler->filter('div')->extract(array('id'));
 
-		foreach ($test_redirects as $row_num => $redirect)
+		foreach ($nodes as $redirect)
 		{
-			$this->assertContains($filesystem->clean_path(self::$root_url) . $redirect, $crawler->filter('#redirect_' . $row_num)->text());
+			if (strpos($redirect, 'redirect_expected') !== 0)
+			{
+				continue;
+			}
+
+			$row_num = str_replace('redirect_expected_', '', $redirect);
+
+			$this->assertContains($filesystem->clean_path(self::$root_url) . $crawler->filter('#redirect_expected_' .  $row_num)->text(), $crawler->filter('#redirect_' . $row_num)->text());
 		}
 
 		$this->phpbb_extension_manager->purge('foo/bar');
diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
index ebb259af2f..18ec756d3c 100644
--- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php
+++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
@@ -41,23 +41,60 @@ class controller
 	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'),
+			array(
+				append_sid($this->root_path . 'index.' . $this->php_ext),
+				'index.php',
+			),
+			array(
+				append_sid($this->root_path . '../index.' . $this->php_ext),
+				'index.php',
+			),
+			array(
+				append_sid($this->root_path . 'tests/index.' . $this->php_ext),
+				'tests/index.php',
+			),
+			array(
+				append_sid($this->root_path . '../tests/index.' . $this->php_ext),
+				'tests/index.php',
+			),
+			array(
+				$this->helper->url('index'),
+				'app.php/index',
+			),
+			array(
+				$this->helper->url('../index'),
+				'app.php/index',
+			),
+			array(
+				$this->helper->url('../../index'),
+				'app.php/index',
+			),
+			array(
+				$this->helper->url('tests/index'),
+				'app.php/tests/index',
+			),
+			array(
+				$this->helper->url('../tests/index'),
+				'app.php/tests/index',
+			),
+			array(
+				$this->helper->url('../../tests/index'),
+				'app.php/tests/index',
+			),
+			array(
+				$this->helper->url('../tests/../index'),
+				'app.php/tests/index',
+			),
 		);
 
 		foreach ($redirects as $redirect)
 		{
 			$this->template->assign_block_vars('redirects', array(
-				'URL'		=> redirect($redirect, true),
+				'URL'		=> redirect($redirect[0], true),
+			));
+
+			$this->template->assign_block_vars('redirects_expected', array(
+				'URL'		=> $redirect[1],
 			));
 		}
 
diff --git a/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html
index 0c261f10ea..2b70b0fe59 100644
--- a/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html
+++ b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/redirect_body.html
@@ -2,4 +2,7 @@
 <!-- BEGIN redirects -->
 <div id="redirect_{redirects.S_ROW_COUNT}">{redirects.URL}</div>
 <!-- END redirects -->
+<!-- BEGIN redirects_expected -->
+<div id="redirect_expected_{redirects_expected.S_ROW_COUNT}">{redirects_expected.URL}</div>
+<!-- END redirects_expected -->
 <!-- INCLUDE overall_footer.html -->

From 235d2069e0e7cecfd51d4eed5c875cc865f35486 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 21 Dec 2013 16:31:20 +0100
Subject: [PATCH 14/23] [ticket/11997] Allow redirects to parent folders like
 previously

Redirects to parent folders were possible with the previous redirect function.
This change will allow these redirects again.

PHPBB3-11997
---
 phpBB/includes/functions.php                   | 9 +++++----
 tests/functional/extension_controller_test.php | 4 +++-
 tests/security/redirect_test.php               | 4 ++--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index d874b7b19e..4c9c3323f7 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2653,7 +2653,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, $phpbb_path_helper;
+	global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper, $phpEx;
 
 	$failover_flag = false;
 
@@ -2696,14 +2696,15 @@ function redirect($url, $return = false, $disable_cd_check = false)
 		// Relative uri
 		$pathinfo = pathinfo($url);
 
-		// Also treat URLs that have a non-existing basename
-		if (!$disable_cd_check && (!file_exists($pathinfo['dirname'] . '/') || !file_exists($pathinfo['basename'])))
+		// Also treat URLs that have a non-existing basename and fit
+		// controller style URLs
+		if (!$disable_cd_check && (!file_exists($pathinfo['dirname'] . '/') || (!file_exists($url) && preg_match('/^[\.]?+[\/]?+(?:app\.php)?+[a-zA-Z0-9\/]/', $url))))
 		{
 			$url = str_replace('../', '', $url);
 			$pathinfo = pathinfo($url);
 
 			// Also treat URLs that have a non-existing basename
-			if (!file_exists($pathinfo['dirname'] . '/') || !file_exists($pathinfo['basename']))
+			if (!file_exists($pathinfo['dirname'] . '/') || (!file_exists($url) && preg_match('/^[\.]?+[\/]?+(?:app\.php)?+[a-zA-Z0-9\/]/', $url)))
 			{
 				// fallback to "last known user page"
 				// at least this way we know the user does not leave the phpBB root
diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php
index 5127aa7f47..2476cf0c19 100644
--- a/tests/functional/extension_controller_test.php
+++ b/tests/functional/extension_controller_test.php
@@ -132,7 +132,9 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c
 
 			$row_num = str_replace('redirect_expected_', '', $redirect);
 
-			$this->assertContains($filesystem->clean_path(self::$root_url) . $crawler->filter('#redirect_expected_' .  $row_num)->text(), $crawler->filter('#redirect_' . $row_num)->text());
+			$redirect = $crawler->filter('#redirect_' . $row_num)->text();
+			$redirect = substr($redirect, 0, strpos($redirect, 'sid') - 1);
+			$this->assertContains($crawler->filter('#redirect_expected_' .  $row_num)->text(), $redirect);
 		}
 
 		$this->phpbb_extension_manager->purge('foo/bar');
diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php
index 48360e3034..24ddaa265d 100644
--- a/tests/security/redirect_test.php
+++ b/tests/security/redirect_test.php
@@ -38,9 +38,9 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 			array('./foobar', false, false, 'http://localhost/phpBB/foobar'),
 			array('foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
 			array('./foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
-			array('./../index.php', false, false, 'http://localhost/phpBB/index.php'),
+			array('./../index.php', false, false, 'http://localhost/index.php'),
 			array('./../index.php', true, false, 'http://localhost/index.php'),
-			array('../index.php', false, false, 'http://localhost/phpBB/index.php'),
+			array('../index.php', false, false, 'http://localhost/index.php'),
 			array('../index.php', true, false, 'http://localhost/index.php'),
 			array('./index.php', false, false, 'http://localhost/phpBB/index.php'),
 		);

From d9358c26da6737044a3c10893e7b954176b205d2 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 21 Dec 2013 20:08:00 +0100
Subject: [PATCH 15/23] [ticket/11997] Add clean_url() method to path_helper

This method will get rid of unnecessary . and .. in URLs.

PHPBB3-11997
---
 phpBB/includes/functions.php             |  2 +-
 phpBB/phpbb/path_helper.php              | 23 +++++++++++++++++++++++
 tests/path_helper/web_root_path_test.php | 23 +++++++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index 4c9c3323f7..aea13f7679 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2777,7 +2777,7 @@ function redirect($url, $return = false, $disable_cd_check = false)
 				$url = (!empty($dir) ? $dir . '/' : '') . $url;
 				$url = generate_board_url() . '/' . $url;
 			}
-			$url = $phpbb_filesystem->clean_path($url);
+			$url = $phpbb_path_helper->clean_url($url);;
 		}
 	}
 
diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php
index f6587fa101..cd4c20bb7d 100644
--- a/phpBB/phpbb/path_helper.php
+++ b/phpBB/phpbb/path_helper.php
@@ -207,4 +207,27 @@ class path_helper
 
 		return generate_board_url() . $url;
 	}
+
+	/**
+	* Eliminates useless . and .. components from specified URL
+	*
+	* @param string $url URL to clean
+	*
+	* @return string Cleaned URL
+	*/
+	public function clean_url($url)
+	{
+		$delimiter_position = strpos($url, '://');
+		// URL should contain :// but it shouldn't start with it.
+		// Do not clean URLs that do not fit these constraints.
+		if (empty($delimiter_position))
+		{
+			return $url;
+		}
+		$scheme = substr($url, 0, $delimiter_position) . '://';
+		// Add length of URL delimiter to position
+		$path = substr($url, $delimiter_position + 3);
+
+		return $scheme . $this->filesystem->clean_path($path);
+	}
 }
diff --git a/tests/path_helper/web_root_path_test.php b/tests/path_helper/web_root_path_test.php
index 2e1a37e02b..2c22511402 100644
--- a/tests/path_helper/web_root_path_test.php
+++ b/tests/path_helper/web_root_path_test.php
@@ -146,4 +146,27 @@ class phpbb_path_helper_web_root_path_test extends phpbb_test_case
 
 		$this->assertEquals($expected, $path_helper->update_web_root_path($input, $symfony_request));
 	}
+
+	public function clean_url_data()
+	{
+		return array(
+			array('', ''),
+			array('://', '://'),
+			array('http://', 'http://'),
+			array('http://one/two/three', 'http://one/two/three'),
+			array('http://../one/two', 'http://../one/two'),
+			array('http://one/../two/three', 'http://two/three'),
+			array('http://one/two/../three', 'http://one/three'),
+			array('http://one/two/../../three', 'http://three'),
+			array('http://one/two/../../../three', 'http://../three'),
+		);
+	}
+
+	/**
+	* @dataProvider clean_url_data
+	*/
+	public function test_clean_url($input, $expected)
+	{
+		$this->assertEquals($expected, $this->path_helper->clean_url($input));
+	}
 }

From 4ced1626467c1b8a137f18ceb3aca3a032b41c76 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 21 Dec 2013 20:08:33 +0100
Subject: [PATCH 16/23] [ticket/11997] Modifiy tests after adding path_helper
 clean_url method

PHPBB3-11997
---
 tests/security/redirect_test.php | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php
index 24ddaa265d..13db4c299b 100644
--- a/tests/security/redirect_test.php
+++ b/tests/security/redirect_test.php
@@ -27,11 +27,11 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 			array('./app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
 			array('app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
 			array('./../app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
-			array('./../app.php/foobar', true, 'INSECURE_REDIRECT', false),
+			array('./../app.php/foobar', true, false, 'http://../../../foobar'),
 			array('./../app.php/foo/bar', false, false, 'http://localhost/phpBB/app.php/foo/bar'),
-			array('./../app.php/foo/bar', true, 'INSECURE_REDIRECT', false),
+			array('./../app.php/foo/bar', true, false, 'http://../../../bar'),
 			array('./../foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
-			array('./../foo/bar', true, 'INSECURE_REDIRECT', false),
+			array('./../foo/bar', true, false, 'http://../../../bar'),
 			array('app.php/', false, false, 'http://localhost/phpBB/app.php/'),
 			array('./app.php/', false, false, 'http://localhost/phpBB/app.php/'),
 			array('foobar', false, false, 'http://localhost/phpBB/foobar'),

From 9161816267ace60d9972e01b7fbe1213fe29a708 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Fri, 27 Dec 2013 13:00:22 +0100
Subject: [PATCH 17/23] [ticket/11997] Do not check if file or dir we redirect
 to exist

The redirect function will now properly redirect to where we want it to.
It will no longer try to check if the file or directory we redirect to exist.
This will ensure compatibility with the new routes.

PHPBB3-11997
---
 phpBB/includes/functions.php     | 88 +++++---------------------------
 tests/security/redirect_test.php | 70 ++++++++++++++++++++-----
 2 files changed, 69 insertions(+), 89 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index aea13f7679..33d0e36a28 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2696,89 +2696,27 @@ function redirect($url, $return = false, $disable_cd_check = false)
 		// Relative uri
 		$pathinfo = pathinfo($url);
 
-		// Also treat URLs that have a non-existing basename and fit
-		// controller style URLs
-		if (!$disable_cd_check && (!file_exists($pathinfo['dirname'] . '/') || (!file_exists($url) && preg_match('/^[\.]?+[\/]?+(?:app\.php)?+[a-zA-Z0-9\/]/', $url))))
+		// Is the uri pointing to the current directory?
+		if ($pathinfo['dirname'] == '.')
 		{
-			$url = str_replace('../', '', $url);
-			$pathinfo = pathinfo($url);
+			$url = str_replace('./', '', $url);
 
-			// Also treat URLs that have a non-existing basename
-			if (!file_exists($pathinfo['dirname'] . '/') || (!file_exists($url) && preg_match('/^[\.]?+[\/]?+(?:app\.php)?+[a-zA-Z0-9\/]/', $url)))
+			// Strip / from the beginning
+			if ($url && substr($url, 0, 1) == '/')
 			{
-				// fallback to "last known user page"
-				// at least this way we know the user does not leave the phpBB root
-				if ($phpbb_path_helper instanceof \phpbb\path_helper)
-				{
-					$url = $phpbb_path_helper->get_controller_redirect_url($url);
-				}
-				else
-				{
-					$url = generate_board_url() . '/' . $user->page['page'];
-				}
-				$failover_flag = true;
+				$url = substr($url, 1);
 			}
 		}
 
-		if (!$failover_flag)
-		{
-			// Is the uri pointing to the current directory?
-			if ($pathinfo['dirname'] == '.')
-			{
-				$url = str_replace('./', '', $url);
+		$url = generate_board_url() . '/' . $url;
+	}
 
-				// Strip / from the beginning
-				if ($url && substr($url, 0, 1) == '/')
-				{
-					$url = substr($url, 1);
-				}
+	// Clean URL and check if we go outside the forum directory
+	$url = $phpbb_path_helper->clean_url($url);
 
-				if ($user->page['page_dir'])
-				{
-					$url = generate_board_url() . '/' . $user->page['page_dir'] . '/' . $url;
-				}
-				else
-				{
-					$url = generate_board_url() . '/' . $url;
-				}
-			}
-			else
-			{
-				// Used ./ before, but $phpbb_root_path is working better with urls within another root path
-				$root_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($phpbb_root_path)));
-				$page_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($pathinfo['dirname'])));
-				$intersection = array_intersect_assoc($root_dirs, $page_dirs);
-
-				$root_dirs = array_diff_assoc($root_dirs, $intersection);
-				$page_dirs = array_diff_assoc($page_dirs, $intersection);
-
-				$dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
-
-				// Strip / from the end
-				if ($dir && substr($dir, -1, 1) == '/')
-				{
-					$dir = substr($dir, 0, -1);
-				}
-
-				// Strip / from the beginning
-				if ($dir && substr($dir, 0, 1) == '/')
-				{
-					$dir = substr($dir, 1);
-				}
-
-				$url = str_replace($pathinfo['dirname'] . '/', '', $url);
-
-				// Strip / from the beginning
-				if (substr($url, 0, 1) == '/')
-				{
-					$url = substr($url, 1);
-				}
-
-				$url = (!empty($dir) ? $dir . '/' : '') . $url;
-				$url = generate_board_url() . '/' . $url;
-			}
-			$url = $phpbb_path_helper->clean_url($url);;
-		}
+	if (!$disable_cd_check && strpos($url, generate_board_url(true)) === false)
+	{
+		trigger_error('INSECURE_REDIRECT', E_USER_ERROR);
 	}
 
 	// Make sure no linebreaks are there... to prevent http response splitting for PHP < 4.4.2
diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php
index 13db4c299b..ce2d865c44 100644
--- a/tests/security/redirect_test.php
+++ b/tests/security/redirect_test.php
@@ -13,8 +13,13 @@ require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
 
 class phpbb_security_redirect_test extends phpbb_security_test_base
 {
+	protected $path_helper;
+
+	protected $controller_helper;
+
 	public function provider()
 	{
+		$this->controller_helper = $this->get_controller_helper();
 		// array(Input -> redirect(), expected triggered error (else false), expected returned result url (else false))
 		return array(
 			array('data://x', false, false, 'http://localhost/phpBB'),
@@ -26,13 +31,15 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 			array('http://localhost/phpBB/app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
 			array('./app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
 			array('app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
-			array('./../app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
-			array('./../app.php/foobar', true, false, 'http://../../../foobar'),
-			array('./../app.php/foo/bar', false, false, 'http://localhost/phpBB/app.php/foo/bar'),
-			array('./../app.php/foo/bar', true, false, 'http://../../../bar'),
-			array('./../foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
-			array('./../foo/bar', true, false, 'http://../../../bar'),
+			array('./../app.php/foobar', false, false, 'http://localhost/app.php/foobar'),
+			array('./../app.php/foobar', true, false, 'http://localhost/app.php/foobar'),
+			array('./../app.php/foo/bar', false, false, 'http://localhost/app.php/foo/bar'),
+			array('./../app.php/foo/bar', true, false, 'http://localhost/app.php/foo/bar'),
+			array('./../foo/bar', false, false, 'http://localhost/foo/bar'),
+			array('./../foo/bar', true, false, 'http://localhost/foo/bar'),
 			array('app.php/', false, false, 'http://localhost/phpBB/app.php/'),
+			array($this->controller_helper->url('a'), false, false, 'http://localhost/phpBB/app.php/a'),
+			array($this->controller_helper->url(''), false, false, 'http://localhost/phpBB/app.php/'),
 			array('./app.php/', false, false, 'http://localhost/phpBB/app.php/'),
 			array('foobar', false, false, 'http://localhost/phpBB/foobar'),
 			array('./foobar', false, false, 'http://localhost/phpBB/foobar'),
@@ -46,6 +53,47 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 		);
 	}
 
+	protected function get_path_helper()
+	{
+		if (!($this->path_helper instanceof \phpbb\path_helper))
+		{
+			$this->path_helper = new \phpbb\path_helper(
+				new \phpbb\symfony_request(
+					new phpbb_mock_request()
+				),
+				new \phpbb\filesystem(),
+				$this->phpbb_root_path,
+				'php'
+			);
+		}
+		return $this->path_helper;
+	}
+
+	protected function get_controller_helper()
+	{
+		if (!($this->controller_helper instanceof \phpbb\controller\helper))
+		{
+			global $phpbb_dispatcher;
+
+			$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
+			$this->user = $this->getMock('\phpbb\user');
+			$phpbb_path_helper = new \phpbb\path_helper(
+				new \phpbb\symfony_request(
+					new phpbb_mock_request()
+				),
+				new \phpbb\filesystem(),
+				$phpbb_root_path,
+				$phpEx
+			);
+			$this->template = new phpbb\template\twig\twig($phpbb_path_helper, $config, $this->user, new \phpbb\template\context());
+
+			// We don't use mod_rewrite in these tests
+			$config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
+			$this->controller_helper = new \phpbb\controller\helper($this->template, $this->user, $config, '', 'php');
+		}
+		return $this->controller_helper;
+	}
+
 	protected function setUp()
 	{
 		parent::setUp();
@@ -54,14 +102,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 			'force_server_vars'	=> '0',
 		);
 
-		$this->path_helper = new \phpbb\path_helper(
-			new \phpbb\symfony_request(
-				new phpbb_mock_request()
-			),
-			new \phpbb\filesystem(),
-			$this->phpbb_root_path,
-			'php'
-		);
+		$this->path_helper = $this->get_path_helper();
+		$this->controller_helper = $this->get_controller_helper();
 	}
 
 	/**

From a304d99b2b5d0b69548b81b013aa711838ff521d Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Fri, 27 Dec 2013 17:50:17 +0100
Subject: [PATCH 18/23] [ticket/11997] Add remove_web_root_path() in order to
 prevent incorrect URLs

Adding a call to this method to the redirect function will make sure that we
do not end up with incorrect URLs after using append_sid().

PHPBB3-11997
---
 phpBB/includes/functions.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index 33d0e36a28..bd6a45685d 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2708,7 +2708,7 @@ function redirect($url, $return = false, $disable_cd_check = false)
 			}
 		}
 
-		$url = generate_board_url() . '/' . $url;
+		$url = generate_board_url() . '/' . $phpbb_path_helper->remove_web_root_path($url);
 	}
 
 	// Clean URL and check if we go outside the forum directory

From 68ce16f9b33c5aea8f7f6530dd06eb4661333b0b Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Fri, 27 Dec 2013 17:51:40 +0100
Subject: [PATCH 19/23] [ticket/11997] Use path_helper in in foo/bar extension
 for redirect URLs

By using path_helper's clean_url() method, we'll be able to properly check
the full URL instead of just parts of the expected URL.

PHPBB3-11997
---
 .../functional/extension_controller_test.php  |  2 +-
 .../fixtures/ext/foo/bar/config/services.yml  |  1 +
 .../ext/foo/bar/controller/controller.php     | 24 ++++++++++++-------
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php
index 2476cf0c19..4725301141 100644
--- a/tests/functional/extension_controller_test.php
+++ b/tests/functional/extension_controller_test.php
@@ -134,7 +134,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c
 
 			$redirect = $crawler->filter('#redirect_' . $row_num)->text();
 			$redirect = substr($redirect, 0, strpos($redirect, 'sid') - 1);
-			$this->assertContains($crawler->filter('#redirect_expected_' .  $row_num)->text(), $redirect);
+			$this->assertEquals($crawler->filter('#redirect_expected_' .  $row_num)->text(), $redirect);
 		}
 
 		$this->phpbb_extension_manager->purge('foo/bar');
diff --git a/tests/functional/fixtures/ext/foo/bar/config/services.yml b/tests/functional/fixtures/ext/foo/bar/config/services.yml
index 13aadf9768..b2730b5c09 100644
--- a/tests/functional/fixtures/ext/foo/bar/config/services.yml
+++ b/tests/functional/fixtures/ext/foo/bar/config/services.yml
@@ -3,6 +3,7 @@ services:
         class: foo\bar\controller\controller
         arguments:
             - @controller.helper
+            - @path_helper
             - @template
             - %core.root_path%
             - %core.php_ext%
diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
index 18ec756d3c..3ba253256a 100644
--- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php
+++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
@@ -8,10 +8,11 @@ class controller
 {
 	protected $template;
 
-	public function __construct(\phpbb\controller\helper $helper, \phpbb\template\template $template, $root_path, $php_ext)
+	public function __construct(\phpbb\controller\helper $helper, \phpbb\path_helper $path_helper, \phpbb\template\template $template, $root_path, $php_ext)
 	{
 		$this->template = $template;
 		$this->helper = $helper;
+		$this->path_helper = $path_helper;
 		$this->root_path = $root_path;
 		$this->php_ext = $php_ext;
 	}
@@ -40,6 +41,7 @@ class controller
 
 	public function redirect()
 	{
+		$url_root = generate_board_url();
 		$redirects = array(
 			array(
 				append_sid($this->root_path . 'index.' . $this->php_ext),
@@ -47,7 +49,7 @@ class controller
 			),
 			array(
 				append_sid($this->root_path . '../index.' . $this->php_ext),
-				'index.php',
+				'../index.php',
 			),
 			array(
 				append_sid($this->root_path . 'tests/index.' . $this->php_ext),
@@ -55,7 +57,7 @@ class controller
 			),
 			array(
 				append_sid($this->root_path . '../tests/index.' . $this->php_ext),
-				'tests/index.php',
+				'../tests/index.php',
 			),
 			array(
 				$this->helper->url('index'),
@@ -63,11 +65,11 @@ class controller
 			),
 			array(
 				$this->helper->url('../index'),
-				'app.php/index',
+				'index',
 			),
 			array(
 				$this->helper->url('../../index'),
-				'app.php/index',
+				'../index',
 			),
 			array(
 				$this->helper->url('tests/index'),
@@ -75,15 +77,19 @@ class controller
 			),
 			array(
 				$this->helper->url('../tests/index'),
-				'app.php/tests/index',
+				'tests/index',
 			),
 			array(
 				$this->helper->url('../../tests/index'),
-				'app.php/tests/index',
+				'../tests/index',
 			),
 			array(
 				$this->helper->url('../tests/../index'),
-				'app.php/tests/index',
+				'index',
+			),
+			array(
+				$this->helper->url('tests/../index'),
+				'app.php/index',
 			),
 		);
 
@@ -94,7 +100,7 @@ class controller
 			));
 
 			$this->template->assign_block_vars('redirects_expected', array(
-				'URL'		=> $redirect[1],
+				'URL'		=> $this->path_helper->clean_url($url_root . '/' . $redirect[1]),
 			));
 		}
 

From ce2c5213d7aad2c24ee83147b167236ce754c671 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Fri, 27 Dec 2013 18:41:44 +0100
Subject: [PATCH 20/23] [ticket/11997] Remove obsolete function
 get_controller_redirect_url()

PHPBB3-11997
---
 phpBB/phpbb/path_helper.php | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php
index cd4c20bb7d..a8e12c4063 100644
--- a/phpBB/phpbb/path_helper.php
+++ b/phpBB/phpbb/path_helper.php
@@ -184,30 +184,6 @@ class path_helper
 		return $this->web_root_path = $this->phpbb_root_path . str_repeat('../', $corrections - 1);
 	}
 
-	/**
-	* Get the redirect URL for controllers
-	*
-	* @param string $url URL to the controller
-	*
-	* @param string Redirect URL for controller
-	*/
-	public function get_controller_redirect_url($url)
-	{
-		// Remove predecing dots
-		$url = ltrim($this->remove_web_root_path($url), '.');
-
-		// Get position of URL delimiter
-		$delimiter_position = strpos($url, '/');
-
-		// Add URL delimiter in front of path if it doesn't exist
-		if ($delimiter_position === false || $delimiter_position > 1)
-		{
-			$url = '/' . $url;
-		}
-
-		return generate_board_url() . $url;
-	}
-
 	/**
 	* Eliminates useless . and .. components from specified URL
 	*

From 4c1569dd8ab6d85aecd0bd30913512542d1f123d Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 28 Dec 2013 00:14:12 +0100
Subject: [PATCH 21/23] [ticket/11997] Add user's page dir to redirect path and
 fix unit tests for it

The user's page directory needs to be added to the redirect URL for proper
redirects outside of the forum root. Fix the unit tests accordingly.

PHPBB3-11997
---
 phpBB/includes/functions.php     | 9 ++++++++-
 tests/security/redirect_test.php | 6 +++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index bd6a45685d..cc8478ace1 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2708,7 +2708,14 @@ function redirect($url, $return = false, $disable_cd_check = false)
 			}
 		}
 
-		$url = generate_board_url() . '/' . $phpbb_path_helper->remove_web_root_path($url);
+		$url = $phpbb_path_helper->remove_web_root_path($url);
+
+		if ($user->page['page_dir'])
+		{
+			$url = $user->page['page_dir'] . '/' . $url;
+		}
+
+		$url = generate_board_url() . '/' . $url;
 	}
 
 	// Clean URL and check if we go outside the forum directory
diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php
index ce2d865c44..77dc955c26 100644
--- a/tests/security/redirect_test.php
+++ b/tests/security/redirect_test.php
@@ -116,9 +116,12 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 		$phpbb_path_helper = $this->path_helper;
 
 		$temp_phpbb_root_path = $phpbb_root_path;
-		// We need to hack phpbb_root_path here, so it matches the actual fileinfo of the testing script.
+		$temp_page_dir = $user->page['page_dir'];
+		// We need to hack phpbb_root_path and the user's page_dir here
+		// so it matches the actual fileinfo of the testing script.
 		// Otherwise the paths are returned incorrectly.
 		$phpbb_root_path = '';
+		$user->page['page_dir'] = '';
 
 		if ($expected_error !== false)
 		{
@@ -133,5 +136,6 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 			$this->assertEquals($expected_result, $result);
 		}
 		$phpbb_root_path = $temp_phpbb_root_path;
+		$user->page['page_dir'] = $temp_page_dir;
 	}
 }

From 3e815616c5e5237cc1201e6e29337c4a601049c5 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 28 Dec 2013 16:50:15 +0100
Subject: [PATCH 22/23] [ticket/11997] Fix redirect tests for mod rewrite

Controller routes that are supposed to link to parent directories can't be
tested as the links are incorrectly created depending on enabled mod rewrite
or not.

PHPBB3-11997
---
 .../fixtures/ext/foo/bar/config/services.yml  |  1 +
 .../ext/foo/bar/controller/controller.php     | 41 ++++++++++++-------
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/tests/functional/fixtures/ext/foo/bar/config/services.yml b/tests/functional/fixtures/ext/foo/bar/config/services.yml
index b2730b5c09..cec69f7807 100644
--- a/tests/functional/fixtures/ext/foo/bar/config/services.yml
+++ b/tests/functional/fixtures/ext/foo/bar/config/services.yml
@@ -5,6 +5,7 @@ services:
             - @controller.helper
             - @path_helper
             - @template
+            - @config
             - %core.root_path%
             - %core.php_ext%
 
diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
index 3ba253256a..fc0f6e21af 100644
--- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php
+++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
@@ -7,12 +7,16 @@ use Symfony\Component\HttpFoundation\Response;
 class controller
 {
 	protected $template;
+	protected $helper;
+	protected $path_helper;
+	protected $config;
 
-	public function __construct(\phpbb\controller\helper $helper, \phpbb\path_helper $path_helper, \phpbb\template\template $template, $root_path, $php_ext)
+	public function __construct(\phpbb\controller\helper $helper, \phpbb\path_helper $path_helper, \phpbb\template\template $template, \phpbb\config\config $config, $root_path, $php_ext)
 	{
 		$this->template = $template;
 		$this->helper = $helper;
 		$this->path_helper = $path_helper;
+		$this->config = $config;
 		$this->root_path = $root_path;
 		$this->php_ext = $php_ext;
 	}
@@ -42,6 +46,9 @@ class controller
 	public function redirect()
 	{
 		$url_root = generate_board_url();
+
+		$rewrite_prefix = (!empty($this->config['enable_mod_rewrite'])) ? '' : 'app.php/';
+
 		$redirects = array(
 			array(
 				append_sid($this->root_path . 'index.' . $this->php_ext),
@@ -61,36 +68,40 @@ class controller
 			),
 			array(
 				$this->helper->url('index'),
-				'app.php/index',
+				$rewrite_prefix . 'index',
 			),
+			array(
+				$this->helper->url('tests/index'),
+				$rewrite_prefix . 'tests/index',
+			),
+			array(
+				$this->helper->url('tests/../index'),
+				$rewrite_prefix . 'index',
+			),
+			/*
+			// helper URLs starting with  ../ are prone to failure.
+			// Do not test them right now.
 			array(
 				$this->helper->url('../index'),
-				'index',
+				'../index',
 			),
 			array(
 				$this->helper->url('../../index'),
 				'../index',
 			),
 			array(
-				$this->helper->url('tests/index'),
-				'app.php/tests/index',
+				$this->helper->url('../tests/index'),
+				$rewrite_prefix . '../tests/index',
 			),
 			array(
-				$this->helper->url('../tests/index'),
-				'tests/index',
+				$this->helper->url('../tests/../index'),
+				'../index',
 			),
 			array(
 				$this->helper->url('../../tests/index'),
 				'../tests/index',
 			),
-			array(
-				$this->helper->url('../tests/../index'),
-				'index',
-			),
-			array(
-				$this->helper->url('tests/../index'),
-				'app.php/index',
-			),
+			*/
 		);
 
 		foreach ($redirects as $redirect)

From f906fb41e9e995c0ea472a8d6594f54df6f208bf Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 28 Dec 2013 20:40:18 +0100
Subject: [PATCH 23/23] [ticket/11997] Use functional test cases that should
 always work

The previous test cases that tried to redirect to ../index.php and similar
might cause us to try to go to http://localhost/../index.php, which will
result in http://index.php. As this obviously will trigger an error as
intended, we should not put this inside our test cases for the redirect
function.

PHPBB3-11997
---
 .../fixtures/ext/foo/bar/controller/controller.php        | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
index fc0f6e21af..558b202948 100644
--- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php
+++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
@@ -55,17 +55,13 @@ class controller
 				'index.php',
 			),
 			array(
-				append_sid($this->root_path . '../index.' . $this->php_ext),
-				'../index.php',
+				append_sid($this->root_path . 'foo/bar/index.' . $this->php_ext),
+				'foo/bar/index.php',
 			),
 			array(
 				append_sid($this->root_path . 'tests/index.' . $this->php_ext),
 				'tests/index.php',
 			),
-			array(
-				append_sid($this->root_path . '../tests/index.' . $this->php_ext),
-				'../tests/index.php',
-			),
 			array(
 				$this->helper->url('index'),
 				$rewrite_prefix . 'index',