From dc7ba949472631d95ef288710a5cdab974eed1c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 05:04:27 +0000 Subject: [PATCH 1/3] fix(ownts): literal-proof useEffect parser + strict listener release match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the merged #144, addressing CodeRabbit's last two Major findings on the OwnTS spike's parser. 1. String/bracket-aware parsing. _expr_end / _effect_callback / _split_cleanup counted raw punctuation, so a string like fetch("/a,b") or a `{` inside a literal truncated the body or split the deps, and the deps regex tore `[items[i]]` at its inner bracket — feeding garbage facts to OWN001/EFF001. Now the source is string-masked once (_mask_strings blanks literal CONTENT, keeping delimiters, length and newlines) and every structural scan runs on the masked copy; the dependency array is matched by balanced brackets with a top-level comma split. Display snippets still come from the original text. 2. addEventListener release now matches the receiver and capture/options, not just the handler: window.addEventListener("x", h, true) is no longer treated as released by removeEventListener("x", h) (option dropped) or by a removeEventListener on a different target — both still leak. Coverage: new fixture EffectHardening.tsx (string commas/braces + nested dep bracket + options-dropped listener -> OWN001 + EFF001), pinned in test_ownts.py alongside direct _is_released discrimination cases, plus a CI step. All existing fixtures unchanged; full suite green (effects 31/31, ownir 194/194, explain); ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi --- .github/workflows/ci.yml | 12 ++ frontend/ownts/examples/EffectHardening.tsx | 31 ++++ frontend/ownts/ownts.py | 155 ++++++++++++++++---- frontend/ownts/test_ownts.py | 25 +++- 4 files changed, 190 insertions(+), 33 deletions(-) create mode 100644 frontend/ownts/examples/EffectHardening.tsx diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79d74be7..2a6cd279 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1076,6 +1076,18 @@ jobs: || { echo "FAIL: expected exactly one OWN001 (the uncleared timer)"; exit 1; } echo "$edges" | grep -q "pollB" \ || { echo "FAIL: the leak must be the second (uncleared) interval"; exit 1; } + - name: Parser hardening — string literals, nested dep brackets, listener options + run: | + python frontend/ownts/ownts.py frontend/ownts/examples/EffectHardening.tsx \ + -o "$RUNNER_TEMP/hard.facts.json" + hard=$(python -m ownlang ownir "$RUNNER_TEMP/hard.facts.json" || true) + echo "$hard" + # a string with commas/braces does not truncate the body (the timer is cleared); + # the leak is the options-dropped listener; the object dep fires EFF001 once + [ "$(echo "$hard" | grep -cE '\[(OWN|EFF|DI)[0-9]{3}\]')" -eq 2 ] \ + || { echo "FAIL: expected one OWN001 (listener) + one EFF001 (object dep)"; exit 1; } + echo "$hard" | grep -q "scroll" \ + || { echo "FAIL: the leak must be the options-dropped scroll listener"; exit 1; } - name: The clean fixture (cleanups + useMemo'd dep) is silent run: | python frontend/ownts/ownts.py frontend/ownts/examples/DashboardClean.tsx \ diff --git a/frontend/ownts/examples/EffectHardening.tsx b/frontend/ownts/examples/EffectHardening.tsx new file mode 100644 index 00000000..60d12b27 --- /dev/null +++ b/frontend/ownts/examples/EffectHardening.tsx @@ -0,0 +1,31 @@ +// Parser-hardening cases (CodeRabbit): string literals must not truncate bodies or +// split deps, deps brackets nest, and a listener is released only by a matching +// target+handler+options removeEventListener. Run: +// python frontend/ownts/ownts.py frontend/ownts/examples/EffectHardening.tsx --check +// Expect exactly: OWN001 (the options-dropped listener) + EFF001 (the object dep). +import { useEffect } from "react"; + +export function Hardening({ id }: { id: string }) { + // A string with commas and braces must not truncate the body or split the deps; + // the timer IS cleared, so this effect leaks nothing. + useEffect(() => { + const t = setInterval(() => fetch("/a,b,{c}"), 1000); + return () => clearInterval(t); + }, [id]); + + // Listener added WITH capture, but cleanup drops the option -> different listener, + // still leaks (one OWN001). + useEffect(() => { + window.addEventListener("scroll", onScroll, true); + return () => window.removeEventListener("scroll", onScroll); + }, []); + + // Nested-bracket dep parses (`items[0]` stays one entry); the object literal dep + // is unstable + IO -> exactly one EFF001 (items[0] is not identifier-like: silent). + const filters = { id }; + useEffect(() => { + fetch("/x"); + }, [filters, items[0]]); + + return
hardening
; +} diff --git a/frontend/ownts/ownts.py b/frontend/ownts/ownts.py index dccdb9f9..7c3f53b7 100644 --- a/frontend/ownts/ownts.py +++ b/frontend/ownts/ownts.py @@ -88,12 +88,26 @@ def _is_released(acq: Acquire, setup: str, pos: int, cleanup: str) -> bool: tok = _lhs_token(setup, pos) return bool(tok and re.search( rf"\b{re.escape(tok)}\s*\.\s*(?:unsubscribe|remove)\s*\(", cleanup)) - if acq.resource == "subscription": # addEventListener(event, handler) - hm = re.search(r"addEventListener\s*\(\s*[^,]+,\s*([A-Za-z_$][\w$.]*)", - setup[pos:]) - handler = hm.group(1) if hm else None - return bool(handler and re.search( - rf"removeEventListener\s*\(\s*[^,]+,\s*{re.escape(handler)}\b", cleanup)) + if acq.resource == "subscription": # target.addEventListener(event, handler[, opts]) + am = re.match( + r"\.addEventListener\s*\(\s*[^,]+,\s*([A-Za-z_$][\w$.]*)\s*(?:,\s*([^)]+?))?\s*\)", + setup[pos:]) + if not am: + return False + handler = am.group(1) + opts = (am.group(2) or "").strip() + # the receiver the listener is attached to (`window`, `el`, `this.ref`) — a + # `removeEventListener` on a DIFFERENT target does not release this one. + rm = re.search(r"([A-Za-z_$][\w$.]*)\s*$", setup[:pos]) + recv = rm.group(1) if rm else "" + # require: same receiver, same handler, and — if this acquire passed a + # capture/options arg — the same options in cleanup (dropping `true` / + # `{capture:true}` is a different listener that still leaks). + recv_pat = rf"{re.escape(recv)}\s*\.\s*" if recv else r"" + pat = rf"{recv_pat}removeEventListener\s*\(\s*[^,]+,\s*{re.escape(handler)}\b" + if opts: + pat += rf"[^)]*{re.escape(opts)}" + return bool(re.search(pat, cleanup)) return False # A React component is a function whose name is Capitalized (the JSX convention). @@ -206,6 +220,68 @@ def _match_block(text: str, open_idx: int) -> int: return len(text) +def _mask_strings(text: str) -> str: + """Blank the CONTENT of string/template literals — keeping the delimiters, the + length, and every newline — so the structural scanners (brace/paren/bracket + depth, top-level commas, the deps array) are never fooled by punctuation inside + a literal: `fetch("/a,b")`, `const s = "{"`, a `;` inside a string. Comments are + already gone (`_strip_comments`). Positions are preserved, so a match found on + the masked copy slices identically out of the original text.""" + out = list(text) + i, n = 0, len(text) + while i < n: + c = text[i] + if c in "\"'`": + i += 1 + while i < n and text[i] != c: + if text[i] == "\\" and i + 1 < n: + for k in (i, i + 1): + if text[k] != "\n": + out[k] = " " + i += 2 + continue + if text[i] != "\n": + out[i] = " " + i += 1 + i += 1 # past the closing delimiter (kept) + else: + i += 1 + return "".join(out) + + +def _match_pair(text: str, i: int, open_c: str, close_c: str) -> int: + """Index just past the `close_c` matching the `open_c` at/after `i`. Assumes + string contents are already masked (so no quote-skipping is needed here).""" + i = text.index(open_c, i) + depth = 0 + while i < len(text): + if text[i] == open_c: + depth += 1 + elif text[i] == close_c: + depth -= 1 + if depth == 0: + return i + 1 + i += 1 + return len(text) + + +def _split_top_commas(s: str) -> list[str]: + """Split on commas at bracket/paren/brace depth 0, so a dependency like + `items[i]` or `f(a, b)` stays one entry instead of being torn at its inner comma.""" + parts: list[str] = [] + depth, start = 0, 0 + for j, c in enumerate(s): + if c in "([{": + depth += 1 + elif c in ")]}": + depth -= 1 + elif c == "," and depth == 0: + parts.append(s[start:j]) + start = j + 1 + parts.append(s[start:]) + return [p.strip() for p in parts if p.strip()] + + def _component_at(text: str, idx: int) -> str: """Name of the React component enclosing position `idx` — the nearest preceding Capitalized function declaration. Falls back to a synthetic name.""" @@ -233,24 +309,35 @@ def _expr_end(text: str, i: int) -> int: return i -def _effect_callback(text: str, after_open: int) -> tuple[str, int, list[str] | None, int]: - """Parse `useEffect(, [deps])` from just past the `(`. Returns +def _effect_callback(masked: str, after_open: int) -> tuple[str, int, list[str] | None, int]: + """Parse `useEffect(, [deps])` from just past the `(`, over the STRING-MASKED + source (so literals never truncate a body or split a dep). Returns (body, body_start, deps, end). Handles BOTH a block `() => { ... }` and an expression `() => fetch(url)` callback — calling `_match_block` blindly on the latter would jump to an unrelated `{` or run off the end. `deps` is None when no - dependency array is present; `body` includes the braces for a block callback.""" - arrow = text.find("=>", after_open) + dependency array is present; the array is matched by BALANCED brackets so a dep + like `items[i]` survives. `body` includes the braces for a block callback.""" + arrow = masked.find("=>", after_open) i = (arrow + 2) if arrow != -1 else after_open - while i < len(text) and text[i] in " \t\r\n": + while i < len(masked) and masked[i] in " \t\r\n": i += 1 - if i < len(text) and text[i] == "{": - end = _match_block(text, i) + if i < len(masked) and masked[i] == "{": + end = _match_block(masked, i) else: - end = _expr_end(text, i) - body = text[i:end] - deps_m = re.search(r"\s*,\s*\[([^\]]*)\]", text[end:end + 200]) - deps = ([d.strip() for d in deps_m.group(1).split(",") if d.strip()] - if deps_m else None) + end = _expr_end(masked, i) + body = masked[i:end] + # the dependency array is the balanced `[ ... ]` after an optional `, ` + deps: list[str] | None = None + j = end + while j < len(masked) and masked[j] in " \t\r\n": + j += 1 + if j < len(masked) and masked[j] == ",": + j += 1 + while j < len(masked) and masked[j] in " \t\r\n": + j += 1 + if j < len(masked) and masked[j] == "[": + close = _match_pair(masked, j, "[", "]") + deps = _split_top_commas(masked[j + 1:close - 1]) return body, i, deps, end @@ -276,18 +363,21 @@ def _split_cleanup(body: str) -> tuple[str, str]: def extract(path: str) -> list[Component]: text = _strip_comments(open(path, encoding="utf-8").read()) + masked = _mask_strings(text) # scan structure on this; show snippets from `text` comps: dict[str, Component] = {} - for eff in _USE_EFFECT.finditer(text): - body, body_start, _deps, _end = _effect_callback(text, eff.end()) + for eff in _USE_EFFECT.finditer(masked): + body, body_start, _deps, _end = _effect_callback(masked, eff.end()) setup, cleanup = _split_cleanup(body) - cname = _component_at(text, eff.start()) + cname = _component_at(masked, eff.start()) comp = comps.setdefault(cname, Component(cname, path)) for acq in ACQUIRES: for hit in acq.pattern.finditer(setup): - line = text.count("\n", 0, body_start + hit.start()) + 1 + abs_pos = body_start + hit.start() + line = masked.count("\n", 0, abs_pos) + 1 released = _is_released(acq, setup, hit.start(), cleanup) - # the acquire expression, trimmed to the call head for a readable tag - snippet = setup[hit.start():].splitlines()[0].strip().rstrip("{").strip() + # the acquire expression for the tag — from the ORIGINAL text, so the + # message shows the real call (string args intact), trimmed to one line. + snippet = text[abs_pos:].splitlines()[0].strip().rstrip("{").strip() comp.resources.append( Resource(snippet or acq.name, line, released, acq.resource, acq.eff)) return [c for c in comps.values() if c.resources] @@ -361,8 +451,10 @@ def _classify_rhs(rhs: str) -> tuple[str, list[str]]: def _render_bindings(text: str) -> dict[str, list[dict]]: - """The render-scope binding table per component: only bindings declared DIRECTLY - in the component body (brace-depth 1). A `const filters = {...}` inside a + """The render-scope binding table per component, scanned over STRING-MASKED + source so a brace/quote inside a literal cannot shift the depth. Only bindings + declared DIRECTLY in the component body (brace-depth 1) count. A `const filters + = {...}` inside a `useEffect` callback, an event handler, or any nested block is NOT render scope — it must not shadow the real outer dependency of the same name and mint a false EFF001. Excluding a render-level `if`/`try` binding only costs a missed finding @@ -393,16 +485,17 @@ def extract_effects(path: str) -> list[dict]: whether its body does network IO, and the render-scope binding table of the component it lives in. The core's effects analysis turns these into a verdict.""" text = _strip_comments(open(path, encoding="utf-8").read()) - binds_by_comp = _render_bindings(text) + masked = _mask_strings(text) + binds_by_comp = _render_bindings(masked) effects: list[dict] = [] - for eff in _USE_EFFECT.finditer(text): - body, _start, deps, _end = _effect_callback(text, eff.end()) + for eff in _USE_EFFECT.finditer(masked): + body, _start, deps, _end = _effect_callback(masked, eff.end()) if deps is None: # no dep array -> not an EFF001 candidate (by-design re-run cadence) continue - cname = _component_at(text, eff.start()) + cname = _component_at(masked, eff.start()) effects.append({ "component": cname, "file": path, - "line": text.count("\n", 0, eff.start()) + 1, + "line": masked.count("\n", 0, eff.start()) + 1, "io": bool(_IO.search(body)), "deps": deps, "bindings": binds_by_comp.get(cname, []), diff --git a/frontend/ownts/test_ownts.py b/frontend/ownts/test_ownts.py index 255d5241..b2b6842e 100644 --- a/frontend/ownts/test_ownts.py +++ b/frontend/ownts/test_ownts.py @@ -51,9 +51,30 @@ def main() -> int: edges = codes("EffectEdges.tsx") assert edges == ["OWN001"], f"EffectEdges -> {edges}" + # Parser hardening: a string with commas/braces does not truncate a body or + # split deps; deps brackets nest (`items[0]`); a listener is released only by a + # matching target+handler+options. -> one OWN001 (options dropped) + one EFF001. + hardening = codes("EffectHardening.tsx") + assert hardening == ["OWN001", "EFF001"], f"EffectHardening -> {hardening}" + + # addEventListener release must match the receiver and capture/options, not just + # the handler (dropping `true` or changing the target still leaks). + sub = next(a for a in ownts.ACQUIRES if a.resource == "subscription") + + def rel(setup: str, cleanup: str) -> bool: + return ownts._is_released(sub, setup, setup.index(".addEventListener"), cleanup) + + assert rel('window.addEventListener("x", onX)', 'window.removeEventListener("x", onX)') + assert rel('window.addEventListener("x", onX, true)', + 'window.removeEventListener("x", onX, true)') + assert not rel('window.addEventListener("x", onX, true)', + 'window.removeEventListener("x", onX)'), "dropped options must still leak" + assert not rel('el.addEventListener("x", onX)', + 'other.removeEventListener("x", onX)'), "wrong target must still leak" + print("OwnTS spike OK: leaky=3xOWN001+EFF001, clean=0, kinds=timer/subscribe/" - "subscription, EffectStorm=2xEFF001, EffectEdges=1xOWN001 (per-resource " - "cleanup + render-scope-only bindings).") + "subscription, EffectStorm=2xEFF001, EffectEdges=1xOWN001, " + "EffectHardening=OWN001+EFF001 (literal-proof parser + strict listener match).") return 0 From 60a54045fdc5804c2b2e3e35b54171b677388aac Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 05:09:18 +0000 Subject: [PATCH 2/3] fix(ownts): match listener removal by capture flag, default false (Codex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit addEventListener's third argument is identified for removal by its capture flag, which defaults to false when omitted. The previous check only required matching options when the acquire HAD a third arg, so add("x", h) (capture false) was wrongly accepted as released by remove("x", h, true) (capture true), suppressing the OWN001 leak. Now _capture_flag normalizes each side (omitted/object-without- capture -> false; true/false; {capture:…}; non-literal -> verbatim) and the listener is released only when receiver, handler AND capture flag all match — in both directions. passive/once/signal don't affect removal identity. test_ownts.py adds the symmetric case (omitted vs remove(...,true) -> leak), {capture:true}==true, and passive-option normalization. Full suite green; ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi --- frontend/ownts/ownts.py | 36 ++++++++++++++++++++++++++++-------- frontend/ownts/test_ownts.py | 9 +++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/frontend/ownts/ownts.py b/frontend/ownts/ownts.py index 7c3f53b7..bd9ff396 100644 --- a/frontend/ownts/ownts.py +++ b/frontend/ownts/ownts.py @@ -75,6 +75,23 @@ def _lhs_token(setup: str, pos: int) -> str | None: return m.group(1) if m else None +def _capture_flag(opts: str) -> str: + """The capture flag an addEventListener/removeEventListener options arg implies — + the only field that identifies a listener for removal. `'true'`/`'false'`, or + `'unknown'` for a non-literal we cannot read. An omitted arg, or an options + object without a `capture` key, defaults to capture **false** (the DOM default); + `passive`/`once`/`signal` do not affect removal identity.""" + o = opts.strip() + if o in ("", "false"): + return "false" + if o == "true": + return "true" + if o.startswith("{"): + m = re.search(r"\bcapture\s*:\s*(true|false)\b", o) + return m.group(1) if m else "false" + return "unknown" # a variable/call — compare verbatim (equal only to itself) + + def _is_released(acq: Acquire, setup: str, pos: int, cleanup: str) -> bool: """Whether THIS acquire (at `pos` in `setup`) is released by the effect's cleanup — matched to its own handle, so two `setInterval`s with one `clearInterval` leave @@ -95,19 +112,22 @@ def _is_released(acq: Acquire, setup: str, pos: int, cleanup: str) -> bool: if not am: return False handler = am.group(1) - opts = (am.group(2) or "").strip() + acq_cap = _capture_flag(am.group(2) or "") # the receiver the listener is attached to (`window`, `el`, `this.ref`) — a # `removeEventListener` on a DIFFERENT target does not release this one. rm = re.search(r"([A-Za-z_$][\w$.]*)\s*$", setup[:pos]) recv = rm.group(1) if rm else "" - # require: same receiver, same handler, and — if this acquire passed a - # capture/options arg — the same options in cleanup (dropping `true` / - # `{capture:true}` is a different listener that still leaks). recv_pat = rf"{re.escape(recv)}\s*\.\s*" if recv else r"" - pat = rf"{recv_pat}removeEventListener\s*\(\s*[^,]+,\s*{re.escape(handler)}\b" - if opts: - pat += rf"[^)]*{re.escape(opts)}" - return bool(re.search(pat, cleanup)) + # released only by a cleanup with the SAME receiver, handler, AND capture + # flag. The capture flag is what identifies a listener for removal; it + # defaults to false when omitted, so add(...,true) is not released by + # remove(...) and add(...) is not released by remove(...,true). + for cm in re.finditer( + rf"{recv_pat}removeEventListener\s*\(\s*[^,]+,\s*" + rf"{re.escape(handler)}\b\s*(?:,\s*([^)]+?))?\s*\)", cleanup): + if _capture_flag(cm.group(1) or "") == acq_cap: + return True + return False return False # A React component is a function whose name is Capitalized (the JSX convention). diff --git a/frontend/ownts/test_ownts.py b/frontend/ownts/test_ownts.py index b2b6842e..99e196f7 100644 --- a/frontend/ownts/test_ownts.py +++ b/frontend/ownts/test_ownts.py @@ -69,6 +69,15 @@ def rel(setup: str, cleanup: str) -> bool: 'window.removeEventListener("x", onX, true)') assert not rel('window.addEventListener("x", onX, true)', 'window.removeEventListener("x", onX)'), "dropped options must still leak" + assert not rel('window.addEventListener("x", onX)', + 'window.removeEventListener("x", onX, true)'), \ + "omitted capture (false) is not released by remove(..., true)" + assert rel('window.addEventListener("x", onX, {capture: true})', + 'window.removeEventListener("x", onX, true)'), \ + "{capture:true} and true are the same listener" + assert rel('window.addEventListener("x", onX, {passive: true})', + 'window.removeEventListener("x", onX)'), \ + "a non-capture option (passive) does not change removal identity" assert not rel('el.addEventListener("x", onX)', 'other.removeEventListener("x", onX)'), "wrong target must still leak" From 943c3fb6715646792a5db80328010dc36fe6bbda Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 05:15:14 +0000 Subject: [PATCH 3/3] fix(ownts): match listeners by full (receiver,event,handler,capture) key, fail closed The listener-release check compared only handler + capture, so a real leak could be suppressed: addEventListener("scroll", h) was "released" by removeEventListener("resize", h) (different event), and an indexed/nested receiver like nodes[i].addEventListener(...) collapsed the receiver to empty so a cleanup on any target matched. Now both the acquire and each cleanup removeEventListener are parsed into the full listener key (receiver, event, handler, capture) and a listener is released only when ALL fields match exactly; otherwise it fails closed (leak). The receiver parser accepts member/index chains (this.ref, nodes[i]). The cleanup scan uses a literal regex + Python comparison (no interpolated pattern), which also clears the ast-grep ReDoS note. To compare event-name string literals, release is now decided on the ORIGINAL (unmasked) setup/cleanup while acquires are still located on the masked copy (_cleanup_span cuts the same span out of both). test_ownts.py adds the different-event and indexed-receiver-mismatch cases. Full suite green; ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi --- frontend/ownts/ownts.py | 103 ++++++++++++++++++++++------------- frontend/ownts/test_ownts.py | 8 +++ 2 files changed, 74 insertions(+), 37 deletions(-) diff --git a/frontend/ownts/ownts.py b/frontend/ownts/ownts.py index bd9ff396..ef0f84c1 100644 --- a/frontend/ownts/ownts.py +++ b/frontend/ownts/ownts.py @@ -92,6 +92,28 @@ def _capture_flag(opts: str) -> str: return "unknown" # a variable/call — compare verbatim (equal only to itself) +_LISTENER = re.compile( + r"\.\s*(?:add|remove)EventListener\s*\(\s*([^,]+?)\s*,\s*([A-Za-z_$][\w$.]*)\s*" + r"(?:,\s*([^)]+?))?\s*\)") + + +def _listener_call(s: str) -> tuple[str, str, str] | None: + """Parse a `.addEventListener`/`.removeEventListener` head into the + (event, handler, capture) triple that identifies the listener for removal, or + None when it doesn't parse. `s` starts at the `.`.""" + m = _LISTENER.match(s.lstrip()) + if not m: + return None + return (m.group(1).strip(), m.group(2), _capture_flag(m.group(3) or "")) + + +def _receiver(text: str, end: int) -> str: + """The receiver expression ending at `end` (just before `.addEventListener`), + allowing a member/index chain like `this.ref` or `nodes[i]`.""" + m = re.search(r"([A-Za-z_$][\w$.\[\]]*)$", text[:end]) + return m.group(1) if m else "" + + def _is_released(acq: Acquire, setup: str, pos: int, cleanup: str) -> bool: """Whether THIS acquire (at `pos` in `setup`) is released by the effect's cleanup — matched to its own handle, so two `setInterval`s with one `clearInterval` leave @@ -106,26 +128,23 @@ def _is_released(acq: Acquire, setup: str, pos: int, cleanup: str) -> bool: return bool(tok and re.search( rf"\b{re.escape(tok)}\s*\.\s*(?:unsubscribe|remove)\s*\(", cleanup)) if acq.resource == "subscription": # target.addEventListener(event, handler[, opts]) - am = re.match( - r"\.addEventListener\s*\(\s*[^,]+,\s*([A-Za-z_$][\w$.]*)\s*(?:,\s*([^)]+?))?\s*\)", - setup[pos:]) - if not am: + # The full listener key is (receiver, event, handler, capture). A listener is + # released ONLY by a removeEventListener whose whole key matches — a different + # event name, target, or capture flag is a different listener that still + # leaks. Fail closed: if the acquire's own target can't be identified, or no + # cleanup call matches every field, treat it as a leak. + a = _listener_call(setup[pos:]) + if a is None: return False - handler = am.group(1) - acq_cap = _capture_flag(am.group(2) or "") - # the receiver the listener is attached to (`window`, `el`, `this.ref`) — a - # `removeEventListener` on a DIFFERENT target does not release this one. - rm = re.search(r"([A-Za-z_$][\w$.]*)\s*$", setup[:pos]) - recv = rm.group(1) if rm else "" - recv_pat = rf"{re.escape(recv)}\s*\.\s*" if recv else r"" - # released only by a cleanup with the SAME receiver, handler, AND capture - # flag. The capture flag is what identifies a listener for removal; it - # defaults to false when omitted, so add(...,true) is not released by - # remove(...) and add(...) is not released by remove(...,true). - for cm in re.finditer( - rf"{recv_pat}removeEventListener\s*\(\s*[^,]+,\s*" - rf"{re.escape(handler)}\b\s*(?:,\s*([^)]+?))?\s*\)", cleanup): - if _capture_flag(cm.group(1) or "") == acq_cap: + a_recv = _receiver(setup, pos) + if not a_recv: + return False + for rm in re.finditer(r"([A-Za-z_$][\w$.\[\]]*)\s*\.\s*removeEventListener\s*\(", + cleanup): + if rm.group(1) != a_recv: + continue + b = _listener_call(cleanup[rm.start() + len(rm.group(1)):]) + if b == a: return True return False return False @@ -361,24 +380,23 @@ def _effect_callback(masked: str, after_open: int) -> tuple[str, int, list[str] return body, i, deps, end -def _split_cleanup(body: str) -> tuple[str, str]: - """Split an effect block body into (setup, cleanup). Cleanup is the block of the - effect's OWN top-level `return () => { ... }` — a `return ... =>` nested inside a - callback (brace-depth > 1) is NOT the effect cleanup and must not suppress a - leak. For an expression-bodied effect there is no cleanup.""" - for m in re.finditer(r"return\s*(?:\(\s*\)|\w+)\s*=>", body): - prefix = body[:m.start()] +def _cleanup_span(mbody: str) -> tuple[int, int] | None: + """The (start, end) span of the effect's OWN top-level cleanup within `mbody` + (the masked body), or None. A `return ... =>` nested inside a callback + (brace-depth > 1) is NOT the effect cleanup and must not suppress a leak. The + span is returned (not slices) so the caller can cut it out of BOTH the masked + body (for acquire scanning) and the original body (for event/string comparison).""" + for m in re.finditer(r"return\s*(?:\(\s*\)|\w+)\s*=>", mbody): + prefix = mbody[:m.start()] if prefix.count("{") - prefix.count("}") != 1: # 1 == the effect body's own brace continue - brace = body.find("{", m.end()) + brace = mbody.find("{", m.end()) if brace == -1: # `return () => clearInterval(id)` — single-expression cleanup, no block. - nl = body.find("\n", m.end()) - tail = body[m.end(): nl if nl != -1 else len(body)] - return body[: m.start()], tail - end = _match_block(body, brace) - return body[: m.start()] + body[end:], body[brace:end] - return body, "" + nl = mbody.find("\n", m.end()) + return (m.start(), nl if nl != -1 else len(mbody)) + return (m.start(), _match_block(mbody, brace)) + return None def extract(path: str) -> list[Component]: @@ -386,15 +404,26 @@ def extract(path: str) -> list[Component]: masked = _mask_strings(text) # scan structure on this; show snippets from `text` comps: dict[str, Component] = {} for eff in _USE_EFFECT.finditer(masked): - body, body_start, _deps, _end = _effect_callback(masked, eff.end()) - setup, cleanup = _split_cleanup(body) + body_m, body_start, _deps, end = _effect_callback(masked, eff.end()) + body_o = text[body_start:end] # original (unmasked) body — same positions + span = _cleanup_span(body_m) + if span: + cs, ce = span + setup_m = body_m[:cs] + body_m[ce:] + setup_o = body_o[:cs] + body_o[ce:] + cleanup_o = body_o[cs:ce] + else: + setup_m, setup_o, cleanup_o = body_m, body_o, "" cname = _component_at(masked, eff.start()) comp = comps.setdefault(cname, Component(cname, path)) for acq in ACQUIRES: - for hit in acq.pattern.finditer(setup): + # find the acquire on the MASKED setup (a keyword inside a string is gone); + # decide release on the ORIGINAL setup/cleanup (so event-name strings and + # handles are intact for the full-key comparison). + for hit in acq.pattern.finditer(setup_m): abs_pos = body_start + hit.start() line = masked.count("\n", 0, abs_pos) + 1 - released = _is_released(acq, setup, hit.start(), cleanup) + released = _is_released(acq, setup_o, hit.start(), cleanup_o) # the acquire expression for the tag — from the ORIGINAL text, so the # message shows the real call (string args intact), trimmed to one line. snippet = text[abs_pos:].splitlines()[0].strip().rstrip("{").strip() diff --git a/frontend/ownts/test_ownts.py b/frontend/ownts/test_ownts.py index 99e196f7..5c50a881 100644 --- a/frontend/ownts/test_ownts.py +++ b/frontend/ownts/test_ownts.py @@ -80,6 +80,14 @@ def rel(setup: str, cleanup: str) -> bool: "a non-capture option (passive) does not change removal identity" assert not rel('el.addEventListener("x", onX)', 'other.removeEventListener("x", onX)'), "wrong target must still leak" + assert not rel('window.addEventListener("scroll", onX)', + 'window.removeEventListener("resize", onX)'), \ + "a different event name is a different listener (still leaks)" + assert rel('nodes[i].addEventListener("x", onX)', + 'nodes[i].removeEventListener("x", onX)'), "indexed receiver matches itself" + assert not rel('nodes[i].addEventListener("x", onX)', + 'nodes[j].removeEventListener("x", onX)'), \ + "a different index is a different target (still leaks)" print("OwnTS spike OK: leaky=3xOWN001+EFF001, clean=0, kinds=timer/subscribe/" "subscription, EffectStorm=2xEFF001, EffectEdges=1xOWN001, "