From 45e9cf589deefaeec5828b323dc9f909cd648a32 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 28 Sep 2026 23:59:17 +0200 Subject: [PATCH] fix(matchers): refuse regexes that fail only at the end of a run The slow-regex guard timed a pattern only on repeated runs it matches, so a repeat inside a repeat, which blows up when the run is followed by something that makes the match fail, walked through. Measured on 0.7.0: ^(\w+\s?)+$ spent 558 ms on one process name, and ^([\d:]+)+$ in the destination field 2.7 s per packet on a real IPv6 address. A pattern near the budget was refused 20 times in 40, so the form could accept a text that Apply then refused. - The ladder also tries every run followed by a character no probe alphabet contains; all of the above are refused within 20 characters. - re.error, OverflowError and RecursionError from re.compile all become the ordinary "not a valid regular expression" ValueError. The last two escaped every caller: --dry-run exited 1 with a traceback, the expression tester raised and the Control page raised per keystroke. - Accepted patterns are kept (lru_cache keeps nothing that raised), so validation and apply give the same answer; a refusal is judged again. - At apply, a destination, block or target expression that cannot be read leaves the field as it was instead of switching it off, which for a destination or a target meant impairing all traffic. Three tests that pinned the old behaviour are rewritten on purpose; new tests and six mutation entries guard the change. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 +++ README.md | 3 + beantester/matchers.py | 124 +++++++++++++++++-------- beantester/nettools/exprtest.py | 11 ++- beantester/settings.py | 29 +++--- lang/en.json | 2 +- lang/pl.json | 2 +- lang/zh.json | 2 +- tests/test_cli_runtime.py | 3 + tests/test_matchers.py | 114 ++++++++++++++++++++--- tests/test_mutation_registry.py | 55 +++++++++++ tests/test_nettools_exprtest.py | 9 ++ tests/test_processes.py | 20 +++- tests/test_settings_config_scenario.py | 38 +++++++- 14 files changed, 345 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6d3c6c1..5712b205 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ### Fixed +- **Patterns with a repeat inside a repeat are refused as too slow.** Patterns such as + `re:^(\w+)+$` used to pass the speed check, then took seconds on a single name or + address and could stall the network. They are now refused when you type them. A + pattern that cannot be built at all, such as `re:a{99999999999}`, is reported as an + invalid expression, and the command line exits with the configuration error code. + +- **An expression that cannot be read no longer switches its field off during a + session.** When a destination, block or target expression could not be read as + settings were applied, the field was switched off, and a destination or target + switched off meant all traffic was impaired. The field now keeps its previous value, + and the log says so. + - **Editing "Target process" during a session changes nothing until you click "Apply changes".** The field used to reach the running session on its own. Clearing it to type a new name, or stopping halfway through an expression such as `re:^fire(`, switched diff --git a/README.md b/README.md index 2c6fa025..ba12dc65 100644 --- a/README.md +++ b/README.md @@ -649,6 +649,9 @@ re:^ch.{1,8}e\.exe$ WRONG - it is split into "re:^ch.{1" and "8}e\.exe * **`2000-1000` is an error** (reversed range), not an empty set. * **A wildcard is not a regex.** In `chrome*` the star means "any run". In `re:chrome*` it means "the letter `e` repeated 0+ times". If you write `re:`, you write a regex. +* **A repeat inside a repeat is refused as too slow**, for example `re:^(\w+)+$` or + `re:^([0-9:]+)+$`. Such a pattern can take seconds on a single name or address that almost + matches, and an address is checked on every packet. Write the repeat once: `re:^\w+$`. Every syntax error is reported **immediately**: in the GUI the field turns red with the reason beneath it (in the UI language), and the CLI ends with a readable `error: ...` - never a silent diff --git a/beantester/matchers.py b/beantester/matchers.py index eb391ec7..06a49611 100644 --- a/beantester/matchers.py +++ b/beantester/matchers.py @@ -42,6 +42,7 @@ the GUI can show it and the CLI can turn it into a clean error message. """ import fnmatch +import functools import ipaddress import re import time @@ -521,9 +522,13 @@ def _compare_predicate(op, number): # The margin between "fine" and "not fine" is four orders of magnitude, which is # why a clock is a fair judge here and why this does not flake on a slow runner. # -# 🔴 The textbook example does NOT work: `^(a+)+$` is 0.001 ms at every length, -# because CPython optimises it away. A guard tested with it would prove nothing -# and look thorough. The two patterns above are the ones that actually blow up. +# 🔴 The textbook example is a trap in BOTH directions. `^(a+)+$` is 0.001 ms at +# every length on a run of `a` it MATCHES - the first way through succeeds - and +# this comment once concluded from that that it was harmless. It is not: it blows +# up when the run is followed by something the pattern refuses (refused at 20 +# characters once `_REGEX_PROBE_TAILS` adds that, measured 2026-09-28). The ladder +# of matching runs let through every pattern of that shape, `^(\w+\s?)+$` among +# them, which spent 558 ms on ONE real process name. # The budget covers the WHOLE trial, not each search in it, which is what keeps # the cost of a refusal small: the ladder stops at the first length that has spent # it, so a pattern is refused after roughly one growth step rather than after @@ -542,11 +547,23 @@ def _compare_predicate(op, number): # IPv4 (digits), process names (letters), IPv6 text (hex and colons, the shape the # reproduction used), dotted names. _REGEX_PROBE_UNITS = ("1", "a", "1:", "a.") +# Every run is tried as it is AND followed by a character none of the alphabets +# above contains. A repeat inside a repeat explodes when a long run it can consume +# is followed by something that makes the whole match fail, and a bare run always +# matches - so the ladder walked straight past `(a+)+$`, `^(\w+\s?)+$` and +# `^([\d:]+)+$`, the last one on the packet path at 2.7 s per packet on a real +# IPv6 address. MEASURED 2026-09-28 with the tail: all three refused by 20 +# characters; the whole ladder for eleven ordinary patterns went from +# 0.013-0.025 ms to 0.021-0.044 ms; and a pattern near the budget that was refused +# 20 times in 40 is now refused 40 in 40. +# Known limit, said rather than hidden: a repeat that explodes only on ONE letter +# no unit contains (`(x+x+)+y`) still passes. That is not written by accident. +_REGEX_PROBE_TAILS = ("", "!") # Up to 45, the longest IPv6 address in text form, which is the longest value an -# IP matcher is ever handed. A process name can be longer, and that is said out -# loud rather than covered badly: a pattern that is still fast at 45 characters -# and slow at 300 exists, and the capture-thread heartbeat in `engine.py` is what -# catches it. +# IP matcher is ever handed - and addresses and ports are the only values matched +# per packet. A process pattern runs on process NAMES (the resolver, the UI +# thread), and a name can be longer: the shapes that still pass the tail probe +# were measured at no more than ~1 ms per search at 256 characters (2026-09-28). # # Close steps at the bottom on purpose. The cost of a refusal is whatever the # first over-budget length cost, so the rungs have to be near each other exactly @@ -554,62 +571,89 @@ def _compare_predicate(op, number): # at 12, and a ladder that stepped straight from 8 to 12 would pay the second # number to learn what the first already showed. _REGEX_PROBE_LENGTHS = (6, 8, 10, 12, 14, 16, 20, 24, 32, 45) +# The ladder itself, built once. Lengths outer, alphabets inner, tails innermost: +# the ladder climbs for every alphabet at once, so a pattern that explodes on +# letters but not on digits is caught at the shortest length that shows it rather +# than after a full pass over the other. +_REGEX_PROBES = tuple((unit * length)[:length] + tail + for length in _REGEX_PROBE_LENGTHS + for unit in _REGEX_PROBE_UNITS + for tail in _REGEX_PROBE_TAILS) def _blows_the_budget(rx): - """True when climbing the ladder spends more than the budget. - - Lengths outer, alphabets inner: the ladder climbs for every alphabet at once, - so a pattern that explodes on letters but not on digits is caught at the - shortest length that shows it rather than after a full pass over the other. - """ + """True when climbing the ladder spends more than the budget.""" deadline = time.perf_counter() + REGEX_BUDGET_S - for length in _REGEX_PROBE_LENGTHS: - for unit in _REGEX_PROBE_UNITS: - rx.search((unit * length)[:length]) - if time.perf_counter() > deadline: - return True + for probe in _REGEX_PROBES: + rx.search(probe) + if time.perf_counter() > deadline: + return True return False -def _refuse_if_too_slow(rx, field, term): - """Raise when a compiled pattern is too slow to sit on the packet path. +class _Refused(Exception): + """A pattern that will not be used, and the ``errors.*`` key that says why.""" - TWICE, and the second run is the one that decides, because a wall clock cannot - tell "this pattern burned five milliseconds" from "this thread lost the CPU for - five milliseconds". The obvious answer to that is a CPU clock, and it does not - work here: `time.get_clock_info("thread_time")` REPORTS a resolution of 1e-07 - on this platform and MEASURES 15.625 ms (200 000 reads returned six distinct - values, 2026-09-02), which cannot see a 5 ms budget at all. A second run can: - a scheduling hiccup does not repeat in the same place, and backtracking does, - every time, deterministically. A good pattern never pays for this - it takes - the first run only, at 0.04 ms. - """ - if _blows_the_budget(rx) and _blows_the_budget(rx): - raise _err("errors.filter_regex_too_slow", field, term) + def __init__(self, key): + super().__init__(key) + self.key = key -def _compile_regex(pattern, field, term): - pattern = pattern.strip() - if not pattern: - raise _err("errors.bad_filter_regex", field, term) +@functools.lru_cache(maxsize=256) +def _accepted_regex(pattern): + """The compiled pattern, once it has been judged fit to run per packet. + + CACHED, and only what is accepted: ``lru_cache`` keeps nothing for a call that + raises. The judgement is a wall clock, so the same text could pass when the + form validated it and fail a moment later when "Apply" compiled it again - a + field that validated and then did not apply (external review, P3-13). Once + accepted, a pattern stays accepted for the life of the process, which also + spares the probe to every later apply, scenario step and keystroke; a refused + one is judged afresh each time, so one unlucky run does not stick. + + The probe runs TWICE before refusing, and the second run decides, because a + wall clock cannot tell "this pattern burned five milliseconds" from "this + thread lost the CPU for five milliseconds". The obvious answer to that is a CPU + clock, and it does not work here: `time.get_clock_info("thread_time")` REPORTS + a resolution of 1e-07 on this platform and MEASURES 15.625 ms (200 000 reads + returned six distinct values, 2026-09-02), which cannot see a 5 ms budget at + all. A second run can: a scheduling hiccup does not repeat in the same place, + and backtracking does, every time, deterministically. A good pattern never + pays for this - it takes the first run only, at 0.04 ms. + """ try: # A user pattern like "[a-z[0-9]]" makes `re` emit a FutureWarning ("possible # nested set"). It is not an error and the pattern still compiles - but the # warning goes to stderr, which in a windowed build DOES NOT EXIST, and in the # CLI lands in the middle of the log channel. Either way it is noise the user # can do nothing about, so it is swallowed here; a pattern that is genuinely - # broken still raises re.error below. + # broken still raises below. with warnings.catch_warnings(): warnings.simplefilter("ignore", FutureWarning) warnings.simplefilter("ignore", DeprecationWarning) rx = re.compile(pattern, re.IGNORECASE) - except re.error as exc: - raise _err("errors.bad_filter_regex", field, term) from exc - _refuse_if_too_slow(rx, field, term) + # Not only re.error: `a{99999999999}` raises OverflowError and a few thousand + # nested groups RecursionError, and neither is a ValueError - the one thing every + # caller catches. MEASURED 2026-09-28: `--dry-run` exited 1 instead of CONFIG, the + # expression tester raised although it promises it never does, and the Control + # page raised on every keystroke. This is the single place all of them pass. + except (re.error, OverflowError, RecursionError) as exc: + raise _Refused("errors.bad_filter_regex") from exc + if _blows_the_budget(rx) and _blows_the_budget(rx): + raise _Refused("errors.filter_regex_too_slow") return rx +def _compile_regex(pattern, field, term): + pattern = pattern.strip() + if not pattern: + raise _err("errors.bad_filter_regex", field, term) + try: + return _accepted_regex(pattern) + except _Refused as refused: + raise _err(refused.key, field, term) from refused + + def _is_glob(body): return "*" in body or "?" in body diff --git a/beantester/nettools/exprtest.py b/beantester/nettools/exprtest.py index 58a4897d..9d64fc06 100644 --- a/beantester/nettools/exprtest.py +++ b/beantester/nettools/exprtest.py @@ -30,11 +30,12 @@ BAD_EXPRESSION = "bad_expression" # The longest test value taken. The expression parser vets a regular expression by -# timing it on probes of up to 45 characters (`matchers._REGEX_PROBE_LENGTHS`), and -# runs on the UI thread, as the Control page's live validation does - so a value far -# longer than anything the vetting saw is refused rather than run through a -# pattern nobody has timed on it. 256 is far past any process name this tool has -# met; an address or a port never gets near it. +# timing it on probes of up to 45 characters (`matchers._REGEX_PROBE_LENGTHS`), each +# also followed by a character it cannot consume, and this runs on the UI thread. +# Not cut down to 45: MEASURED 2026-09-28, the shapes that still pass that vetting +# cost at most ~1 ms per search at 256 characters (`(\w|\d)+z`), while 45 would +# refuse long process names the tester exists to try. 256 is far past any process +# name this tool has met; an address or a port never gets near it. MAX_VALUE_CHARS = 256 # ASCII digits, not ``str.isdigit()``: MEASURED 2026-09-23, for "443" written in diff --git a/beantester/settings.py b/beantester/settings.py index c118f5cc..227641c5 100644 --- a/beantester/settings.py +++ b/beantester/settings.py @@ -440,6 +440,8 @@ def apply_targeting(engine, target, log=lambda *_: None, announce=True): Returns the live :class:`~beantester.targeting.ProcessTargeting` (iterable, ``len()``-able), or ``None`` when targeting is off / could not be resolved. + An expression that cannot be read changes nothing: the engine keeps the target + it had, and that is what is returned. The object keeps re-resolving itself while the session runs, so a connection the target opens a second from now is impaired too - the old code handed the engine a frozen set of ports and everything opened afterwards escaped it. @@ -453,9 +455,11 @@ def apply_targeting(engine, target, log=lambda *_: None, announce=True): try: matcher = parse_matcher(expr, KIND_PROCESS, TARGET_FIELD) except ValueError as e: + # Left as it was, like the destination in apply_settings: switching + # targeting off here means impairing every connection in the filter, + # which is the widest answer to an expression that could not be read. log(f"{T('log.targeting_error')}: {e}") - engine.set_target(False) - return None + return engine.targeting() if matcher.is_empty: engine.set_target(False) return None @@ -608,11 +612,14 @@ def apply_settings(engine, s, log=lambda *_: None): try: dest = (bool(dst_ip or dst_port), *compile_endpoint(dst_ip, dst_port)) except ValueError as e: - # Tolerant like the schedule below: a bad expression disables destination - # targeting instead of killing a scenario thread. The GUI and the CLI - # validate up front (validate_settings), so a user never reaches this. + # Tolerant like the schedule below - a scenario thread must not die of it + # - but NEVER by switching the destination off: off means "impair + # everything", so a field that could not be read widened the session to + # the whole machine (external review, P3-13). It is left as it was. The + # GUI and the CLI validate up front, and an accepted regex stays accepted + # (matchers._accepted_regex), so a user does not reach this. log(f"{T('log.filter_skipped')}: {e}") - dest = (False, *compile_endpoint(None, None)) + dest = None # The same shape as the pair below, and said for the same reason: two "only" # switches that exclude each other leave nothing to aim at, the symptom is a # session that changes nothing, and that looks like a broken tool rather than @@ -636,15 +643,14 @@ def apply_settings(engine, s, log=lambda *_: None): log(T("log.asym_one_way_filter")) block_ip = setting_expression("block_ip", g("block_ip")) block_port = setting_expression("block_port", g("block_port")) + block = None # None = leave the block alone try: block = (bool(block_ip or block_port), *compile_endpoint(block_ip, block_port), bool(g("block_reject"))) except ValueError as e: - # Tolerant like destination above: a bad expression disables blocking - # instead of killing a scenario thread. GUI and CLI validate up front. - # The mode goes with it: with no block there is nothing to refuse. + # Tolerant like destination above, and by the same rule: a field that could + # not be read is left as it was, mode included, rather than guessed at. log(f"{T('log.filter_skipped')}: {e}") - block = (False, *compile_endpoint(None, None), False) try: schedule = parse_schedule(g("rate_schedule")) except ValueError as e: @@ -668,7 +674,8 @@ def apply_settings(engine, s, log=lambda *_: None): engine.set_ip_family(bool(g("ipv4_only")), bool(g("ipv6_only"))) engine.set_lan(bool(g("lan_mode"))) engine.set_internet_only(bool(g("internet_only"))) - engine.set_block(*block) + if block is not None: + engine.set_block(*block) engine.set_advanced(g("syn_drop"), g("max_size")) engine.set_spike(g("spike_prob"), g("spike_ms")) engine.set_nat(g("nat_timeout")) diff --git a/lang/en.json b/lang/en.json index 802c7a8a..10fc2bd4 100644 --- a/lang/en.json +++ b/lang/en.json @@ -296,7 +296,7 @@ "log.duration_reached": "Time limit reached ({v} s) - session stopped.", "log.engine_fault": "Engine fault: {e} - the session was stopped, your network is back to normal.", "log.error": "Error", - "log.filter_skipped": "This expression could not be read, so it was switched off for this session", + "log.filter_skipped": "This expression could not be read, so this field was left as it was", "log.ipv4_and_ipv6_only": "IPv4 addresses only and IPv6 addresses only are both on. No packet is both, so nothing will be impaired.", "log.lan_and_internet_only": "LAN mode and Internet only are both on - nothing but loopback gets through.", "log.layout_reset": "Window layout reset.", diff --git a/lang/pl.json b/lang/pl.json index 01ca88a3..b4f5da87 100644 --- a/lang/pl.json +++ b/lang/pl.json @@ -296,7 +296,7 @@ "log.duration_reached": "Osiągnięto limit czasu ({v} s) - sesja zatrzymana.", "log.engine_fault": "Awaria silnika: {e} - sesja została zatrzymana, sieć działa normalnie.", "log.error": "Błąd", - "log.filter_skipped": "Nie udało się odczytać tego wyrażenia, więc zostało wyłączone na tę sesję", + "log.filter_skipped": "Nie udało się odczytać tego wyrażenia, więc to pole zostało bez zmian", "log.ipv4_and_ipv6_only": "Włączone są naraz Tylko adresy IPv4 i Tylko adresy IPv6. Żaden pakiet nie jest jednym i drugim, więc nic nie zostanie zmienione.", "log.lan_and_internet_only": "Tryb LAN i Tylko internet są włączone naraz - poza loopbackiem nic nie przejdzie.", "log.layout_reset": "Układ okna zresetowany.", diff --git a/lang/zh.json b/lang/zh.json index 7651d922..69273b82 100644 --- a/lang/zh.json +++ b/lang/zh.json @@ -296,7 +296,7 @@ "log.duration_reached": "已达到时间限制（{v} 秒），会话已停止。", "log.engine_fault": "引擎故障：{e}。会话已停止，网络已恢复正常。", "log.error": "错误", - "log.filter_skipped": "无法解析此表达式，本次会话已将其关闭", + "log.filter_skipped": "无法解析此表达式，因此该字段保持不变", "log.ipv4_and_ipv6_only": "同时启用了“仅 IPv4 地址”和“仅 IPv6 地址”。没有数据包同时属于两者，因此不会有任何改动。", "log.lan_and_internet_only": "“局域网模式”和“仅互联网”同时启用，因此除环回流量外，其他流量都无法通过。", "log.layout_reset": "窗口布局已重置。", diff --git a/tests/test_cli_runtime.py b/tests/test_cli_runtime.py index c94c8245..575f51b9 100644 --- a/tests/test_cli_runtime.py +++ b/tests/test_cli_runtime.py @@ -87,6 +87,9 @@ def test_exit_code_config_for_bad_input(): cases = { "unknown preset": ["--preset", "nope", "--simulate"], "bad expression": ["--dst-port", "80,abc", "--simulate"], + # OverflowError out of `re` used to escape as exit 1 with a traceback + "regex re cannot build": ["--dst-ip", "re:a{99999999999}", "--simulate"], + "regex too slow per packet": ["--dst-ip", r"re:^([\d:]+)+$", "--simulate"], "bad schedule": ["--rate-schedule", "1:x:2", "--simulate"], "out of range": ["--loss", "250", "--simulate"], "negative duration": ["--duration", "-5", "--simulate"], diff --git a/tests/test_matchers.py b/tests/test_matchers.py index 7d304c5a..76638a10 100644 --- a/tests/test_matchers.py +++ b/tests/test_matchers.py @@ -435,13 +435,13 @@ def test_add_term_keeps_the_comma_escape_of_a_regex(): def test_a_pattern_that_cannot_finish_is_refused_at_parse_time(): - """The two shapes that really explode in CPython, on the kinds that can see them. + """The two shapes that explode even on the runs the ladder feeds them. - 🔴 NOT `^(a+)+$`. The textbook ReDoS example is 0.001 ms at every input length - here, because CPython optimises it away, so a guard written with it would pass - while proving nothing. Measured 2026-09-02, milliseconds at 8 / 12 / 16 / 20 / - 24 characters: `^([1:]+)+x$` is 0.016 / 0.132 / 2.076 / 37.4 / 684, and - `^((a*)*)*b$` is 7.6 / 1254. + Measured 2026-09-02, milliseconds at 8 / 12 / 16 / 20 / 24 characters: + `^([1:]+)+x$` is 0.016 / 0.132 / 2.076 / 37.4 / 684, and `^((a*)*)*b$` is + 7.6 / 1254. This docstring used to add "NOT `^(a+)+$`, CPython optimises it + away" - true only on a run it matches. With a character after the run it + explodes too; see the next test. """ for text, kind, field in ((r"re:^([1:]+)+x$", KIND_IP, "fields.ip"), (r"re:^((a*)*)*b$", KIND_PROCESS, "fields.target")): @@ -449,8 +449,8 @@ def test_a_pattern_that_cannot_finish_is_refused_at_parse_time(): parse_matcher(text, kind, field) # A ValueError like every other parse error, because every caller already # handles that one: the CLI turns it into exit code CONFIG, the form marks - # the field red, `apply_settings` logs it and disables the target rather - # than killing a scenario thread. + # the field red, `apply_settings` logs it and leaves the field as it was + # rather than killing a scenario thread. check(f"{text}: says it is too SLOW, not that it is malformed", "too slow" in str(caught.value).lower() or "za wolne" in str(caught.value).lower(), @@ -462,10 +462,17 @@ def test_the_patterns_a_person_would_actually_write_are_still_accepted(): Every one of these is heavier than average and none of them backtracks. Measured 2026-09-02, worst SINGLE search over the whole probe ladder: 0.0046 ms - for the alternation, 0.0036 ms for the character class, 0.0049 ms for the - nested groups. The budget is 5 ms, so the gap between "fine" and "not fine" is - three orders of magnitude - which is why a wall clock is a fair judge here and - why this does not flake on a loaded runner. + for the alternation, 0.0036 ms for the character class. The budget is 5 ms, so + the gap between "fine" and "not fine" is three orders of magnitude - which is + why a wall clock is a fair judge here and why this does not flake on a loaded + runner. Re-measured 2026-09-28 with the probe tail: the WHOLE ladder costs + these 0.021-0.044 ms. + + 🔴 `^((a|b|c)+(d|e)*)+$` stood on this list as "nested groups, fine" and was + the proof of the ladder's blind spot, not of its accuracy: a repeat inside a + repeat over the same letters, harmless only on runs it matches. It is refused + now and listed with the others in the next test; the unambiguous form + `^((a|b|c)(d|e)*)+$` takes its place here. """ # The repeat counts carry `\,`, which is the mini-language's escape and what a # user has to type: an unescaped comma is a TERM SEPARATOR, so `{1,40}` splits @@ -477,9 +484,12 @@ def test_the_patterns_a_person_would_actually_write_are_still_accepted(): (r"re:" + "|".join(f"proc{n}" for n in range(50)), KIND_PROCESS), (r"re:^[a-zA-Z0-9._\-]{1\,40}\.(exe|dll|sys)$", KIND_PROCESS), (r"re:^(?=.*chrome)(?!.*helper).*$", KIND_PROCESS), - (r"re:^((a|b|c)+(d|e)*)+$", KIND_PROCESS), + (r"re:^((a|b|c)(d|e)*)+$", KIND_PROCESS), + (r"re:.*chrome.*helper.*", KIND_PROCESS), + (r"re:^(svchost|chrome|msedge)\.exe$", KIND_PROCESS), (r"re:^10\.0\.\d+", KIND_IP), (r"re:^([0-9a-f]{1\,4}:){7}[0-9a-f]{1\,4}$", KIND_IP), + (r"re:^(fe80|fd[0-9a-f]{2}):", KIND_IP), (r"re:.*", KIND_PROCESS), (r"re:^8(0|443)$", KIND_INT), ] @@ -492,6 +502,84 @@ def test_the_patterns_a_person_would_actually_write_are_still_accepted(): check(f"{text[:40]}: accepted", True) +def test_a_pattern_that_fails_only_at_its_end_is_refused_too(): + """The blind spot of a ladder made only of runs the pattern MATCHES. + + A repeat inside a repeat blows up when a long run it can consume is followed by + something that makes the whole match fail. The ladder fed bare runs, which + these patterns match at once, so every one of them passed. MEASURED 2026-09-28: + `^(\\w+\\s?)+$` spent 558 ms on StartMenuExperienceHost.exe, and + `^([\\d:]+)+$` in the destination field 2.7 s per packet on a real IPv6 + address - on the capture thread, inside `core._lock`. + """ + for text, kind in ((r"re:^(\w+\s?)+$", KIND_PROCESS), + (r"re:(a+)+$", KIND_PROCESS), + (r"re:^([\d:]+)+$", KIND_IP), + (r"re:^(\d+)+$", KIND_INT), + (r"re:^((a|b|c)+(d|e)*)+$", KIND_PROCESS)): + with pytest.raises(ValueError) as caught: + parse_matcher(text, kind, "fields.target") + check(f"{text}: refused as too slow", + "too slow" in str(caught.value).lower() + or "za wolne" in str(caught.value).lower(), + f"({str(caught.value)[:90]!r})") + + +def test_a_pattern_the_parser_cannot_build_is_a_value_error_on_every_kind(): + """`re` raises more than ``re.error``, and every caller catches only ValueError. + + MEASURED 2026-09-28 before the fix: `a{99999999999}` raised OverflowError and a + few thousand nested groups RecursionError, straight out of `parse_matcher` - + `--dry-run` exited 1 instead of CONFIG, the expression tester raised although + it promises never to, and the Control page raised on every keystroke. + """ + from beantester import matchers + deep = "re:" + "(" * 2000 + "a" + ")" * 2000 + for text in ("re:a{99999999999}", deep): + for kind in (KIND_INT, KIND_IP, KIND_PROCESS): + with pytest.raises(ValueError) as caught: + parse_matcher(text, kind, "fields.target") + said = matchers._err("errors.bad_filter_regex", "fields.target", text) + check(f"{text[:20]} ({kind}): the ordinary 'not a valid regular " + "expression' error", str(caught.value) == str(said), + f"({str(caught.value)[:90]!r})") + + +def test_an_accepted_pattern_is_not_judged_again(monkeypatch): + """Validated by the form, refused by "Apply" a moment later (report P3-13). + + The judgement is a wall clock, so one text could get two answers - MEASURED: + a pattern near the budget was refused 20 times in 40. Apply compiled it again, + and a refusal there used to switch the field off. Now an accepted pattern stays + accepted for the life of the process, while a refusal is not remembered, so one + unlucky run cannot stick either. + """ + from beantester import matchers + getattr(matchers._accepted_regex, "cache_clear", lambda: None)() + judged = [] + real = matchers._blows_the_budget + monkeypatch.setattr(matchers, "_blows_the_budget", + lambda rx: judged.append(rx.pattern) or real(rx)) + good = r"re:^r4-accepted-once\d+$" + _proc(good) + _proc(good) + check("an accepted pattern is timed once, then remembered", + judged == [r"^r4-accepted-once\d+$"], f"({judged})") + + # From here the judge refuses EVERYTHING. + judged.clear() + monkeypatch.setattr(matchers, "_blows_the_budget", + lambda rx: judged.append(rx.pattern) or True) + _proc(good) + check("what was accepted is still accepted, without a second trial", judged == [], + f"({judged})") + for _ in range(2): + with pytest.raises(ValueError): + _proc(r"re:^r4-refused\d+$") + check("a refusal is judged afresh each time (two runs per trial, two trials)", + judged.count(r"^r4-refused\d+$") == 4, f"({judged})") + + def test_refusing_a_slow_pattern_changes_nothing_about_a_good_one(): """The regression a speed guard can cause: matching quietly narrowing. diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index bec3f46d..47bf6cdf 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -477,6 +477,61 @@ "test": ("test_a_pattern_that_cannot_finish_leaves_the_table_search" "_answering_normally"), }, + { + # Back to a ladder of bare runs, which the pattern matches: every repeat + # inside a repeat that fails only at its end walks through again. + "label": "matchers: the probe forgets the character after the run", + "file": "beantester/matchers.py", + "old": "_REGEX_PROBE_TAILS = (\"\", \"!\")", + "new": "_REGEX_PROBE_TAILS = (\"\",)", + "test": "test_a_pattern_that_fails_only_at_its_end_is_refused_too", + }, + { + # OverflowError and RecursionError escape `parse_matcher` again. + "label": "matchers: only re.error becomes a ValueError again", + "file": "beantester/matchers.py", + "old": " except (re.error, OverflowError, RecursionError) as exc:", + "new": " except re.error as exc:", + "test": "test_a_pattern_the_parser_cannot_build_is_a_value_error_on_every_kind", + }, + { + # Every compile judges again: the form's "yes" and Apply's answer can differ. + "label": "matchers: an accepted pattern is judged again at every compile", + "file": "beantester/matchers.py", + "old": "@functools.lru_cache(maxsize=256)\ndef _accepted_regex(pattern):", + "new": "def _accepted_regex(pattern):", + "test": "test_an_accepted_pattern_is_not_judged_again", + }, + { + # The shipped tolerance: a destination that could not be read is switched + # OFF, which impairs everything. + "label": "settings: a bad destination at apply switches it off again", + "file": "beantester/settings.py", + "old": " log(f\"{T('log.filter_skipped')}: {e}\")\n" + " dest = None", + "new": " log(f\"{T('log.filter_skipped')}: {e}\")\n" + " dest = (False, *compile_endpoint(None, None))", + "test": "test_a_bad_destination_at_apply_leaves_the_previous_one_in_place", + }, + { + "label": "settings: a bad block at apply switches blocking off again", + "file": "beantester/settings.py", + "old": " if block is not None:\n" + " engine.set_block(*block)", + "new": " engine.set_block(*(block or (False, *compile_endpoint(None, None)," + " False)))", + "test": "test_a_bad_block_at_apply_leaves_the_previous_one_in_place", + }, + { + # Targeting switched off by an expression that could not be read: every + # connection in the filter impaired. + "label": "settings: a bad target at apply switches targeting off again", + "file": "beantester/settings.py", + "old": " return engine.targeting()", + "new": " engine.set_target(False)\n" + " return None", + "test": "test_apply_targeting_logs_and_keeps_the_target_on_a_bad_expression", + }, { # Back to the check as it stood before 2026-09-02, which is the exact # shape that let NaN through: `float('nan') <= 0` is False. diff --git a/tests/test_nettools_exprtest.py b/tests/test_nettools_exprtest.py index 4a1e331e..c6fb5637 100644 --- a/tests/test_nettools_exprtest.py +++ b/tests/test_nettools_exprtest.py @@ -93,6 +93,15 @@ def test_an_expression_that_cannot_be_read_carries_the_parsers_own_sentence(): assert verdict.canonical == "" and not verdict.selected_by +def test_a_regex_re_cannot_build_is_a_verdict_not_an_exception(): + """The docstring of ``evaluate`` promises it never raises for what a person + types. `re:a{99999999999}` made it raise OverflowError (measured 2026-09-28): + `re` raises more than ``re.error``, and only ValueError was caught.""" + for field_key in ("dst_ip", "dst_port", "target"): + verdict = evaluate(field_key, "re:a{99999999999}", "1") + assert verdict.state == BAD_EXPRESSION, (field_key, verdict) + + def test_nothing_typed_is_its_own_state(): for field, expression in (("dst_ip", "10.0.0.0/8"), ("dst_port", "443"), ("target", "chrome")): diff --git a/tests/test_processes.py b/tests/test_processes.py index 87cba9ad..6ffd8653 100644 --- a/tests/test_processes.py +++ b/tests/test_processes.py @@ -199,14 +199,30 @@ def test_apply_targeting_disables_on_empty_expression(fake_psutil): engine.core.target_active is False) -def test_apply_targeting_logs_and_disables_on_a_bad_expression(fake_psutil): +def test_apply_targeting_logs_and_keeps_the_target_on_a_bad_expression(fake_psutil): + """Rewritten on purpose (external review, P3-13). + + It used to be "a bad expression DISABLES targeting", and targeting off means + every connection in the filter is impaired: the widest possible answer to an + expression that could not be read. The engine now keeps the target it had. + """ engine = BeanEngine() lines = [] apply_targeting(engine, ">chrome", lines.append) - check("a bad expression disables targeting rather than crashing a thread", + check("on a fresh engine a bad expression leaves targeting off, not a crash", engine.core.target_active is False) check("a bad expression is reported in the log", lines, f"({lines})") + apply_targeting(engine, "chrome, !chromedriver", lambda *_: None) + lines.clear() + kept = apply_targeting(engine, ">chrome", lines.append) + check("a bad expression does not switch targeting off", + engine.core.target_active is True + and engine.core.target_ports == {5001, 5002}, f"({engine.core.target_ports})") + check("what is returned is the target still in force", + kept is engine.targeting(), f"({kept!r})") + check("and the problem is still logged", lines, f"({lines})") + # -- make_targeting: the LIVE targeting object used by the engine ------------ # def test_make_targeting_returns_none_for_an_empty_expression(fake_psutil): diff --git a/tests/test_settings_config_scenario.py b/tests/test_settings_config_scenario.py index 42f7486a..0f155a24 100644 --- a/tests/test_settings_config_scenario.py +++ b/tests/test_settings_config_scenario.py @@ -462,25 +462,53 @@ def test_apply_settings_with_expressions(): check("apply: port exclusion matches", not c.dst_port_matcher.matches(53)) -def test_apply_settings_bad_expression_disables_dest_targeting(): +def test_a_bad_destination_at_apply_leaves_the_previous_one_in_place(): + """Rewritten on purpose (external review, P3-13). + + This test used to be "a bad expression DISABLES destination targeting" - and a + destination switched off means impairing every connection, so a field that + could not be read widened the session to the whole machine. It asserted that on + a fresh engine only, where "off" and "left as it was" look the same. + """ from beantester import BeanEngine, apply_settings + from beantester.i18n import T sh = BeanEngine() lines = [] apply_settings(sh, dict(dst_ip="999.1.1.1"), lines.append) - check("apply: a bad expression disables destination targeting", + check("apply: on a fresh engine a bad expression leaves it off", sh.core.dst_active is False) check("apply: the problem is logged, not silently ignored", lines, f"({lines})") - -def test_apply_settings_bad_expression_disables_blocking(): + apply_settings(sh, dict(dst_ip="10.0.0.1")) + lines.clear() + apply_settings(sh, dict(dst_ip="999.1.1.1"), lines.append) + check("apply: a bad expression does not widen the session to everything", + sh.core.dst_active is True) + check("apply: the destination it had is still the one in force", + sh.core.dst_ip_matcher.matches("10.0.0.1") + and not sh.core.dst_ip_matcher.matches("10.0.0.2")) + check("apply: and the log says so", any(T("log.filter_skipped") in line + for line in lines), f"({lines})") + + +def test_a_bad_block_at_apply_leaves_the_previous_one_in_place(): + """Same rule as the destination above: what could not be read is left as it + was, mode included, instead of being switched off or guessed at.""" from beantester import BeanEngine, apply_settings sh = BeanEngine() lines = [] apply_settings(sh, dict(block_ip="999.1.1.1"), lines.append) - check("apply: a bad block expression disables blocking, not a crash", + check("apply: on a fresh engine a bad block leaves blocking off, not a crash", sh.core.block_active is False) check("apply: the block problem is logged", lines, f"({lines})") + apply_settings(sh, dict(block_ip="203.0.113.0/24", block_reject=True)) + apply_settings(sh, dict(block_ip="999.1.1.1", block_reject=False)) + check("apply: a bad block keeps the block it had", + sh.core.block_active is True + and sh.core.block_ip_matcher.matches("203.0.113.9")) + check("apply: and its mode", sh.core.block_reject is True) + def test_apply_settings_with_block_expressions(): from beantester import BeanEngine, apply_settings