Merge branch 'develop-olympus' into develop

* develop-olympus:
  [develop-olympus] Make this test run on windows with backslash-paths.
  [bug/56965] Redirect fails with directory traversal
  [bug/58685] Correct spelling errors in append_sid() comments.
  [bug/58025] Search robots are now redirected if they send a SID in the request

phpBB/docs/CHANGELOG.html

@@ -104,6 +104,7 @@
 		<li>[Fix] Prevent wrong tar archive type detection. (Bug #12531)</li>
 		<li>[Fix] Correct redirection after login to forum not in web root (Bug #58755)</li>
 		<li>[Fix] Allow setting parent forums regardless of permission settings. (Bug #57415)</li>
+		<li>[Fix] Redirect search engines that access pages with SIDs in the URL. (Bug #58025)</li>
 		<li>[Feature] Support for Microsoft's Native SQL Server Driver for PHP (Bug #57055 - Patch by Chris Pucci at Microsoft)</li>
 		<li>[Feature] The memcache acm plugin now supports multiple memcache servers.</li>
 	</ul>

phpBB/includes/functions.php

@@ -2139,8 +2139,8 @@ function append_sid($url, $params = false, $is_amp = true, $session_id = false)
 {
 	global $_SID, $_EXTRA_URL, $phpbb_hook;
 
-	// Developers using the hook function need to globalise the $_SID and $_EXTRA_URL on their own and also handle it appropiatly.
+	// Developers using the hook function need to globalise the $_SID and $_EXTRA_URL on their own and also handle it appropriately.
-	// They could mimick most of what is within this function
+	// They could mimic most of what is within this function
 	if (!empty($phpbb_hook) && $phpbb_hook->call_hook(__FUNCTION__, $url, $params, $is_amp, $session_id))
 	{
 		if ($phpbb_hook->hook_return(__FUNCTION__))
@@ -2297,6 +2297,8 @@ function redirect($url, $return = false, $disable_cd_check = false)
 {
 	global $db, $cache, $config, $user, $phpbb_root_path;
 
+	$failover_flag = false;
+
 	if (empty($user->lang))
 	{
 		$user->add_lang('common');
@@ -2344,66 +2346,70 @@ function redirect($url, $return = false, $disable_cd_check = false)
 			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'];
-				break;
+				$failover_flag = true;
 			}
 		}
 
-		// Is the uri pointing to the current directory?
+		if (!$failover_flag)
-		if ($pathinfo['dirname'] == '.')
 		{
-			$url = str_replace('./', '', $url);
+			// Is the uri pointing to the current directory?
-
+			if ($pathinfo['dirname'] == '.')
-			// Strip / from the beginning
-			if ($url && substr($url, 0, 1) == '/')
 			{
-				$url = substr($url, 1);
+				$url = str_replace('./', '', $url);
-			}
 
-			if ($user->page['page_dir'])
+				// Strip / from the beginning
-			{
+				if ($url && substr($url, 0, 1) == '/')
-				$url = generate_board_url() . '/' . $user->page['page_dir'] . '/' . $url;
+				{
+					$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
 			{
+				// 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;
 			}
 		}
-		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;
-		}
 	}
 
 	// Make sure no linebreaks are there... to prevent http response splitting for PHP < 4.4.2

phpBB/includes/session.php

@@ -608,6 +608,12 @@ class session
 		}
 		else
 		{
+			// Bot user, if they have a SID in the Request URI we need to get rid of it
+			// otherwise they'll index this page with the SID, duplicate content oh my!
+			if (isset($_GET['sid']))
+			{
+				redirect(build_url(array('sid')));
+			}
 			$this->data['session_last_visit'] = $this->time_now;
 		}
 

tests/template/template.php

@@ -256,7 +256,7 @@ class phpbb_template_template_test extends phpbb_test_case
 		$this->template->set_filenames(array('test' => $filename));
 		$this->assertFileNotExists($this->template_path . '/' . $filename, 'Testing missing file, file cannot exist');
 
-		$expecting = sprintf('template->_tpl_load_file(): File %s does not exist or is empty', realpath($this->template_path) . '/' . $filename);
+		$expecting = sprintf('template->_tpl_load_file(): File %s does not exist or is empty', realpath($this->template_path . '/../') . '/templates/' . $filename);
 		$this->setExpectedTriggerError(E_USER_ERROR, $expecting);
 
 		$this->display('test');