From 44366f48bf1a6cf96a25ba7fae5e653bbe0cd079 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sat, 2 Mar 2019 13:53:43 -0500 Subject: [PATCH] Remove arbitrary search term limits; fixes #150 --- README.md | 1 - lib/AbstractException.php | 1 + lib/Database.php | 39 +++++++++++---------- locale/en.php | 2 ++ tests/cases/Database/SeriesArticle.php | 11 +----- tests/cases/REST/TinyTinyRSS/TestSearch.php | 1 - 6 files changed, 25 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index d4fca7f3..ab7dc2ed 100644 --- a/README.md +++ b/README.md @@ -142,7 +142,6 @@ We are not aware of any other extensions to the TTRSS protocol. If you know of a - Providing the `setArticleLabel` operation with an invalid label normally silently fails; The Arsse returns an `INVALID_USAGE` error instead - Processing of the `search` parameter of the `getHeadlines` operation differs in the following ways: - Values other than `"true"` or `"false"` for the `unread`, `star`, and `pub` special keywords treat the entire token as a search term rather than as `"false"` - - Limits are placed on the number of search terms: ten each for `title`, `author`, and `note`, and twenty for content searching; exceeding the limits will yield a non-standard `TOO_MANY_SEARCH_TERMS` error - Invalid dates are ignored rather than assumed to be `"1970-01-01"` - Only a single negative date is allowed (this is a known bug rather than intentional) - Dates are always relative to UTC diff --git a/lib/AbstractException.php b/lib/AbstractException.php index 0249678e..7df22630 100644 --- a/lib/AbstractException.php +++ b/lib/AbstractException.php @@ -11,6 +11,7 @@ abstract class AbstractException extends \Exception { "Exception.uncoded" => -1, "Exception.unknown" => 10000, "Exception.constantUnknown" => 10001, + "Exception.arrayEmpty" => 10002, "ExceptionType.strictFailure" => 10011, "ExceptionType.typeUnknown" => 10012, "Lang/Exception.defaultFileMissing" => 10101, diff --git a/lib/Database.php b/lib/Database.php index f0987c33..7c47ad39 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -185,23 +185,33 @@ class Database { * @param boolean $matchAny Whether the search is successful when it matches any (true) or all (false) terms */ protected function generateSearch(array $terms, array $cols, bool $matchAny = false): array { + if (!$cols) { + throw new Exception("arrayEmpty", "cols"); // @codeCoverageIgnore + } $clause = []; $types = []; $values = []; $like = $this->db->sqlToken("like"); + $embedSet = sizeof($terms) > ((int) (self::LIMIT_SET_SIZE / sizeof($cols))); foreach($terms as $term) { + $embedTerm = ($embedSet && strlen($term) <= self::LIMIT_SET_STRING_LENGTH); $term = str_replace(["%", "_", "^"], ["^%", "^_", "^^"], $term); $term = "%$term%"; + $term = $embedTerm ? $this->db->literalString($term) : $term; $spec = []; foreach ($cols as $col) { - $spec[] = "$col $like ? escape '^'"; - $types[] = "str"; - $values[] = $term; + if ($embedTerm) { + $spec[] = "$col $like $term escape '^'"; + } else { + $spec[] = "$col $like ? escape '^'"; + $types[] = "str"; + $values[] = $term; + } } $clause[] = "(".implode(" or ", $spec).")"; } $glue = $matchAny ? "or" : "and"; - $clause = "(".implode(" $glue ", $clause).")"; + $clause = $clause ? "(".implode(" $glue ", $clause).")" : ""; return [$clause, $types, $values]; } @@ -1307,34 +1317,27 @@ class Database { } // handle text-matching context options $options = [ - "titleTerms" => [10, ["arsse_articles.title"]], - "searchTerms" => [20, ["arsse_articles.title", "arsse_articles.content"]], - "authorTerms" => [10, ["arsse_articles.author"]], - "annotationTerms" => [20, ["arsse_marks.note"]], + "titleTerms" => ["arsse_articles.title"], + "searchTerms" => ["arsse_articles.title", "arsse_articles.content"], + "authorTerms" => ["arsse_articles.author"], + "annotationTerms" => ["arsse_marks.note"], ]; - foreach ($options as $m => list($max, $cols)) { + foreach ($options as $m => $cols) { if (!$context->$m()) { continue; } elseif (!$context->$m) { throw new Db\ExceptionInput("tooShort", ['field' => $m, 'action' => $this->caller(), 'min' => 1]); // must have at least one array element - } elseif (sizeof($context->$m) > $max) { - throw new Db\ExceptionInput("tooLong", ['field' => $m, 'action' => $this->caller(), 'max' => $max]); } $q->setWhere(...$this->generateSearch($context->$m, $cols)); } // further handle exclusionary text-matching context options - foreach ($options as $m => list($max, $cols)) { - if (!$context->not->$m()) { + foreach ($options as $m => $cols) { + if (!$context->not->$m() || !$context->not->$m) { continue; - } elseif (!$context->not->$m) { - continue; - } elseif (sizeof($context->not->$m) > $max) { - throw new Db\ExceptionInput("tooLong", ['field' => "$m (not)", 'action' => $this->caller(), 'max' => $max]); } $q->setWhereNot(...$this->generateSearch($context->not->$m, $cols, true)); } // return the query - //var_export((string) $q); return $q; } diff --git a/locale/en.php b/locale/en.php index f576442d..5e8ad0fe 100644 --- a/locale/en.php +++ b/locale/en.php @@ -36,6 +36,8 @@ return [ 'Exception.JKingWeb/Arsse/Exception.unknown' => 'An unknown error has occurred', // indicates programming error 'Exception.JKingWeb/Arsse/Exception.constantUnknown' => 'Supplied constant value ({0}) is unknown or invalid in the context in which it was used', + // indicates programming error + 'Exception.JKingWeb/Arsse/Exception.arrayEmpty' => 'Supplied array "{0}" is empty, but should have at least one element', 'Exception.JKingWeb/Arsse/ExceptionType.strictFailure' => 'Supplied value could not be normalized to {0, select, 1 {null} 2 {boolean} diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index e2aa5988..f652c6f8 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -456,6 +456,7 @@ trait SeriesArticle { "Excluded folder tree" => [(new Context)->not->folder(1), [1,2,3,4,19,20]], "Excluding label ID 2" => [(new Context)->not->label(2), [2,3,4,6,7,8,19]], "Excluding label 'Fascinating'" => [(new Context)->not->labelName("Fascinating"), [2,3,4,6,7,8,19]], + "Search 501 terms" => [(new Context)->searchTerms(array_merge(range(1,500),[str_repeat("a", 1000)])), []], ]; } @@ -991,18 +992,8 @@ trait SeriesArticle { Arsse::$db->articleList($this->user, (new Context)->searchTerms([])); } - public function testSearchTooManyTerms() { - $this->assertException("tooLong", "Db", "ExceptionInput"); - Arsse::$db->articleList($this->user, (new Context)->searchTerms(range(1, 105))); - } - public function testSearchTooFewTermsInNote() { $this->assertException("tooShort", "Db", "ExceptionInput"); Arsse::$db->articleList($this->user, (new Context)->annotationTerms([])); } - - public function testSearchTooManyTermsInNote() { - $this->assertException("tooLong", "Db", "ExceptionInput"); - Arsse::$db->articleList($this->user, (new Context)->annotationTerms(range(1, 105))); - } } diff --git a/tests/cases/REST/TinyTinyRSS/TestSearch.php b/tests/cases/REST/TinyTinyRSS/TestSearch.php index 62ad553d..c858d1be 100644 --- a/tests/cases/REST/TinyTinyRSS/TestSearch.php +++ b/tests/cases/REST/TinyTinyRSS/TestSearch.php @@ -120,7 +120,6 @@ class TestSearch extends \JKingWeb\Arsse\Test\AbstractTest { /** @dataProvider provideSearchStrings */ public function testApplySearchToContext(string $search, $exp) { $act = Search::parse($search); - //var_export($act); $this->assertEquals($exp, $act); } }