diff --git a/classes/models/FrmSpamCheckDenylist.php b/classes/models/FrmSpamCheckDenylist.php index f71323d883..12f8b20d1d 100644 --- a/classes/models/FrmSpamCheckDenylist.php +++ b/classes/models/FrmSpamCheckDenylist.php @@ -17,6 +17,31 @@ class FrmSpamCheckDenylist extends FrmSpamCheck { const COMPARE_EQUALS = 'equals'; + /** + * How many leading characters of a denylist line are used as its index key. + * Four measured best on the shipped denylists: shorter keys are not selective + * enough, and longer ones push more lines below the length the index needs. + * + * @since x.x + */ + const PREFIX_LENGTH = 4; + + /** + * Values shorter than this are not indexed. Comparing them is already cheap + * enough that building the index would cost more than it saves. + * + * @since x.x + */ + const MIN_LENGTH_TO_INDEX = 1024; + + /** + * Values longer than this are not indexed, so the index cannot grow without + * bound on an unusually large submission. + * + * @since x.x + */ + const MAX_LENGTH_TO_INDEX = 524288; + /** * @var array|null */ @@ -263,10 +288,74 @@ protected function add_values_to_check( &$denylist ) { $denylist['values_to_check'] = $values_to_check; $denylist['values_string'] = $values_string; $denylist['values_string_lower'] = $this->convert_to_lowercase( $values_string ); + $denylist['values_prefix_index'] = $this->get_values_prefix_index( $denylist ); return true; } + /** + * Indexes every PREFIX_LENGTH character window of the values. + * + * A line can only be inside the values if its own first PREFIX_LENGTH + * characters are somewhere in them, so a line whose prefix is missing from + * this index cannot match and does not need to be compared at all. The shipped + * denylists hold tens of thousands of lines and each comparison reads the whole + * values string, so ruling a line out with one array lookup is worth the index. + * + * Returns an empty array when the index would not answer for this denylist, or + * would not pay for itself. Every line is then compared as before. + * + * @since x.x + * + * @param array $denylist Denylist data, holding the values strings. + * + * @return array Index of value prefixes, or an empty array for no index. + */ + protected function get_values_prefix_index( $denylist ) { + if ( ! empty( $denylist['is_regex'] ) || self::COMPARE_CONTAINS !== $denylist['compare'] ) { + // A regex line is a pattern rather than a literal, so its leading + // characters are not text to look for. Only "contains" is indexable. + return array(); + } + + $values = $denylist['values_string_lower']; + $length = strlen( $values ); + + if ( $length < self::MIN_LENGTH_TO_INDEX || $length > self::MAX_LENGTH_TO_INDEX ) { + return array(); + } + + $index = array(); + $last = $length - self::PREFIX_LENGTH; + + for ( $i = 0; $i <= $last; $i++ ) { + $index[ substr( $values, $i, self::PREFIX_LENGTH ) ] = true; + } + + return $index; + } + + /** + * Checks the values index to rule a line out before comparing it. + * + * A `false` here does not mean the line matches, only that the index cannot + * rule it out, so the caller still has to compare it. + * + * @since x.x + * + * @param string $line The lowercased denylist line. + * @param array $args Check args, holding the index when there is one. + * + * @return bool True when the line cannot be inside the values. + */ + protected function line_is_ruled_out( $line, $args ) { + if ( empty( $args['values_prefix_index'] ) || strlen( $line ) < self::PREFIX_LENGTH ) { + return false; + } + + return ! isset( $args['values_prefix_index'][ substr( $line, 0, self::PREFIX_LENGTH ) ] ); + } + /** * Gets words from setting. * @@ -325,6 +414,10 @@ protected function single_line_check_values( $line, $args ) { return false; } + if ( $this->line_is_ruled_out( $line, $args ) ) { + return false; + } + return str_contains( $args['values_string_lower'], $line ); } diff --git a/tests/phpunit/misc/test_FrmSpamCheckDenylist.php b/tests/phpunit/misc/test_FrmSpamCheckDenylist.php index 38e129a6af..35daa35988 100644 --- a/tests/phpunit/misc/test_FrmSpamCheckDenylist.php +++ b/tests/phpunit/misc/test_FrmSpamCheckDenylist.php @@ -757,4 +757,176 @@ public function filter_denylist_to_one_word() { ), ); } + + /** + * Text long enough that the values pass MIN_LENGTH_TO_INDEX once encoded. + * + * @return string + */ + private function get_filler_text() { + return str_repeat( 'ordinary sentence about a kitchen remodel. ', 60 ); + } + + /** + * Builds a submission whose values are long enough to be indexed, with the + * given text planted in the middle of them. + * + * @param string $planted Text to plant in the middle of a value. + * + * @return FrmSpamCheckDenylist + */ + private function get_long_submission( $planted = '' ) { + $filler = $this->get_filler_text(); + + // Only the one field, so nothing in the other default values can match. + return new FrmSpamCheckDenylist( + array( + 'form_id' => $this->default_values['form_id'], + 'item_meta' => array( + $this->text_field_id => $filler . $planted . ' ' . $filler, + ), + ) + ); + } + + /** + * Returns the denylist shape get_values_prefix_index() reads: the comparison + * settings and the lowercased values string. + * + * @param string $values_string_lower The values to index. + * @param array $extra Denylist settings to override. + * + * @return array + */ + private function get_indexable_denylist( $values_string_lower, $extra = array() ) { + return array_merge( + array( + 'is_regex' => false, + 'compare' => FrmSpamCheckDenylist::COMPARE_CONTAINS, + 'values_string_lower' => $values_string_lower, + ), + $extra + ); + } + + /** + * Runs get_values_prefix_index() for the given denylist shape. + * + * @param array $denylist Denylist data. + * + * @return array + */ + private function get_prefix_index( $denylist ) { + return $this->run_private_method( + array( $this->spam_check, 'get_values_prefix_index' ), + array( $denylist ) + ); + } + + /** + * The index holds every PREFIX_LENGTH character window of the values, and only + * gets built for values long enough to be worth it. + */ + public function test_get_values_prefix_index() { + $long = strtolower( wp_json_encode( array( $this->get_filler_text() ) ) ); + $index = $this->get_prefix_index( $this->get_indexable_denylist( $long ) ); + + $this->assertNotEmpty( $index ); + + // Every window of the values is a key, and nothing else is. + $this->assertArrayHasKey( 'ordi', $index ); + $this->assertArrayHasKey( 'kitc', $index ); + $this->assertArrayNotHasKey( 'zzzz', $index ); + + // Short values are not indexed, so they are compared exactly as before. + $this->assertSame( array(), $this->get_prefix_index( $this->get_indexable_denylist( 'short values' ) ) ); + + // A regex line is a pattern, not text to look for, so it is never indexed. + $this->assertSame( + array(), + $this->get_prefix_index( $this->get_indexable_denylist( $long, array( 'is_regex' => true ) ) ) + ); + + // An equals comparison is against whole values, not the joined string. + $this->assertSame( + array(), + $this->get_prefix_index( + $this->get_indexable_denylist( $long, array( 'compare' => FrmSpamCheckDenylist::COMPARE_EQUALS ) ) + ) + ); + } + + /** + * The index only rules lines out, so an indexed submission has to reach the + * same verdict as an unindexed one, for words and for file lines. + */ + public function test_prefix_index_does_not_change_the_verdict() { + $long = $this->get_long_submission( 'buy-cheap-widgets.example' ); + + $this->assertTrue( + $this->check_values_with( array( array( 'words' => array( 'buy-cheap-widgets.example' ) ) ), $long ) + ); + $this->assertFalse( + $this->check_values_with( array( array( 'words' => array( 'zzzz-not-in-the-values' ) ) ), $long ) + ); + + // A word that shares its first characters with the values but is not in + // them survives the index and is then ruled out by the comparison. + $this->assertFalse( + $this->check_values_with( array( array( 'words' => array( 'kitchen-remodel-spam.example' ) ) ), $long ) + ); + } + + /** + * A match on the last line of a file is still found once the lines before it + * have been ruled out by the index. + */ + public function test_prefix_index_still_finds_a_match_on_the_last_line() { + $denylist = array( + array( + // The file is `wordpress`, a blank line, then `plugin`. + 'file' => __DIR__ . '/denylist-email-contain.txt', + ), + ); + + // Only the word on the last line is present, so the first line has to be + // ruled out and the last one still found. + $long = $this->get_long_submission( 'a plugin for you' ); + $this->assertTrue( $this->check_values_with( $denylist, $long ) ); + + // Neither word present, and the blank line must not match. + $clean = $this->get_long_submission( 'nothing to see' ); + $this->assertFalse( $this->check_values_with( $denylist, $clean ) ); + } + + /** + * A regex denylist still matches on a long submission, where the index would + * have wrongly ruled the pattern out had it been applied. + */ + public function test_regex_denylist_matches_on_an_indexed_length_submission() { + $long = $this->get_long_submission( 'buy prada handbags' ); + + $this->assertTrue( + $this->check_values_with( + array( + array( + 'words' => array( 'prada\s+handbags' ), + 'is_regex' => true, + ), + ), + $long + ) + ); + } + + /** + * Lines shorter than the index key cannot be looked up, so they are compared + * rather than wrongly ruled out. + */ + public function test_lines_shorter_than_the_index_key_are_still_compared() { + $long = $this->get_long_submission( 'aaa' ); + + $this->assertTrue( $this->check_values_with( array( array( 'words' => array( 'aaa' ) ) ), $long ) ); + $this->assertFalse( $this->check_values_with( array( array( 'words' => array( 'zqx' ) ) ), $long ) ); + } }