Merge pull request #2195 from Elsensee/ticket/10423

[ticket/10423] Remove * from search or highlight string

* Elsensee/ticket/10423:
  [ticket/10423] Replace foreach with function in viewtopic.php
  [ticket/10423] Remove unnecessary include in test
  [ticket/10423] Match multiple wildcards
  [ticket/10423] Move code into a function and add tests for it
  [ticket/10423] Remove * from search or highlight string

phpBB/includes/functions_content.php

@@ -21,6 +21,7 @@
 * make_jumpbox()
 * bump_topic_allowed()
 * get_context()
+* phpbb_clean_search_string()
 * decode_message()
 * strip_bbcode()
 * generate_text_for_display()
@@ -360,6 +361,23 @@ function get_context($text, $words, $length = 400)
 	}
 }
 
+/**
+* Cleans a search string by removing single wildcards from it and replacing multiple spaces with a single one.
+*
+* @param string $search_string The full search string which should be cleaned.
+*
+* @return string The cleaned search string without any wildcards and multiple spaces.
+*/
+function phpbb_clean_search_string($search_string)
+{
+	// This regular expressions matches every single wildcard.
+	// That means one after a whitespace or the beginning of the string or one before a whitespace or the end of the string.
+	$search_string = preg_replace('#(?<=^|\s)\*+(?=\s|$)#', '', $search_string);
+	$search_string = trim($search_string);
+	$search_string = preg_replace(array('#\s+#u', '#\*+#u'), array(' ', '*'), $search_string);
+	return $search_string;
+}
+
 /**
 * Decode text whereby text is coming from the db and expected to be pre-parsed content
 * We are placing this outside of the message parser because we are often in need of it...

phpBB/search.php

@@ -574,9 +574,9 @@
 	}
 
 	// define some vars for urls
-	$hilit = implode('|', explode(' ', preg_replace('#\s+#u', ' ', str_replace(array('+', '-', '|', '(', ')', '"'), ' ', $keywords))));
-	// Do not allow *only* wildcard being used for hilight
-	$hilit = (strspn($hilit, '*') === strlen($hilit)) ? '' : $hilit;
+	// A single wildcard will make the search results look ugly
+	$hilit = phpbb_clean_search_string(str_replace(array('+', '-', '|', '(', ')', '"'), ' ', $keywords));
+	$hilit = str_replace(' ', '|', $hilit);
 
 	$u_hilit = urlencode(htmlspecialchars_decode(str_replace('|', ' ', $hilit)));
 	$u_show_results = '&sr=' . $show_results;
@@ -840,7 +840,8 @@
 			$hilit_array = array_filter(explode('|', $hilit), 'strlen');
 			foreach ($hilit_array as $key => $value)
 			{
-				$hilit_array[$key] = str_replace('\*', '\w*?', preg_quote($value, '#'));
+				$hilit_array[$key] = phpbb_clean_search_string($value);
+				$hilit_array[$key] = str_replace('\*', '\w*?', preg_quote($hilit_array[$key], '#'));
 				$hilit_array[$key] = preg_replace('#(^|\s)\\\\w\*\?(\s|$)#', '$1\w+?$2', $hilit_array[$key]);
 			}
 			$hilit = implode('|', $hilit_array);

phpBB/viewtopic.php

@@ -471,17 +471,11 @@
 $highlight_match = $highlight = '';
 if ($hilit_words)
 {
-	foreach (explode(' ', trim($hilit_words)) as $word)
-	{
-		if (trim($word))
-		{
-			$word = str_replace('\*', '\w+?', preg_quote($word, '#'));
-			$word = preg_replace('#(^|\s)\\\\w\*\?(\s|$)#', '$1\w+?$2', $word);
-			$highlight_match .= (($highlight_match != '') ? '|' : '') . $word;
-		}
-	}
-
-	$highlight = urlencode($hilit_words);
+	$highlight_match = phpbb_clean_search_string($hilit_words);
+	$highlight = urlencode($highlight_match);
+	$highlight_match = str_replace('\*', '\w+?', preg_quote($highlight_match, '#'));
+	$highlight_match = preg_replace('#(?<=^|\s)\\\\w\*\?(?=\s|$)#', '\w+?', $highlight_match);
+	$highlight_match = str_replace(' ', '|', $highlight_match);
 }
 
 // Make sure $start is set to the last page if it exceeds the amount

tests/functions_content/phpbb_clean_search_string_test.php

@@ -0,0 +1,38 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2014 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/functions_content.php';
+
+class phpbb_functions_content_phpbb_clean_search_string_test extends phpbb_test_case
+{
+	public function phpbb_clean_search_string_data()
+	{
+		return array(
+			array('*', ''),
+			array('* *', ''),
+			array('test', 'test'),
+			array(' test ', 'test'),
+			array(' test * ', 'test'),
+			array('test* *', 'test*'),
+			array('* *test*', '*test*'),
+			array('test  test  * test', 'test test test'),
+			array(' some  wild*cards *    between wo*rds  ', 'some wild*cards between wo*rds'),
+			array(' we * now have*** multiple wild***cards * ', 'we now have* multiple wild*cards'),
+			array('pi is *** . * **** * *****', 'pi is .'),
+		);
+	}
+
+	/**
+	* @dataProvider phpbb_clean_search_string_data
+	*/
+	public function test_phpbb_clean_search_string($search_string, $expected)
+	{
+		$this->assertEquals($expected, phpbb_clean_search_string($search_string));
+	}
+}