Browse Source

Config + increase default values for search max length and depth (#9280)

Fix https://github.com/FreshRSS/FreshRSS/discussions/9279
Follow-up of https://github.com/FreshRSS/FreshRSS/pull/9277

Values can be overridden in `data/config.php`, or in `./data/config.custom.php` before the install process
Alexandre Alapetite 12 hours ago
parent
commit
61d7925af1

+ 3 - 6
app/Models/BooleanSearch.php

@@ -6,9 +6,6 @@ declare(strict_types=1);
  */
 class FreshRSS_BooleanSearch implements \Stringable {
 
-	private const MAX_SEARCH_LENGTH = 4096;
-	private const MAX_PARENTHESES_DEPTH = 32;
-
 	private string $raw_input = '';
 	/** @var list<FreshRSS_BooleanSearch|FreshRSS_Search> */
 	private array $searches = [];
@@ -36,7 +33,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 		$this->raw_input = $input;
 
 		if ($level === 0) {
-			if (strlen($input) > self::MAX_SEARCH_LENGTH) {
+			if (strlen($input) > FreshRSS_Context::systemConf()->limits['max_search_length']) {
 				throw new Minz_BadRequestException('Search is too long!');
 			}
 			$input = self::escapeLiterals($input);
@@ -231,7 +228,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 	 * @throws Minz_BadRequestException if the search is too long or if the parentheses are nested too deeply
 	 */
 	public static function consistentOrParentheses(string $input): string {
-		if (strlen($input) > self::MAX_SEARCH_LENGTH) {
+		if (strlen($input) > FreshRSS_Context::systemConf()->limits['max_search_length']) {
 			throw new Minz_BadRequestException('Search is too long!');
 		}
 		if (!preg_match('/(?<!\\\\)\\(/', $input)) {
@@ -259,7 +256,7 @@ class FreshRSS_BooleanSearch implements \Stringable {
 						}
 						$c = '';
 					}
-					if ($parenthesesCount >= self::MAX_PARENTHESES_DEPTH) {	// @phpstan-ignore greaterOrEqual.alwaysFalse
+					if ($parenthesesCount >= FreshRSS_Context::systemConf()->limits['max_search_parentheses_depth']) {
 						throw new Minz_BadRequestException('Search has too deeply nested parentheses!');
 					}
 					$parenthesesCount++;

+ 5 - 0
config.default.php

@@ -152,6 +152,11 @@ return [
 		# Limits for regex, useful to limit regex during user searches
 		'regex_backtrack_limit' => 10000,
 		'regex_recursion_limit' => 100,
+
+		# Max length of a Boolean search query, in bytes
+		'max_search_length' => 16384,
+		# Max depth of parentheses nesting in a Boolean search query
+		'max_search_parentheses_depth' => 32,
 	],
 
 	# Options used by cURL when making HTTP requests, e.g. when the SimplePie library retrieves feeds.

+ 18 - 3
tests/app/Models/BooleanSearchTest.php

@@ -5,6 +5,13 @@ use PHPUnit\Framework\Attributes\DataProvider;
 
 final class BooleanSearchTest extends \PHPUnit\Framework\TestCase {
 
+	public function __construct(string $name) {
+		parent::__construct($name);
+		if (!FreshRSS_Context::hasSystemConf()) {
+			FreshRSS_Context::initSystem();
+		}
+	}
+
 	/**
 	 * `FreshRSS_BooleanSearch::prepend()` is used to restrict an existing search with an extra condition,
 	 * such as the maximum publication date of the “mark as read → articles older than one day/week” action.
@@ -31,10 +38,19 @@ final class BooleanSearchTest extends \PHPUnit\Framework\TestCase {
 		self::assertSame($expectedValues, $values);
 	}
 
+	public function test_constructor_acceptsSearchesAtTheLimits(): void {
+		$input = str_repeat('a', FreshRSS_Context::systemConf()->limits['max_search_length']);
+		self::assertSame($input, (string)new FreshRSS_BooleanSearch($input));
+		$input = str_repeat('(', FreshRSS_Context::systemConf()->limits['max_search_parentheses_depth']) . 'ab' .
+			str_repeat(')', FreshRSS_Context::systemConf()->limits['max_search_parentheses_depth']);
+		self::assertSame('ab', (string)new FreshRSS_BooleanSearch($input));
+	}
+
 	/** @return list<list{string}> */
 	public static function provideTooLongOrTooDeepSearches(): array {
-		$tooLong = str_repeat('ab ', 1400);	// Long enough to exceed the maximum search length
-		$tooDeep = str_repeat('(', 40) . 'ab' . str_repeat(')', 40);	// Deeper than the maximum parentheses depth
+		$tooLong = str_repeat('a', FreshRSS_Context::systemConf()->limits['max_search_length'] + 1);
+		$tooDeep = str_repeat('(', FreshRSS_Context::systemConf()->limits['max_search_parentheses_depth'] + 1) . 'ab' .
+			str_repeat(')', FreshRSS_Context::systemConf()->limits['max_search_parentheses_depth'] + 1);
 		return [
 			[$tooLong],
 			[$tooDeep],
@@ -44,7 +60,6 @@ final class BooleanSearchTest extends \PHPUnit\Framework\TestCase {
 	#[DataProvider('provideTooLongOrTooDeepSearches')]
 	public function test_constructor_rejectsTooLongOrTooDeepSearches(string $input): void {
 		self::expectException(Minz_BadRequestException::class);
-		// Tests run at the default PHP memory limit; a brute-force 1400-deep search would consume too much memory
 		new FreshRSS_BooleanSearch($input);
 	}
 }

+ 5 - 3
tests/app/Models/EntryTest.php

@@ -3,9 +3,11 @@ declare(strict_types=1);
 
 final class EntryTest extends \PHPUnit\Framework\TestCase {
 
-	#[\Override]
-	public static function setUpBeforeClass(): void {
-		FreshRSS_Context::initSystem();
+	public function __construct(string $name) {
+		parent::__construct($name);
+		if (!FreshRSS_Context::hasSystemConf()) {
+			FreshRSS_Context::initSystem();
+		}
 	}
 
 	/**

+ 7 - 0
tests/app/Models/SearchTest.php

@@ -7,6 +7,13 @@ require_once LIB_PATH . '/lib_date.php';
 
 final class SearchTest extends \PHPUnit\Framework\TestCase {
 
+	public function __construct(string $name) {
+		parent::__construct($name);
+		if (!FreshRSS_Context::hasSystemConf()) {
+			FreshRSS_Context::initSystem();
+		}
+	}
+
 	#[DataProvider('provideEmptyInput')]
 	public static function test__construct_whenInputIsEmpty_getsOnlyNullValues(string $input): void {
 		$search = new FreshRSS_Search($input);

+ 5 - 3
tests/app/Models/SimplePieCustomTest.php

@@ -9,9 +9,11 @@ use PHPUnit\Framework\Attributes\DataProvider;
  */
 final class SimplePieCustomTest extends \PHPUnit\Framework\TestCase {
 
-	#[\Override]
-	public static function setUpBeforeClass(): void {
-		FreshRSS_Context::initSystem();
+	public function __construct(string $name) {
+		parent::__construct($name);
+		if (!FreshRSS_Context::hasSystemConf()) {
+			FreshRSS_Context::initSystem();
+		}
 	}
 
 	public static function test_sanitizeHTML_whenEmptyString_returnsEmptyString(): void {

+ 7 - 0
tests/app/Models/UserQueryTest.php

@@ -8,6 +8,13 @@ use PHPUnit\Framework\TestCase;
  */
 class UserQueryTest extends TestCase {
 
+	public function __construct(string $name) {
+		parent::__construct($name);
+		if (!FreshRSS_Context::hasSystemConf()) {
+			FreshRSS_Context::initSystem();
+		}
+	}
+
 	public static function test__construct_whenAllQuery_storesAllParameters(): void {
 		$query = ['get' => 'a'];
 		$user_query = new FreshRSS_UserQuery($query, [], []);