From 252703305758e2ed0e6257ae94fdb3b4b90f7184 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Sat, 3 Jan 2026 16:52:33 +0100 Subject: Fix unwanted expansion of user queries in some cases (#8395) fix https://github.com/FreshRSS/FreshRSS/issues/8378 --- app/Controllers/indexController.php | 6 +- app/Controllers/subscriptionController.php | 4 +- app/Models/BooleanSearch.php | 88 ++++++++++++++--------- app/Models/FilterAction.php | 2 +- app/Models/FilterActionsTrait.php | 4 +- app/Models/Search.php | 2 +- app/Models/UserQuery.php | 4 +- app/layout/header.phtml | 2 +- app/layout/nav_menu.phtml | 6 +- app/views/configure/queries.phtml | 4 +- app/views/configure/reading.phtml | 4 +- app/views/helpers/category/update.phtml | 2 +- app/views/helpers/configure/query.phtml | 3 +- app/views/helpers/export/opml.phtml | 2 +- app/views/helpers/feed/update.phtml | 2 +- app/views/helpers/stream-footer.phtml | 2 +- app/views/tag/update.phtml | 2 +- p/api/query.php | 10 +-- tests/app/Models/SearchTest.php | 108 ++++++++++++++++++++++++++--- 19 files changed, 184 insertions(+), 73 deletions(-) diff --git a/app/Controllers/indexController.php b/app/Controllers/indexController.php index 0d704c496..ca88c9171 100644 --- a/app/Controllers/indexController.php +++ b/app/Controllers/indexController.php @@ -94,8 +94,8 @@ class FreshRSS_index_Controller extends FreshRSS_ActionController { }; $searchString = $operator . ':' . ($offset < 0 ? '/' : '') . date('Y-m-d', $timestamp + ($offset * 86400)) . ($offset > 0 ? '/' : ''); return Minz_Url::display(Minz_Request::modifiedCurrentRequest([ - 'search' => FreshRSS_Context::$search->__toString() === '' ? $searchString : - FreshRSS_Context::$search->enforce(new FreshRSS_Search($searchString))->__toString(), + 'search' => FreshRSS_Context::$search->toString() === '' ? $searchString : + FreshRSS_Context::$search->enforce(new FreshRSS_Search($searchString))->toString(), ])); } @@ -135,7 +135,7 @@ class FreshRSS_index_Controller extends FreshRSS_ActionController { $this->view->rss_title = FreshRSS_Context::$name . ' | ' . FreshRSS_View::title(); $title = FreshRSS_Context::$name; - $search = FreshRSS_Context::$search->__toString(); + $search = FreshRSS_Context::$search->toString(expandUserQueries: false); if ($search !== '') { $title = '“' . htmlspecialchars($search, ENT_COMPAT, 'UTF-8') . '”'; } diff --git a/app/Controllers/subscriptionController.php b/app/Controllers/subscriptionController.php index 02942583f..eb9fbf58f 100644 --- a/app/Controllers/subscriptionController.php +++ b/app/Controllers/subscriptionController.php @@ -421,7 +421,7 @@ class FreshRSS_subscription_Controller extends FreshRSS_ActionController { $actionsSearch = new FreshRSS_BooleanSearch('', operator: 'AND'); foreach ($filteractions as $action) { $actionSearch = new FreshRSS_BooleanSearch($action, operator: 'OR'); - if ($actionSearch->__toString() === '') { + if ($actionSearch->toString() === '') { continue; } $actionsSearch->add($actionSearch); @@ -433,7 +433,7 @@ class FreshRSS_subscription_Controller extends FreshRSS_ActionController { 'c' => 'index', 'a' => 'index', 'params' => [ - 'search' => $search->__toString(), + 'search' => $search->toString(), ], ], redirect: true); } diff --git a/app/Models/BooleanSearch.php b/app/Models/BooleanSearch.php index 1c0d6c18c..263fa8a58 100644 --- a/app/Models/BooleanSearch.php +++ b/app/Models/BooleanSearch.php @@ -20,7 +20,8 @@ class FreshRSS_BooleanSearch implements \Stringable { string $input, int $level = 0, private readonly string $operator = 'AND', - bool $allowUserQueries = true + bool $allowUserQueries = true, + bool $expandUserQueries = true ) { $input = trim($input); if ($input === '') { @@ -30,8 +31,10 @@ class FreshRSS_BooleanSearch implements \Stringable { if ($level === 0) { $input = self::escapeLiterals($input); - $input = $this->parseUserQueryNames($input, $allowUserQueries); - $input = $this->parseUserQueryIds($input, $allowUserQueries); + if ($expandUserQueries || !$allowUserQueries) { + $input = $this->parseUserQueryNames($input, $allowUserQueries); + $input = $this->parseUserQueryIds($input, $allowUserQueries); + } $input = trim($input); } @@ -46,6 +49,8 @@ class FreshRSS_BooleanSearch implements \Stringable { foreach ($this->searches as $key => $search) { $this->searches[$key] = clone $search; } + $this->expanded = null; + $this->notExpanded = null; } /** @@ -76,13 +81,11 @@ class FreshRSS_BooleanSearch implements \Stringable { } for ($i = count($matches['search']) - 1; $i >= 0; $i--) { $name = trim($matches['search'][$i]); - if (!empty($queries[$name])) { - $fromS[] = $matches[0][$i]; - if ($allowUserQueries) { - $toS[] = '(' . self::escapeLiterals($queries[$name]) . ')'; - } else { - $toS[] = ''; - } + $fromS[] = $matches[0][$i]; + if ($allowUserQueries && !empty($queries[$name])) { + $toS[] = '(' . self::escapeLiterals($queries[$name]) . ')'; + } else { + $toS[] = ''; } } } @@ -124,12 +127,9 @@ class FreshRSS_BooleanSearch implements \Stringable { $matchedQueries[] = $queries[$id]; } } - if (empty($matchedQueries)) { - continue; - } $fromS[] = $matches[0][$i]; - if ($allowUserQueries) { + if ($allowUserQueries && !empty($matchedQueries)) { $escapedQueries = array_map(fn(string $query): string => self::escapeLiterals($query), $matchedQueries); $toS[] = '(' . implode(') OR (', $escapedQueries) . ')'; } else { @@ -447,6 +447,8 @@ class FreshRSS_BooleanSearch implements \Stringable { public function enforce(FreshRSS_Search $search): self { $result = clone $this; $result->raw_input = ''; + $result->expanded = null; + $result->notExpanded = null; if (count($result->searches) === 1 && $result->searches[0] instanceof FreshRSS_Search) { $result->searches[0] = $result->searches[0]->enforce($search); @@ -489,6 +491,8 @@ class FreshRSS_BooleanSearch implements \Stringable { public function remove(FreshRSS_Search $search): self { $result = clone $this; $result->raw_input = ''; + $result->expanded = null; + $result->notExpanded = null; if (count($result->searches) === 1 && $result->searches[0] instanceof FreshRSS_Search) { $result->searches[0] = $result->searches[0]->remove($search); @@ -511,33 +515,53 @@ class FreshRSS_BooleanSearch implements \Stringable { return $result; } + private ?string $expanded = null; + #[\Override] public function __toString(): string { - $result = ''; - foreach ($this->searches as $search) { - $part = $search->__toString(); - if ($part === '') { - continue; - } - $operator = $search instanceof FreshRSS_BooleanSearch ? $search->operator : 'OR'; + if ($this->expanded === null) { + $result = ''; + foreach ($this->searches as $search) { + $part = $search->__toString(); + if ($part === '') { + continue; + } + $operator = $search instanceof FreshRSS_BooleanSearch ? $search->operator : 'OR'; + + if ((str_contains($part, ' ') || str_starts_with($part, '-')) && (count($this->searches) > 1 || in_array($operator, ['OR NOT', 'AND NOT'], true))) { + $part = '(' . $part . ')'; + } - if ((str_contains($part, ' ') || str_starts_with($part, '-')) && (count($this->searches) > 1 || in_array($operator, ['OR NOT', 'AND NOT'], true))) { - $part = '(' . $part . ')'; + $result .= match ($operator) { + 'OR' => $result === '' ? '' : ' OR ', + 'OR NOT' => $result === '' ? '-' : ' OR -', + 'AND NOT' => $result === '' ? '-' : ' -', + 'AND' => $result === '' ? '' : ' ', + default => throw new InvalidArgumentException('Invalid operator: ' . $operator), + } . $part; } + $this->expanded = trim($result); + } + return $this->expanded; + } - $result .= match ($operator) { - 'OR' => $result === '' ? '' : ' OR ', - 'OR NOT' => $result === '' ? '-' : ' OR -', - 'AND NOT' => $result === '' ? '-' : ' -', - 'AND' => $result === '' ? '' : ' ', - default => throw new InvalidArgumentException('Invalid operator: ' . $operator), - } . $part; + private ?string $notExpanded = null; + + /** + * @param bool $expandUserQueries Whether to expand user queries (saved searches) or not + */ + public function toString(bool $expandUserQueries = true): string { + if ($expandUserQueries) { + return $this->__toString(); } - return trim($result); + if ($this->notExpanded === null) { + $this->notExpanded = (new FreshRSS_BooleanSearch($this->raw_input, expandUserQueries: false))->__toString(); + } + return $this->notExpanded; } /** @return string Plain text search query. Must be XML-encoded or URL-encoded depending on the situation */ - #[Deprecated('Use __tostring() instead')] + #[Deprecated('Use __toString(expanded: false) instead')] public function getRawInput(): string { return $this->raw_input; } diff --git a/app/Models/FilterAction.php b/app/Models/FilterAction.php index 5d11d26db..9fc965733 100644 --- a/app/Models/FilterAction.php +++ b/app/Models/FilterAction.php @@ -33,7 +33,7 @@ class FreshRSS_FilterAction { public function toJSON(): array { if (is_array($this->actions) && $this->booleanSearch != null) { return [ - 'search' => $this->booleanSearch->__toString(), + 'search' => $this->booleanSearch->toString(expandUserQueries: false), 'actions' => $this->actions, ]; } diff --git a/app/Models/FilterActionsTrait.php b/app/Models/FilterActionsTrait.php index 819f2d975..6f4eafd6e 100644 --- a/app/Models/FilterActionsTrait.php +++ b/app/Models/FilterActionsTrait.php @@ -71,7 +71,7 @@ trait FreshRSS_FilterActionsTrait { //Check existing filters for ($i = count($filterActions) - 1; $i >= 0; $i--) { $filterAction = $filterActions[$i]; - if ($filterAction === null || !is_array($filterAction->actions()) || $filterAction->booleanSearch()->__toString() === '') { + if ($filterAction === null || !is_array($filterAction->actions()) || $filterAction->booleanSearch()->toString() === '') { array_splice($filterActions, $i, 1); continue; } @@ -85,7 +85,7 @@ trait FreshRSS_FilterActionsTrait { //Update existing filter with new action for ($k = count($filters) - 1; $k >= 0; $k--) { $filter = $filters[$k]; - if ($filter === $filterAction->booleanSearch()->__toString()) { + if ($filter === $filterAction->booleanSearch()->toString()) { $actions[] = $action; array_splice($filters, $k, 1); } diff --git a/app/Models/Search.php b/app/Models/Search.php index 80db5a58c..4d33f36b2 100644 --- a/app/Models/Search.php +++ b/app/Models/Search.php @@ -447,7 +447,7 @@ class FreshRSS_Search implements \Stringable { return trim($result); } - #[Deprecated('Use __tostring() instead')] + #[Deprecated('Use __toString() instead')] public function getRawInput(): string { return $this->raw_input; } diff --git a/app/Models/UserQuery.php b/app/Models/UserQuery.php index 154f4d92c..919330e64 100644 --- a/app/Models/UserQuery.php +++ b/app/Models/UserQuery.php @@ -123,7 +123,7 @@ class FreshRSS_UserQuery { 'get' => $this->get, 'name' => $this->name, 'order' => $this->order, - 'search' => $this->search->__toString(), + 'search' => $this->search->toString(expandUserQueries: false), 'state' => $this->state, 'url' => $this->url, 'token' => $this->token, @@ -221,7 +221,7 @@ class FreshRSS_UserQuery { * Check if there is a search in the search object */ public function hasSearch(): bool { - return $this->search->__toString() !== ''; + return $this->search->toString() !== ''; } public function getGet(): string { diff --git a/app/layout/header.phtml b/app/layout/header.phtml index 73dfb3119..56856f84c 100644 --- a/app/layout/header.phtml +++ b/app/layout/header.phtml @@ -40,7 +40,7 @@
diff --git a/app/layout/nav_menu.phtml b/app/layout/nav_menu.phtml index 3c3a34243..debc06611 100644 --- a/app/layout/nav_menu.phtml +++ b/app/layout/nav_menu.phtml @@ -39,7 +39,7 @@ -