From 1781271a016fa32caa56ebdd97608b3b4f193f52 Mon Sep 17 00:00:00 2001 From: zogstrip Date: Thu, 20 Nov 2025 10:35:44 +0100 Subject: [PATCH 1/3] FIX: silent failure when watched words contains invalid regex If a watched words list contains any invalid regex, the test modal won't work because the "compiled" regex (that contains all the watched words) will be invalid. This PR introduces a fallback to matching individual watched words when the compiled regex is invalid as well as displaying the error for each invalid regex. Internal ref - t/168206 --- app/assets/stylesheets/admin/staff_logs.scss | 7 + app/services/word_watcher.rb | 16 +- config/locales/client.en.yml | 1 + .../components/modal/watched-word-testing.gjs | 147 +++++++++---- .../controllers/admin-watched-words/action.js | 30 ++- .../templates/admin-watched-words/action.gjs | 14 +- .../acceptance/admin-watched-words-test.js | 204 ++++++++++++++++++ spec/services/word_watcher_spec.rb | 36 ++++ 8 files changed, 405 insertions(+), 50 deletions(-) diff --git a/app/assets/stylesheets/admin/staff_logs.scss b/app/assets/stylesheets/admin/staff_logs.scss index 3616157784035..c927c8416c933 100644 --- a/app/assets/stylesheets/admin/staff_logs.scss +++ b/app/assets/stylesheets/admin/staff_logs.scss @@ -307,6 +307,13 @@ table.screened-ip-addresses { } // Watched words +.watched-word-regex-errors { + max-height: 200px; + overflow-y: auto; + list-style-type: disc; + padding-left: 20px; +} + .watched-word-box { display: inline-block; width: 250px; diff --git a/app/services/word_watcher.rb b/app/services/word_watcher.rb index ab453bce8a7b9..119c8e71da2c1 100644 --- a/app/services/word_watcher.rb +++ b/app/services/word_watcher.rb @@ -79,8 +79,12 @@ def self.compiled_regexps_for_action(action, engine: :ruby, raise_errors: false) r = word_to_regexp(word, match_word: SiteSetting.watched_words_regular_expressions?) begin r if Regexp.new(r) - rescue RegexpError + rescue RegexpError => e raise if raise_errors + Rails.logger.warn( + "Watched word '#{word}' has invalid regex '#{r}' for #{action}: #{e.message}", + ) + nil end end .select { |r| r.present? } @@ -96,7 +100,15 @@ def self.compiled_regexps_for_action(action, engine: :ruby, raise_errors: false) ) if !SiteSetting.watched_words_regular_expressions? # Add case insensitive flag if needed - Regexp.new(regexp, group_key == :case_sensitive ? nil : Regexp::IGNORECASE) + begin + Regexp.new(regexp, group_key == :case_sensitive ? nil : Regexp::IGNORECASE) + rescue RegexpError => e + raise if raise_errors + Rails.logger.warn( + "Watched word compilation failed for #{action} (#{group_key}): #{e.message}. Regexp: #{regexp}", + ) + nil + end end .compact end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 89c0df13a6396..c18873d0c1d85 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -7750,6 +7750,7 @@ en: clear_all: Clear all clear_all_confirm: "Are you sure you want to clear all watched words for the %{action} action?" invalid_regex: 'The watched word "%{word}" is an invalid regular expression.' + invalid_regex_multiple: "Invalid Regular Expressions:" regex_warning: 'Watched words are regular expressions and they do not automatically include word boundaries. If you want the regular expression to match whole words, include \b at the start and end of your regular expression.' actions: block: "Block" diff --git a/frontend/discourse/admin/components/modal/watched-word-testing.gjs b/frontend/discourse/admin/components/modal/watched-word-testing.gjs index 51467574aa32d..52d24b2042dad 100644 --- a/frontend/discourse/admin/components/modal/watched-word-testing.gjs +++ b/frontend/discourse/admin/components/modal/watched-word-testing.gjs @@ -1,5 +1,5 @@ import Component from "@glimmer/component"; -import { tracked } from "@glimmer/tracking"; +import { cached, tracked } from "@glimmer/tracking"; import { Textarea } from "@ember/component"; import DModal from "discourse/components/d-modal"; import { or } from "discourse/truth-helpers"; @@ -20,70 +20,135 @@ export default class WatchedWordTesting extends Component { return this.args.model.watchedWord.nameKey === "link"; } - get matches() { - if ( - !this.value || - this.args.model.watchedWord.compiledRegularExpression.length === 0 - ) { - return []; + cleanErrorMessage(message) { + const parts = message.split(": "); + return parts[parts.length - 1]; + } + + @cached + get matchesAndErrors() { + const errors = []; + + if (!this.value) { + return { matches: [], errors: [] }; } if (this.isReplace || this.isLink) { const matches = []; this.args.model.watchedWord.words.forEach((word) => { - const regexp = new RegExp( - word.regexp, - word.case_sensitive ? "gu" : "gui" - ); - let match; - - while ((match = regexp.exec(this.value)) !== null) { - matches.push({ - match: match[1], - replacement: word.replacement, + try { + const regexp = new RegExp( + word.regexp, + word.case_sensitive ? "gu" : "gui" + ); + let match; + + while ((match = regexp.exec(this.value)) !== null) { + matches.push({ + match: match[1], + replacement: word.replacement, + }); + } + } catch (e) { + errors.push({ + word: word.word, + error: this.cleanErrorMessage(e.message), }); } }); - return matches; + return { matches, errors }; } if (this.isTag) { const matches = new Map(); this.args.model.watchedWord.words.forEach((word) => { - const regexp = new RegExp( - word.regexp, - word.case_sensitive ? "gu" : "gui" - ); - let match; - - while ((match = regexp.exec(this.value)) !== null) { - if (!matches.has(match[1])) { - matches.set(match[1], new Set()); + try { + const regexp = new RegExp( + word.regexp, + word.case_sensitive ? "gu" : "gui" + ); + let match; + + while ((match = regexp.exec(this.value)) !== null) { + if (!matches.has(match[1])) { + matches.set(match[1], new Set()); + } + + const tags = matches.get(match[1]); + word.replacement.split(",").forEach((tag) => tags.add(tag)); } - - const tags = matches.get(match[1]); - word.replacement.split(",").forEach((tag) => tags.add(tag)); + } catch (e) { + errors.push({ + word: word.word, + error: this.cleanErrorMessage(e.message), + }); } }); - return Array.from(matches, ([match, tagsSet]) => ({ - match, - tags: Array.from(tagsSet), - })); + return { + matches: Array.from(matches, ([match, tagsSet]) => ({ + match, + tags: Array.from(tagsSet), + })), + errors, + }; } let matches = []; + let hasCompiledExpressionError = false; + this.args.model.watchedWord.compiledRegularExpression.forEach((entry) => { - const [regexp, options] = Object.entries(entry)[0]; - const wordRegexp = new RegExp( - regexp, - options.case_sensitive ? "gu" : "gui" - ); + try { + const [regexp, options] = Object.entries(entry)[0]; + const wordRegexp = new RegExp( + regexp, + options.case_sensitive ? "gu" : "gui" + ); - matches.push(...(this.value.match(wordRegexp) || [])); + matches.push(...(this.value.match(wordRegexp) || [])); + } catch { + hasCompiledExpressionError = true; + } }); - return matches; + if (hasCompiledExpressionError) { + matches = []; + this.args.model.watchedWord.words.forEach((word) => { + try { + const regexp = new RegExp( + word.regexp, + word.case_sensitive ? "gu" : "gui" + ); + let match; + + while ((match = regexp.exec(this.value)) !== null) { + matches.push(match[1] || match[0]); + } + } catch (e) { + const cleanError = this.cleanErrorMessage(e.message); + if ( + !errors.some( + (err) => err.word === word.word && err.error === cleanError + ) + ) { + errors.push({ + word: word.word, + error: cleanError, + }); + } + } + }); + } + + return { matches, errors }; + } + + get matches() { + return this.matchesAndErrors.matches; + } + + get regexErrors() { + return this.matchesAndErrors.errors; }