Просмотр исходного кода

Fix user query parsing (#8543)

* Fix user query parsing
Fix https://github.com/FreshRSS/FreshRSS/issues/8531
We used to take some shortcuts in the code, but now that the logic has complexified, we need to parse those user queries more properly.

* More fixes
Alexandre Alapetite 1 месяц назад
Родитель
Сommit
b68b80e5de
3 измененных файлов с 187 добавлено и 10 удалено
  1. 3 2
      app/Models/BooleanSearch.php
  2. 131 0
      app/Models/Search.php
  3. 53 8
      tests/app/Models/SearchTest.php

+ 3 - 2
app/Models/BooleanSearch.php

@@ -61,7 +61,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 		if (preg_match_all('/\bsearch:(?P<delim>[\'"])(?P<search>.*)(?P=delim)/U', $input, $matchesFound)) {
 			$all_matches[] = $matchesFound;
 		}
-		if (preg_match_all('/\bsearch:(?P<search>[^\s"]*)/', $input, $matchesFound)) {
+		if (preg_match_all('/\bsearch:(?P<search>[^\s"\']*)/', $input, $matchesFound)) {
 			$all_matches[] = $matchesFound;
 		}
 
@@ -81,6 +81,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 				}
 				for ($i = count($matches['search']) - 1; $i >= 0; $i--) {
 					$name = trim($matches['search'][$i]);
+					$name = self::unescapeLiterals($name);
 					$fromS[] = $matches[0][$i];
 					if ($allowUserQueries && !empty($queries[$name])) {
 						$toS[] = '(' . self::escapeLiterals($queries[$name]) . ')';
@@ -131,7 +132,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 					$fromS[] = $matches[0][$i];
 					if ($allowUserQueries && !empty($matchedQueries)) {
 						$escapedQueries = array_map(fn(string $query): string => self::escapeLiterals($query), $matchedQueries);
-						$toS[] = '(' . implode(') OR (', $escapedQueries) . ')';
+						$toS[] = '((' . implode(') OR (', $escapedQueries) . '))';
 					} else {
 						$toS[] = '';
 					}

+ 131 - 0
app/Models/Search.php

@@ -27,6 +27,10 @@ class FreshRSS_Search implements \Stringable {
 	private $label_ids = null;
 	/** @var list<list<string>>|null */
 	private ?array $label_names = null;
+	/** @var list<list<int>|'*'>|null */
+	private $user_query_ids = null;
+	/** @var list<list<string>>|null */
+	private ?array $user_query_names = null;
 	/** @var list<string>|null */
 	private ?array $intitle = null;
 	/** @var list<string>|null */
@@ -74,6 +78,10 @@ class FreshRSS_Search implements \Stringable {
 	private $not_label_ids = null;
 	/** @var list<list<string>>|null */
 	private ?array $not_label_names = null;
+	/** @var list<list<int>|'*'>|null */
+	private $not_user_query_ids = null;
+	/** @var list<list<string>>|null */
+	private ?array $not_user_query_names = null;
 	/** @var list<string>|null */
 	private ?array $not_intitle = null;
 	/** @var list<string>|null */
@@ -122,6 +130,8 @@ class FreshRSS_Search implements \Stringable {
 		$input = $this->parseNotCategoryIds($input);
 		$input = $this->parseNotLabelIds($input);
 		$input = $this->parseNotLabelNames($input);
+		$input = $this->parseNotUserQueryIds($input);
+		$input = $this->parseNotUserQueryNames($input);
 
 		$input = $this->parseNotUserdateSearch($input);
 		$input = $this->parseNotModifiedDateSearch($input);
@@ -139,6 +149,8 @@ class FreshRSS_Search implements \Stringable {
 		$input = $this->parseCategoryIds($input);
 		$input = $this->parseLabelIds($input);
 		$input = $this->parseLabelNames($input);
+		$input = $this->parseUserQueryIds($input);
+		$input = $this->parseUserQueryNames($input);
 
 		$input = $this->parseUserdateSearch($input);
 		$input = $this->parseModifiedDateSearch($input);
@@ -367,6 +379,16 @@ class FreshRSS_Search implements \Stringable {
 				$result .= ' ' . self::quote($s);
 			}
 		}
+		if ($this->user_query_ids !== null) {
+			foreach ($this->user_query_ids as $ids) {
+				$result .= ' S:' . (is_array($ids) ? implode(',', $ids) : $ids);
+			}
+		}
+		if ($this->user_query_names !== null) {
+			foreach ($this->user_query_names as $names) {
+				$result .= ' search:' . self::quote(implode(',', $names));
+			}
+		}
 
 		if ($this->not_entry_ids !== null) {
 			$result .= ' -e:' . implode(',', $this->not_entry_ids);
@@ -461,6 +483,16 @@ class FreshRSS_Search implements \Stringable {
 				$result .= ' -' . self::quote($s);
 			}
 		}
+		if ($this->not_user_query_ids !== null) {
+			foreach ($this->not_user_query_ids as $ids) {
+				$result .= ' -S:' . (is_array($ids) ? implode(',', $ids) : $ids);
+			}
+		}
+		if ($this->not_user_query_names !== null) {
+			foreach ($this->not_user_query_names as $names) {
+				$result .= ' -search:' . self::quote(implode(',', $names));
+			}
+		}
 
 		return trim($result);
 	}
@@ -821,6 +853,105 @@ class FreshRSS_Search implements \Stringable {
 		return $input;
 	}
 
+	/**
+	 * Parse the search string to find user query IDs.
+	 */
+	private function parseUserQueryIds(string $input): string {
+		if (preg_match_all('/\\b[S]:(?P<search>[0-9,]+|[*])/', $input, $matches)) {
+			$input = str_replace($matches[0], '', $input);
+			$ids_lists = $matches['search'];
+			$this->user_query_ids = [];
+			foreach ($ids_lists as $ids_list) {
+				if ($ids_list === '*') {
+					$this->user_query_ids[] = '*';
+					break;
+				}
+				$user_query_ids = explode(',', $ids_list);
+				$user_query_ids = self::removeEmptyValues($user_query_ids);
+				/** @var list<int> $user_query_ids */
+				$user_query_ids = array_map('intval', $user_query_ids);
+				if (!empty($user_query_ids)) {
+					$this->user_query_ids[] = $user_query_ids;
+				}
+			}
+		}
+		return $input;
+	}
+
+	private function parseNotUserQueryIds(string $input): string {
+		if (preg_match_all('/(?<=[\\s(]|^)[!-][S]:(?P<search>[0-9,]+|[*])/', $input, $matches)) {
+			$input = str_replace($matches[0], '', $input);
+			$ids_lists = $matches['search'];
+			$this->not_user_query_ids = [];
+			foreach ($ids_lists as $ids_list) {
+				if ($ids_list === '*') {
+					$this->not_user_query_ids[] = '*';
+					break;
+				}
+				$user_query_ids = explode(',', $ids_list);
+				$user_query_ids = self::removeEmptyValues($user_query_ids);
+				/** @var list<int> $user_query_ids */
+				$user_query_ids = array_map('intval', $user_query_ids);
+				if (!empty($user_query_ids)) {
+					$this->not_user_query_ids[] = $user_query_ids;
+				}
+			}
+		}
+		return $input;
+	}
+
+	/**
+	 * Parse the search string to find user query names.
+	 */
+	private function parseUserQueryNames(string $input): string {
+		$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);
+		}
+		if (preg_match_all('/\\bsearch?:(?P<search>[^\s"]*)/', $input, $matches)) {
+			$names_lists = array_merge($names_lists, $matches['search']);
+			$input = str_replace($matches[0], '', $input);
+		}
+		if (!empty($names_lists)) {
+			$this->user_query_names = [];
+			foreach ($names_lists as $names_list) {
+				$names_array = explode(',', $names_list);
+				$names_array = self::removeEmptyValues($names_array);
+				if (!empty($names_array)) {
+					$this->user_query_names[] = $names_array;
+				}
+			}
+		}
+		return $input;
+	}
+
+	/**
+	 * Parse the search string to find user query names to exclude.
+	 */
+	private function parseNotUserQueryNames(string $input): string {
+		$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);
+		}
+		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);
+		}
+		if (!empty($names_lists)) {
+			$this->not_user_query_names = [];
+			foreach ($names_lists as $names_list) {
+				$names_array = explode(',', $names_list);
+				$names_array = self::removeEmptyValues($names_array);
+				if (!empty($names_array)) {
+					$this->not_user_query_names[] = $names_array;
+				}
+			}
+		}
+		return $input;
+	}
+
 	/**
 	 * Parse the search string to find tags (labels) IDs.
 	 */

