cloneNode(true) round-tripped through a throwaway <div>'s innerHTML, so the
HTML fragment parser discarded elements that are not valid children of <div>
(<tr>, <td>, <option>, <tbody>, …) — createElement('tr').cloneNode(true)
returned null — and JS-set inline styles were lost. The shallow path also
re-derived attributes by regex-scraping serialized HTML.
Clone structurally with real DOM APIs: shallow-clone each node via
createElement/createElementNS plus a getAttributeNames() attribute copy (no
parsing context, every attribute preserved), then descend into children with an
explicit stack so a deep subtree cannot overflow the JS stack. Text, Comment,
DocumentFragment and <template> contents are handled.
Fixes#490
The MCP HTTP server serves connections sequentially on the single
current-thread runtime (the browser session is !Send). The SSE branch
(GET /mcp, Accept: text/event-stream) ran an infinite keep-alive ping loop
inline, so handle_connection never returned and the accept loop never ran
again — one open SSE stream hung every later POST /mcp tool call
indefinitely (functional break + trivial unauthenticated DoS).
The keep-alive carries no browser state and its write half is Send + 'static,
so spawn the ping loop on its own task and return from the handler. POST
handling stays serialized (correct for a single browser session), but a live
SSE stream no longer blocks the accept loop.
Fixes#489
parentNode() inspected only the immediate parent, which caused three
spec deviations:
- With currentNode at root, the `parent !== this.root` guard did not
fire (parent is root's OWN parent), so a node OUTSIDE the walker's
subtree was returned — createTreeWalker(root).parentNode() yielded
<body>. A TreeWalker must never surface a node above its root.
- The same guard excluded root as a *result*, so when currentNode's
accepted ancestor was root itself, parentNode() returned null instead
of root.
- A skipped (or otherwise non-accepted) immediate parent ended the
search rather than continuing up to the first accepted ancestor.
Replaced with the DOM 6.1 algorithm: climb from currentNode, returning
the first ancestor that filters to FILTER_ACCEPT, with the loop's
`node !== this.root` guard keeping the walk inside root while still
letting root be returned. Reuses the three-valued `_filter` from #461.
Last of the TreeWalker mover deviations, after nextNode (#461),
previousNode (#462) and the child/sibling movers (#469).
createNodeIterator returned a TreeWalker with a flag on it. The two are
different interfaces, and the alias got two things wrong.
An iterator's pointer starts *before* its root, so the first nextNode()
returns the root itself; a TreeWalker starts *at* its root and never
returns it. So createNodeIterator(el, SHOW_ELEMENT) silently skipped
exactly the element it was rooted at:
<div id=root><a></a></div> -> ["A"] (Chrome: ["DIV","A"])
It also exposed the wrong surface: no referenceNode,
pointerBeforeReferenceNode or detach (older library code still calls
detach and hit "not a function"), while leaking currentNode, firstChild,
parentNode and the rest of TreeWalker's movers.
Implemented DOM 6.2 "traverse" directly. Keeping the pointer as
(referenceNode, pointerBeforeReferenceNode) rather than a single cursor
is what makes a direction change re-yield the current node instead of
stepping over it, and a failed traversal leaves both untouched so the
iterator stays resumable either way.
The backward step is a new `DomTree::prev_in_subtree`, mirroring
`next_in_subtree`. It can live entirely in the DOM layer because a
NodeIterator prunes nothing -- unlike the TreeWalker's backward walk
(#462), where FILTER_REJECT forces the filter to be consulted at every
step of the descent.
FILTER_REJECT still behaves as FILTER_SKIP here, which the existing test
keeps covering; its expectation gains the root node the alias used to
drop. The TreeWalker's now-dead `_rejectIsSkip` branch is removed.
firstChild/lastChild/nextSibling/previousSibling stepped straight to the
next sibling whenever a node was not accepted, so a FILTER_SKIP node hid
its children instead of exposing them:
<div id=root><section><a></a></section></div>, SECTION -> FILTER_SKIP
w.firstChild() // null (Chrome: <a>)
They also used the boolean `_accept`, so FILTER_REJECT (which must not
descend) and FILTER_SKIP (which must) were indistinguishable — the same
root cause #461 fixed for nextNode().
Replaced all four with the spec's "traverse children" and "traverse
siblings" algorithms, parameterised by direction so forward and backward
share one implementation and cannot drift: `edge` is first/last child and
`step` is next/previous sibling. Both consult the three-valued `_filter`
from #461, so SKIP descends, REJECT prunes, and the climb back out stops
at root or the node the walk started from.
parentNode() is left alone here: its deviations are different (it checks
only the immediate parent, and excludes root, which the spec permits it
to return), so it belongs in its own change.
The parser already put template children in a separate contents document
per the HTML spec (tree_sink.rs), but nothing exposed that node, so
`get content()` fabricated an empty DocumentFragment and the parsed
markup was unreachable:
document.body.innerHTML = '<template id=t><p>a</p></template>';
t.content.childNodes.length // 0 (Chrome: 1)
t.innerHTML // "" (Chrome: '<p>a</p>')
Every <template>-driven UI — custom elements cloning
`template.content`, lit, Vue runtime templates, htmx fragments, Alpine
x-if/x-for — stamped nothing, with no error raised.
Three layers were involved:
- `DomTree::template_contents` returns the contents node, allocating one
on demand so templates built via `createElement` work too. Exposed as
the `template_contents` op; `get content()` wraps it as a
DocumentFragment cached by node id, so `.content` keeps a stable
identity across reads.
- `import_node_from` cloned the element data verbatim, carrying the
*source* tree's contents NodeId into the destination tree, where it
indexed whatever unrelated node occupied that slot. That is why a
template injected via innerHTML resolved `.content` to a bogus node
rather than merely an empty one. The contents are now imported into a
fresh node and the reference remapped, queued onto the existing
worklist so nested templates stay iterative.
- The serializer emitted a template's own (always empty) children, so
contents vanished from outerHTML/innerHTML and `cloneNode(true)` —
which round-trips through outer_html — produced an empty template.
`content_source` resolves a template to its contents document for both
inner_html and outer_html.
With serialization fixed on the Rust side, the `set innerHTML` template
branch that routed to `.content` now has a matching getter for free, so
no template special case is needed on the JS hot path.
window.scrollTo/scrollBy/scroll were empty stubs, scrollX/scrollY and
pageXOffset/pageYOffset were hard-coded 0 data properties, no scroll
event was ever fired at the window, and document.scrollingElement did
not exist.
that "scroll-driven lazy loaders advance instead of throwing". But the
dominant infinite-scroll idiom is window-level:
window.scrollTo(0, document.body.scrollHeight);
if (window.scrollY + innerHeight >= body.scrollHeight) loadMore();
all of which was inert -- the call succeeded, the offset never moved, the
sentinel never tripped -- so automation that scrolls a page to trigger
loading silently got one screenful.
The offset is stored on the scrolling element rather than in separate
window state, so window.scrollY and document.scrollingElement.scrollTop
are two views of one value in both directions, which is what pages
assume when they mix the two. scrollX/scrollY/pageXOffset/pageYOffset
become read-only accessors over it, matching a real Window, and
document.scrollingElement returns documentElement (standards mode; we
never parse in quirks mode).
The scroll event is dispatched at both the document and the window.
Chrome propagates a page scroll to both, but Document.dispatchEvent here
runs only its own listeners and does not propagate, so firing once would
strand half the listeners.
As with #431 there is no layout, so the offset still cannot be clamped to
a real maximum; negative values clamp to 0 exactly as they do for
elements.
previousNode() considered exactly one candidate — the previous sibling's
deepest last child — and returned null as soon as the filter rejected it,
so a backward walk died mid-tree. This is the backward mirror of #432,
which was fixed forward-only in #447.
<div id=root><a><b></b></a><c></c></div>, SHOW_ELEMENT, B -> FILTER_SKIP
forward: ["A","C"] backward: [] (Chrome: ["A"])
Replaced with the DOM 6.1 algorithm verbatim: scan previous siblings,
descend to the deepest last descendant of each, then step up to the
parent, continuing until a node is accepted or root is reached. The
descent stops on FILTER_REJECT, which is what makes rejection prune
subtrees in the backward direction too.
Unlike nextNode this stays in JS rather than adding a DOM traversal op:
the descent has to consult the filter at every step to honour REJECT, so
there is no run of JS/native crossings for a native helper to collapse.
Depends on the three-valued `_filter` from #461.
`_accept` collapsed every non-ACCEPT filter result into a single boolean,
so FILTER_REJECT behaved exactly like FILTER_SKIP: the node was not
returned, but the walk still descended into its subtree. Per DOM 6.1,
FILTER_REJECT rejects a node *and* its descendants.
This was unreachable until #440 exposed the real NodeFilter constants,
which made the canonical prune idiom
if (node.tagName === 'SCRIPT') return NodeFilter.FILTER_REJECT;
start yielding nodes from inside the pruned subtree instead of failing
loudly — sanitizers and text extractors built on TreeWalker silently got
extra content.
Split the filter into a three-valued `_filter` (keeping `_accept` as the
boolean wrapper the sibling/child movers use) and give nextNode a second
traversal step for the reject case. That step lives in the DOM layer as
`DomTree::next_after_subtree`, mirroring how #447 put `next_in_subtree`
there: both share the ancestor climb, and REJECT costs no more JS/native
crossings than SKIP.
NodeIterator shares this walker but has no subtree pruning (DOM 6.2:
FILTER_REJECT behaves as FILTER_SKIP), so `createNodeIterator` marks the
walker `_rejectIsSkip` and keeps the plain step. Covered by its own test
so the shared implementation cannot drift.
NodeFilter was defined twice: a partial live definition (SHOW_ELEMENT,
SHOW_TEXT, SHOW_ALL) and a complete one behind an `if (typeof NodeFilter
=== 'undefined')` guard that never fired because the partial one already
defined it. So NodeFilter.FILTER_ACCEPT/REJECT/SKIP and most SHOW_* values
were undefined at runtime, and the canonical filter idiom
`acceptNode() { return NodeFilter.FILTER_ACCEPT; }` returned undefined —
making TreeWalker/NodeIterator reject every node.
Make the live definition authoritative with the full standard constant set
and drop the dead duplicate. Adds tests asserting the constant values and
that a filter returning NodeFilter.FILTER_ACCEPT accepts the node.
CookieJar stored cookies as domain -> name -> entry, so a second
Set-Cookie with the same name but a different path silently overwrote the
first (RFC 6265 §5.3 identifies a cookie by name+domain+path). The
retrieval side already treated path as part of identity — get_cookie_header
path-matches, and delete_cookies_filtered keys on (name, path) — so only
storage lost cookies.
Rekey the inner map by (name, path). A Max-Age=0 / expired Set-Cookie now
deletes exactly the (name, path) it targets instead of every same-name
cookie; delete_cookie(name) still clears all paths (retain by entry name).
Adds tests: same-name/different-path coexistence and no cross-path leak,
same-name/same-path replacement, and path-scoped deletion leaving a
sibling-path cookie intact.
The keyDown/keyUp paths interpolated `key`/`code` into the generated
KeyboardEvent snippet with single-quote escaping only. Chrome sends
`key: "\"` (U+005C) for the backslash key, which became `key:'\'` — the
backslash escaped the closing quote, ran the string literal on, and made
the whole page.evaluate a syntax error, so the keydown was silently
dropped. Any key/code carrying a backslash broke dispatch.
Escape backslash before single-quote (backslash first, so the quote-escape
backslashes are not themselves doubled), matching the sibling text path in
the same function. Adds a regression test asserting the backslash key
arrives at a keydown listener alongside an ordinary key.
* fix(js): structuredClone cycles through Error.cause and clones own __proto__ (#419, #420)
The structuredClone polyfill had two correctness/parity gaps in _structuredClone:
- The Error branch recursed into `cause` before recording the clone in `seen`,
so a cycle through the error (e.cause === e) recursed until the stack
overflowed. Chrome clones such graphs and preserves identity. Fix: seen.set
the clone before recursing into cause. (#419)
- The generic-object branch built the clone on the source's prototype and
copied properties by assignment, so an own enumerable `__proto__` data
property (exactly what JSON.parse('{"__proto__":…}') produces) went through
the inherited __proto__ setter and reparented the clone instead of copying
the property. Fix: clone plain objects onto Object.prototype (like Chrome)
and define each property instead of assigning it. (#420)
Adds two parity tests to structured_clone_crypto_parity.rs; full obscura-cdp
suite (73) green.
* perf(js): keep structuredClone fast path for all keys but __proto__
The own-__proto__ fix copied every own property with Object.defineProperty
to avoid triggering the inherited prototype setter. That is ~3x slower
than plain assignment per key, so cloning a large object regressed
(~75ms -> ~228ms for a 1000-key object x200).
Only the __proto__ key needs defineProperty; every other key is safe to
assign. Special-case it so the fast path applies to all other keys.
Measured back to parity with main (85ms vs 85ms); the own-__proto__
correctness test still passes.
---------
Co-authored-by: SGavrl <[email protected]>
HTMLFormElement.requestSubmit(submitter) accepted any element and submitted
silently. Per the HTML spec (and Chrome) it must throw a TypeError when the
submitter is not a submit button, and a NotFoundError DOMException when the
submitter is not owned by the form; both checks run before the submit event.
Adds an _isSubmitButton predicate (a <button> whose type is submit — the
default, including a missing/invalid type — or an <input> of type submit/image)
and validates the argument at the top of requestSubmit. submit() is untouched:
it correctly bypasses the event (#395).
Element.click() now gates on the same predicate rather than its own looser
check, so an internal click can never hand requestSubmit a submitter it would
reject (e.g. <div type="submit">, which is not a submit button and no longer
submits — matching Chrome). That also aligns click() with the CDP click path in
input.rs, which already treats <input type=image> as a submit button. The
input.rs call sites are already guarded by equivalent conditions and pass no
submitter on the Enter-key path, so they are unaffected.
Constraint validation is deliberately out of scope: obscura has no validity
engine (checkValidity is a stub returning true), so wiring it here would be a
no-op. Noted in #424 as separate work.
Full obscura-cdp suite (72) green.
_structuredClone dispatches platform-object hooks as hook(value, seen), but the
CryptoKey hook took only `src` and never touched the memo, and hook branches
return before the generic seen.set. So one CryptoKey referenced twice in a graph
cloned into two distinct objects (and re-registered its key material each time),
where structuredClone preserves reference identity within a graph.
Consult and populate `seen` in the hook. Adds a parity test asserting a key
reached twice clones to one shared object that crypto.subtle still accepts.
* fix(net): default cookie Path to the request directory (RFC 6265 5.1.4)
When a Set-Cookie carried no Path attribute, the jar stored the full request
path as the cookie's path. RFC 6265 5.1.4 default-path is the request URI's
directory — the path up to but not including the right-most '/'. Storing the
full path scoped a session cookie to the exact URL that set it: a cookie set on
GET /app/login was stored with path=/app/login and then, because path-match
(5.1.4, fixed earlier) requires a '/' boundary, was not sent to /app/dashboard.
The user appeared logged out on the very next navigation.
Add default_cookie_path() implementing the RFC algorithm and use it as the
initial path in both set_cookie (HTTP) and set_cookie_from_js (document.cookie).
An explicit Path attribute still overrides it. Tests cover the algorithm and the
end-to-end 'cookie set on /app/login is sent to /app/dashboard' path for both
the HTTP and JS entry points; both failed before this change.
* fix(cdp): apply RFC 6265 default cookie Path to CDP-injected cookies
Network.setCookie/setCookies (the Puppeteer page.setCookie and
Playwright addCookies surface) derived the cookie Path from the full
request URL when the caller omitted path, the same bug the parent
commit fixed for Set-Cookie responses. Reuse default_cookie_path (now
pub) so a cookie set under /v1/things scopes to /v1 and is resent on
sibling paths. Fixes the CDP half of #401.
Correct the test that locked in the old full-path behavior and add a
case confirming an explicit Path still wins.
---------
Co-authored-by: SGavrl <[email protected]>
The previous fix escaped only "-->" in comment data, but the HTML tokenizer
ends a comment on four sequences: "-->", "--!>", a leading ">", and a leading
"->". document.createComment(...) accepts arbitrary strings, so a scripted
comment like "><img src=x onerror=alert(1)>" still serialized to
"<!--><img ...>-->": the leading ">" abrupt-closes the empty comment and the
trailing <img> parses as live markup (mXSS). The "--!>" and "->" forms escape
the same way.
Every terminator form requires a ">", and entities are not decoded inside
comments, so escape every ">" to ">": this neutralizes all four at once,
keeps the payload inside a single comment, and supersedes the "-->"-only guard.
Regression test builds a scripted comment for each terminator form, serializes,
re-parses, and asserts no <img> escapes; the leading-">", leading-"->", and
"--!>" cases failed before this change.
server.rs logged previews of attacker-controlled strings with raw byte
slicing: `&text[..text.len().min(200)]` for a malformed CDP frame (inside a
`warn!`, which is active at the worker's default log level) and the same for
an intercepted event JSON. `&s[..n]` panics when byte `n` falls inside a
multi-byte UTF-8 character, so a single WebSocket frame that is invalid JSON,
>=201 bytes, with a multi-byte char straddling byte 200 panics the CDP
processor task — an unauthenticated one-frame crash. page.rs had the same
pattern (behind --verbose debug logging) for evaluated JS expressions.
Add truncate_on_char_boundary (str::floor_char_boundary is still unstable) and
route all four sites through it. Regression test constructs the exact 'a'*199 +
'€' input, asserts the old raw slice panics (via catch_unwind) and the helper
truncates on the boundary instead.
DOM.getDocument{depth:-1} (the standard full-tree call from Puppeteer/
Playwright/browser-use) arrived as u32::MAX and drove serialize_node to
recurse once per DOM nesting level. A page can nest nodes tens of thousands
deep (trivially scriptable; html5ever caps no generic nesting), so the
recursion overflowed the stack — a SIGABRT that catch_unwind cannot recover,
crashing the whole worker (AGENTS.md: "one page must never crash a worker").
Deriving the builder alone is not enough: the response is inherently nested
JSON, and serde_json's own serialization and the Value's Drop recurse over
that nesting too, overflowing the ~2 MiB tokio worker stack. So this both
rewrites serialize_node as an explicit heap worklist and clamps the effective
depth to MAX_SERIALIZE_DEPTH (256), mirroring DOMSnapshot's MAX_NODES guard.
Real DOMs are far shallower; clients wanting a deeper subtree re-request it
with DOM.requestChildNodes / describeNode.
Adds a regression test that builds a 50k-deep tree and serializes with
depth:-1; it SIGABRTs before this change and passes after (run on nextest's
2 MiB test thread, mirroring production).
* fix(js): fire trusted input/change from setFileInputFiles
DOM.setFileInputFiles dispatched plain input/change events, so
event.isTrusted was false. Upload flows and anti-bot code that gate their
change handler on isTrusted ignored them, which silently broke the exact
case the feature targets. The Input domain already marks its synthesized
clicks/keys trusted via __obscura_markTrusted; do the same here.
Fixes#384.
* fix(cdp): bound IO stream buffers so downloads cannot leak
Fetch.takeResponseBodyAsStream moved a response body out of the page's
LRU-bounded cache into CdpContext.io_streams, a server-lifetime map freed only
by an explicit IO.close. A client that opens a stream and disconnects, errors
mid-download, or never closes it leaked the full buffer forever; no teardown
path (WS close, Browser.close, Target.closeTarget) touched io_streams. That
reintroduced the unbounded memory growth streaming was meant to avoid (#360).
Replace the raw map + counter with an IoStreamStore that caps the number of
open streams and their total bytes (OBSCURA_IO_STREAM_MAX_ENTRIES / _BYTES),
evicting the oldest first and always keeping the just-opened stream. Reading an
evicted handle fails cleanly, which is the right trade against an OOM. Read
chunking is unchanged.
Fixes#386.
Cookie path filtering used `request_path.starts_with(cookie_path)`, so a
Path=/admin cookie was sent to sibling paths that merely share the string
prefix, such as /administrator or /admin-backup. Per RFC 6265 5.1.4 a prefix
match also requires the boundary to fall on a '/'.
Add a path_matches helper (identical path, or a prefix ending in '/', or a
prefix followed by '/') and use it in get_cookie_header and
get_js_visible_cookies.
Fixes#382.
DOM.setFileInputFiles dispatched plain input/change events, so
event.isTrusted was false. Upload flows and anti-bot code that gate their
change handler on isTrusted ignored them, which silently broke the exact
case the feature targets. The Input domain already marks its synthesized
clicks/keys trusted via __obscura_markTrusted; do the same here.
Fixes#384.
handle_fetch_resolution built InterceptResolution::Continue with url, method,
headers, and body hardcoded to None, so Fetch.continueRequest silently sent the
request unmodified. Playwright route.continue({ headers, url, postData }) and
Puppeteer request.continue reported success while their overrides were dropped.
op_fetch_url already applies all four (and re-validates a rewritten URL through
the SSRF gate); only the CDP layer was discarding them. Read url, method,
headers, and postData from the continueRequest params and pass them through.
Fixes#365.
The cdp_processor decided whether to take the spawn-and-defer navigation path
with `text.contains("Page.navigate")`, which also matches
`Page.navigateToHistoryEntry`. goBack / goForward (both Puppeteer and
Playwright drive Page.navigateToHistoryEntry) were misrouted into the raw
navigate path, which reads an absent `url` param and fails with
"Invalid URL: relative URL without a base", so history navigation never
reached its real handler. Any frame merely embedding the literal
"Page.navigate" (e.g. a Runtime.evaluate expression) was misrouted the same
way.
Match on the parsed CDP method instead, so only an exact Page.navigate takes
the navigation path.
Fixes#363.
Andrey Mnatsakanov