Fix overlapping saved search references (#9484)

Match the longest saved query reference before shorter references so
expansion does not depend on their order. Apply the same replacement
behaviour when preserving unexpanded searches, including negated ones.

Add regression coverage for overlapping IDs, names, and ID groups.

Refs #9483

Co-authored-by: Busy-SweetPotato <346315145+Busy-SweetPotato@users.noreply.github.com>
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>

CHANGELOG.md

@@ -10,6 +10,7 @@ See also [the FreshRSS releases](https://github.com/FreshRSS/FreshRSS/releases).
 	* Fix HTTP Retry-After cache when proxy is used [#9508](https://github.com/FreshRSS/FreshRSS/pull/9508)
 	* Fix 32-bit systems provide a string for SQL BIGINT [#9486](https://github.com/FreshRSS/FreshRSS/pull/9486)
 	* Keep JSON feed attachments as enclosures [#9510](https://github.com/FreshRSS/FreshRSS/pull/9510)
+	* Fix saved search references with overlapping IDs or names [#9484](https://github.com/FreshRSS/FreshRSS/pull/9484)
 	* Fix non-ASCII characters in scripts of fetched HTML pages [#9384](https://github.com/FreshRSS/FreshRSS/pull/9384)
 * Security
 	* 💥 Replace bcrypt challenge auth with direct password comparison [#9358](https://github.com/FreshRSS/FreshRSS/pull/9358)

CREDITS.md

@@ -57,6 +57,7 @@ People are sorted by name so please keep this order.
 * [Brewal Bouvet](https://github.com/Jucgshu): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:Jucgshu)
 * [Brooke.](https://github.com/BrookeDot): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:BrookeDot), [Web](https://brooke.codes/)
 * [Bryan Butler](https://github.com/BryanButlerGit): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:BryanButlerGit)
+* [Busy-SweetPotato](https://github.com/Busy-SweetPotato): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:Busy-SweetPotato)
 * [caminsha](https://github.com/caminsha): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:caminsha)
 * [CarelessCaution](https://github.com/ghost): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:CarelessCaution)
 * [Carey Metcalfe](https://github.com/pR0Ps): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:pR0Ps), [Web](https://cmetcalfe.ca/)

app/Models/BooleanSearch.php

@@ -97,7 +97,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 				}
 			}
 
-			$input = str_replace($fromS, $toS, $input);
+			$input = strtr($input, array_combine($fromS, $toS));
 		}
 		return $input;
 	}
@@ -145,7 +145,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 				}
 			}
 
-			$input = str_replace($fromS, $toS, $input);
+			$input = strtr($input, array_combine($fromS, $toS));
 		}
 		return $input;
 	}

app/Models/Search.php

@@ -873,7 +873,7 @@ class FreshRSS_Search implements \Stringable {
 	 */
 	private function parseUserQueryIds(string $input): string {
 		if (preg_match_all('/\\b[S]:(?P<search>[0-9,]+|[*])/', $input, $matches)) {
-			$input = str_replace($matches[0], '', $input);
+			$input = strtr($input, array_fill_keys($matches[0], ''));
 			$ids_lists = $matches['search'];
 			$this->user_query_ids = [];
 			foreach ($ids_lists as $ids_list) {
@@ -895,7 +895,7 @@ class FreshRSS_Search implements \Stringable {
 
 	private function parseNotUserQueryIds(string $input): string {
 		if (preg_match_all('/(?<=[\\s(]|^)[!-][S]:(?P<search>[0-9,]+|[*])/', $input, $matches)) {
-			$input = str_replace($matches[0], '', $input);
+			$input = strtr($input, array_fill_keys($matches[0], ''));
 			$ids_lists = $matches['search'];
 			$this->not_user_query_ids = [];
 			foreach ($ids_lists as $ids_list) {
@@ -922,11 +922,11 @@ class FreshRSS_Search implements \Stringable {
 		$names_lists = [];
 		if (preg_match_all('/\\bsearch?:(?P<delim>[\'"])(?P<search>.*)(?P=delim)/U', $input, $matches)) {
 			$names_lists = $matches['search'];
-			$input = str_replace($matches[0], '', $input);
+			$input = strtr($input, array_fill_keys($matches[0], ''));
 		}
 		if (preg_match_all('/\\bsearch?:(?P<search>[^\s"]*)/', $input, $matches)) {
 			$names_lists = array_merge($names_lists, $matches['search']);
-			$input = str_replace($matches[0], '', $input);
+			$input = strtr($input, array_fill_keys($matches[0], ''));
 		}
 		if (!empty($names_lists)) {
 			$this->user_query_names = [];
@@ -948,11 +948,11 @@ class FreshRSS_Search implements \Stringable {
 		$names_lists = [];
 		if (preg_match_all('/(?<=[\\s(]|^)[!-]search?:(?P<delim>[\'"])(?P<search>.*)(?P=delim)/U', $input, $matches)) {
 			$names_lists = $matches['search'];
-			$input = str_replace($matches[0], '', $input);
+			$input = strtr($input, array_fill_keys($matches[0], ''));
 		}
 		if (preg_match_all('/(?<=[\\s(]|^)[!-]search?:(?P<search>[^\\s"]*)/', $input, $matches)) {
 			$names_lists = array_merge($names_lists, $matches['search']);
-			$input = str_replace($matches[0], '', $input);
+			$input = strtr($input, array_fill_keys($matches[0], ''));
 		}
 		if (!empty($names_lists)) {
 			$this->not_user_query_names = [];

tests/app/Models/SearchTest.php

@@ -1230,6 +1230,10 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 	 * @return array<string,array{0:list<array{search:string,name?:string}>,1:string,2:string,3:string}>
 	 */
 	public static function provideBooleanSearchToStringExpansion(): array {
+		$overlappingQueries = array_fill(0, 11, ['search' => 'author:Unused']);
+		$overlappingQueries[1] = ['name' => 'News', 'search' => 'author:Alice'];
+		$overlappingQueries[10] = ['name' => 'Newsletter', 'search' => 'author:Bob'];
+
 		return [
 			'Not found ID' => [
 				[
@@ -1267,6 +1271,60 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 				'search:First search:Second',
 				'author:Alice intitle:World',
 			],
+			'Longer ID first' => [
+				$overlappingQueries,
+				'S:10 OR S:1',
+				'S:10 OR S:1',
+				'author:Bob OR author:Alice',
+			],
+			'Shorter ID first' => [
+				$overlappingQueries,
+				'S:1 OR S:10',
+				'S:1 OR S:10',
+				'author:Alice OR author:Bob',
+			],
+			'Overlapping IDs with AND' => [
+				$overlappingQueries,
+				'S:1 S:10',
+				'S:1 S:10',
+				'author:Alice author:Bob',
+			],
+			'Overlapping ID groups' => [
+				$overlappingQueries,
+				'S:1,10 S:1',
+				'S:1,10 S:1',
+				'(author:Alice OR author:Bob) author:Alice',
+			],
+			'Longer name first' => [
+				$overlappingQueries,
+				'search:Newsletter OR search:News',
+				'search:Newsletter OR search:News',
+				'author:Bob OR author:Alice',
+			],
+			'Shorter name first' => [
+				$overlappingQueries,
+				'search:News OR search:Newsletter',
+				'search:News OR search:Newsletter',
+				'author:Alice OR author:Bob',
+			],
+			'Overlapping names with AND' => [
+				$overlappingQueries,
+				'search:News search:Newsletter',
+				'search:News search:Newsletter',
+				'author:Alice author:Bob',
+			],
+			'Negated overlapping IDs' => [
+				$overlappingQueries,
+				'-S:1 -S:10',
+				'-S:1 -S:10',
+				'-author:Alice -author:Bob',
+			],
+			'Negated overlapping names' => [
+				$overlappingQueries,
+				'-search:News -search:Newsletter',
+				'-search:News -search:Newsletter',
+				'-author:Alice -author:Bob',
+			],
 		];
 	}