+ 53 - 8
tests/app/Models/SearchTest.php

@@ -266,7 +266,8 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 	 * @param array{0:string,1:list<string|int>} $expectedResult
 	 */
 	#[DataProvider('provideSavedQueriesExpansion')]
-	public static function test__construct_whenInputContainsSavedQueries_expandsSavedSearches(array $queries, string $input, array $expectedResult): void {
+	public static function test__construct_whenInputContainsSavedQueries_expandsSavedSearches(array $queries, string $input,
+		array $expectedResult, string $expectedToString): void {
 		$previousUserConf = FreshRSS_Context::hasUserConf() ? FreshRSS_Context::userConf() : null;
 		$newUserConf = $previousUserConf instanceof FreshRSS_UserConfiguration ? clone $previousUserConf : clone FreshRSS_UserConfiguration::default();
 		$newUserConf->queries = $queries;
@@ -277,13 +278,14 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 			[$actualValues, $actualSql] = FreshRSS_EntryDAOPGSQL::sqlBooleanSearch('e.', $search);
 			self::assertSame($expectedResult[0], trim($actualSql));
 			self::assertSame($expectedResult[1], $actualValues);
+			self::assertSame($expectedToString, $search->toString(expandUserQueries: false));
 		} finally {
 			FreshRSS_Context::setUserConf($previousUserConf);
 		}
 	}
 
 	/**
-	 * @return array<string,array{0:list<array{search:string}>,1:string,2:array{0:string,1:list<string|int>}}>
+	 * @return array<string,array{0:list<array{search:string}>,1:string,2:array{0:string,1:list<string|int>},3:string}>
 	 */
 	public static function provideSavedQueriesExpansion(): array {
 		return [
@@ -297,6 +299,7 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 					'',
 					[],
 				],
+				'S:3',
 			],
 			'not found name' => [
 				[
@@ -308,6 +311,7 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 					'',
 					[],
 				],
+				'search:Third',
 			],
 			'expanded single group name' => [
 				[
@@ -319,6 +323,45 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 					'((e.author LIKE ?)) OR ((e.title LIKE ?))',
 					['%Alice%', '%World%'],
 				],
+				'search:First OR search:Second',
+			],
+			'expanded single group name quotes' => [
+				[
+					['search' => 'author:Alice', 'name' => 'A First'],
+					['search' => 'intitle:World', 'name' => 'A Second'],
+				],
+				'search:"A First" OR search:\'A Second\'',
+				[
+					'((e.author LIKE ?)) OR ((e.title LIKE ?))',
+					['%Alice%', '%World%'],
+				],
+				'(search:"A First") OR (search:"A Second")',
+			],
+			'expanded single group name quotes special characters' => [
+				[
+					['search' => 'author:Alice', 'name' => 'A or B'],
+					['search' => 'intitle:World', 'name' => '(C OR D)'],
+				],
+				'search:"A or B" OR search:\'(C OR D)\'',
+				[
+					'((e.author LIKE ?)) OR ((e.title LIKE ?))',
+					['%Alice%', '%World%'],
+				],
+				'(search:"A or B") OR (search:"(C OR D)")',
+			],
+			'separate groups with AND' => [
+				[
+					['search' => 'author:Alice'],
+					['search' => 'intitle:World'],
+					['search' => 'inurl:Example'],
+					['search' => 'author:Bob'],
+				],
+				'S:0,1 S:2,3,5',
+				[
+					'(((e.author LIKE ?)) OR ((e.title LIKE ?))) AND (((e.link LIKE ?)) OR ((e.author LIKE ?)))',
+					['%Alice%', '%World%', '%Example%', '%Bob%'],
+				],
+				'S:0,1 S:2,3,5',
 			],
 			'separate groups with OR' => [
 				[
@@ -329,20 +372,22 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 				],
 				'S:0,1 OR S:2,3,5',
 				[
-					'((e.author LIKE ?)) OR ((e.title LIKE ?)) OR ((e.link LIKE ?)) OR ((e.author LIKE ?))',
+					'(((e.author LIKE ?)) OR ((e.title LIKE ?))) OR (((e.link LIKE ?)) OR ((e.author LIKE ?)))',
 					['%Alice%', '%World%', '%Example%', '%Bob%'],
 				],
+				'S:0,1 OR S:2,3,5',
 			],
 			'mixed with other clauses' => [
 				[
 					['search' => 'author:Alice'],
 					['search' => 'intitle:World'],
 				],
-				'intitle:Hello S:0,1 date:2025-10',
+				'date:2025-10 intitle:Hello S:0,1',
 				[
-					'((e.title LIKE ?)) AND ((e.author LIKE ?)) OR ((e.title LIKE ?)) AND ((e.id >= ? AND e.id <= ?))',
-					['%Hello%', '%Alice%', '%World%', strtotime('2025-10-01') . '000000', (strtotime('2025-11-01') - 1) . '000000'],
+					'((e.id >= ? AND e.id <= ? AND e.title LIKE ?)) AND (((e.author LIKE ?)) OR ((e.title LIKE ?)))',
+					[strtotime('2025-10-01') . '000000', (strtotime('2025-11-01') - 1) . '000000', '%Hello%', '%Alice%', '%World%'],
 				],
+				'date:2025-10 intitle:Hello S:0,1',
 			],
 		];
 	}
@@ -1061,7 +1106,7 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 					author:/Bob/ author:"/u/Alice" author:Alice
 					inurl:/https/ inurl:example.net
 					#/tag2/ #tag1
-					/search_regex/i "quoted search" search
+					/search_regex/i "quoted search" search search:"A user search" search:U1
 					-e:3,4 -f:12,13 -c:22,23 -L:32,33 -labels:"Not label,Not other label"
 					-userdate:2025-06-01T00:00:00/2025-09-01T00:00:00
 					-mdate:2025-12-27
@@ -1072,7 +1117,7 @@ final class SearchTest extends \PHPUnit\Framework\TestCase {
 					-author:/Dave/i -author:"/u/Charlie" -author:Charlie
 					-inurl:/ftp/ -inurl:example.com
 					-#/tag4/ -#tag3
-					-/not_regex/i -"not quoted" -not_search
+					-/not_regex/i -"not quoted" -not_search -search:"Negative user search" -search:U2
 					EOD
 			],
 		];