diff --git a/AGENTS.md b/AGENTS.md index ec192f1..032b119 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ Fix root cause. Unsure: read more code; if stuck, ask w/ short options. Unrecogn | formats/format-support | Formats, bytes-in API, detection order, DOCX + spreadsheet mapping | | integrations/tool-contract | `MarkkitTool` LLM tool (`tool` feature), Everruns toolkit contract | | security/security-model | One-page threat/invariant summary | -| security/threat-model | Threats T1-T9, mitigations, test map, resource limits, invariants S1-S20 | +| security/threat-model | Threats T1-T9, mitigations, test map, resource limits, invariants S1-S21 | | runtimes/python-package | PyO3 `markkit` module, maturin, PyPI | | runtimes/wasm-package | wasm-bindgen `@everruns/markkit-wasm`, browser/edge, build + tests | | runtimes/node-package | NAPI-RS `@everruns/markkit`, npm, binding parity table | diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ba5047..132f985 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -152,6 +152,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Fixed +- HTML: deeply nested input no longer costs quadratic time. 687 KiB of nested `
`s took + 12.6 s and 292 KiB of unclosed nested tables ran past the 60 s timeout; tags nested past + `max_nesting_depth` are now dropped (text kept) before html5ever parses, so both take + milliseconds. +- HTML: rows of nested tables are no longer copied into every enclosing table (output grew 4x + per nesting level). +- HTML: text flattened past `max_nesting_depth` no longer includes `" + ); + assert!(matches!(cap_nesting(&html, 5, true), Cow::Borrowed(_))); + } + + #[test] + fn optional_end_tags_and_voids_do_not_count() { + let html = "

a

  • b
    c
  • ` closes the unclosed spans inside it; depth must not drift. + let html = "
    x
    ".repeat(1000); + assert!(matches!(cap_nesting(&html, 10, true), Cow::Borrowed(_))); + } + + #[test] + fn raw_text_end_tags_survive_while_dropping() { + // Dropping `` would turn the rest of the page into script. + let html = format!( + "{}

    after

    {}", + "
    ".repeat(50), + "
    ".repeat(50) + ); + let capped = cap_nesting(&html, 10, true); + assert!(capped.contains("")); + assert!(capped.contains("after")); + } + + #[test] + fn uncounted_end_tags_do_not_consume_overflow() { + let html = format!( + "{}

    a

  • b
  • {}", + "
    ".repeat(20), + "
    ".repeat(20) + ); + let capped = cap_nesting(&html, 10, true); + // Past the cap the p/li tags go too, their text stays, and their end + // tags do not close dropped divs. + assert!(capped.contains("ab")); + assert_eq!(capped.matches("
    ").count(), 10); + assert_eq!(capped.matches("
    ").count(), 10); + } + + #[test] + fn non_content_past_the_cap_is_dropped_whole() { + let html = format!( + "{}
    f

    after

    ", + "
    ".repeat(20) + ); + let capped = cap_nesting(&html, 10, true); + assert!(!capped.contains("tpl") && !capped.contains("in<") && !capped.contains("f")); + assert!(capped.contains("after")); + let objects = "".repeat(100_000) + "x"; + assert!(!cap_nesting(&objects, 10, true).contains('x')); + } + + #[test] + fn hidden_tags_past_the_cap_are_dropped_whole() { + // 11 divs: the last is dropped, so even optional-end tags (`p`) are + // in the dropped region (at the cap they would stay in the DOM, where + // the walker's flattener checks them). + let deep = "
    ".repeat(11); + for tag in [ + r#""#, + r#"
    S
    "#, + r#"

    S

    "#, + r#"S"#, + ] { + let html = format!("{deep}a{tag}b"); + let html = html.as_str(); + let capped = cap_nesting(html, 10, true); + assert!( + capped.contains("ab") && !capped.contains('S'), + "{tag}: {capped}" + ); + // Opted in: only the tag goes, the text stays. + assert!(cap_nesting(html, 10, false).contains('S'), "{tag}"); + } + let shown = format!("{deep}T"); + let capped = cap_nesting(&shown, 10, true); + assert!(capped.contains('S') && capped.contains('T'), "{capped}"); + } + + #[test] + fn raw_attrs_parse_quotes_and_bare_values() { + assert_eq!( + raw_attrs(r#" a b=1 c = "x y" d='z>' /"#), + [("a", ""), ("b", "1"), ("c", "x y"), ("d", "z>")] + ); + assert_eq!(raw_attrs(r#" e="unterminated"#), [("e", "unterminated")]); + } + + #[test] + fn malformed_tails_do_not_panic() { + for s in [ + "<", "", "", "<1>", "<\u{e9}>", + ] { + let _ = cap_nesting(s, 1, true); + let _ = cap_nesting(&format!("
    {s}"), 0, true); + } + } +} diff --git a/crates/markkit/src/html/parser.rs b/crates/markkit/src/html/parser.rs index f6c385c..2ffd2e8 100644 --- a/crates/markkit/src/html/parser.rs +++ b/crates/markkit/src/html/parser.rs @@ -37,26 +37,32 @@ use crate::rules::ElementAction; use url::Url; use super::metadata::PageInfo; +use super::nesting::{self, NON_CONTENT}; use super::{readability, urls}; +/// Extra levels kept above `max_nesting_depth` before raw tags are dropped: +/// the walker flattens there anyway, the slack covers `html`/`body` and +/// implied elements the pre-scan does not count. +const NESTING_SLACK: usize = 8; + /// Hidden-content policy (threat model T9): an element a reader never sees is /// skipped unless `include_hidden_content(true)`. Only signals on the element /// itself count (`hidden`, `aria-hidden="true"`, inline `style`); stylesheet and /// class-based hiding (`.sr-only`, off-screen positioning) is not detected. fn is_hidden(el: &scraper::node::Element) -> bool { - if el - .attr("hidden") - .is_some_and(|v| !v.eq_ignore_ascii_case("until-found")) - { + hidden_by(|name| el.attr(name)) +} + +/// [`is_hidden`] over any attribute lookup (also used by the nesting pre-scan, +/// which sees raw tags). +pub(crate) fn hidden_by<'v>(attr: impl Fn(&str) -> Option<&'v str>) -> bool { + if attr("hidden").is_some_and(|v| !v.eq_ignore_ascii_case("until-found")) { return true; } - if el - .attr("aria-hidden") - .is_some_and(|v| v.trim().eq_ignore_ascii_case("true")) - { + if attr("aria-hidden").is_some_and(|v| v.trim().eq_ignore_ascii_case("true")) { return true; } - let Some(style) = el.attr("style") else { + let Some(style) = attr("style") else { return false; }; style.split(';').any(|decl| { @@ -173,12 +179,21 @@ impl<'a> HtmlParser<'a> { /// Parse an HTML string into a [`Document`]. pub fn parse(&self, html: &str) -> Result { - self.parse_dom(&Html::parse_document(html)) + self.parse_dom(&self.parse_html5ever(html)) + } + + /// html5ever parse of `html` with nesting capped first (see `nesting`). + fn parse_html5ever(&self, html: &str) -> Html { + Html::parse_document(&nesting::cap_nesting( + html, + self.max_depth + NESTING_SLACK, + !self.config.include_hidden_content, + )) } /// Parse an HTML string into a [`Document`] plus page title and metadata. pub(crate) fn parse_page(&self, html: &str) -> Result<(Document, PageInfo), Error> { - let dom = Html::parse_document(html); + let dom = self.parse_html5ever(html); let this = self.for_document(&dom); Ok(( this.convert_dom(&dom)?, @@ -325,7 +340,7 @@ impl<'a> HtmlParser<'a> { } if depth >= self.max_depth { // At max depth, just extract text - let text: String = parent.text().collect(); + let text = flat_text(parent, !self.config.include_hidden_content); let trimmed = text.trim(); if !trimmed.is_empty() { out.push(Node::Text(trimmed.to_string())); @@ -410,7 +425,7 @@ impl<'a> HtmlParser<'a> { return None; } if depth >= self.max_depth { - let text: String = el.text().collect(); + let text = flat_text(el, !self.config.include_hidden_content); let trimmed = text.trim(); if trimmed.is_empty() { return None; @@ -609,9 +624,7 @@ impl<'a> HtmlParser<'a> { "hr" => Some(Node::ThematicBreak), // Skip non-content elements - "script" | "style" | "noscript" | "template" | "iframe" | "object" | "embed" - | "form" | "input" | "button" | "select" | "textarea" | "svg" | "canvas" | "video" - | "audio" | "source" | "track" | "map" | "area" => None, + name if NON_CONTENT.contains(&name) => None, // Unknown element: try to extract inline content _ => { @@ -927,7 +940,7 @@ impl<'a> HtmlParser<'a> { /// `` that is a footnote reference: record target and build the node. fn footnote_reference(&self, el: scraper::ElementRef<'_>) -> Option { let target = el.value().attr("href")?.strip_prefix('#')?; - let text: String = el.text().collect(); + let text = flat_text(el, !self.config.include_hidden_content); let text = text .trim() .trim_start_matches('[') @@ -951,7 +964,7 @@ impl<'a> HtmlParser<'a> { return nodes; } if depth >= self.max_depth { - let text: String = el.text().collect(); + let text = flat_text(el, !self.config.include_hidden_content); let trimmed = text.trim(); if !trimmed.is_empty() { nodes.push(Node::Text(trimmed.to_string())); @@ -1040,17 +1053,15 @@ impl<'a> HtmlParser<'a> { exclude: &[Selector], depth: usize, ) -> Option { - let thead_sel = Selector::parse("thead").unwrap(); - let tbody_sel = Selector::parse("tbody").unwrap(); - let tr_sel = Selector::parse("tr").unwrap(); - let mut headers: Vec = Vec::new(); let mut alignments: Vec = Vec::new(); let mut rows: Vec> = Vec::new(); - if let Some(thead) = table_el.select(&thead_sel).next() - && let Some(tr) = thead.select(&tr_sel).next() - { + // Direct structure only: a descendant query would also pick up the + // rows of nested tables, duplicating them into every enclosing table + // (output and work grew 4x per nesting level). THREAT[T2c]. + let (head_row, body_rows) = table_rows(table_el); + if let Some(tr) = head_row { let ths = row_cells(tr, &["th", "td"]); alignments = column_alignments(&ths); headers = ths @@ -1059,13 +1070,6 @@ impl<'a> HtmlParser<'a> { .collect(); } - let body_rows: Vec> = - if let Some(tbody) = table_el.select(&tbody_sel).next() { - tbody.select(&tr_sel).collect() - } else { - table_el.select(&tr_sel).collect() - }; - for tr in body_rows { if self.is_excluded(&tr, exclude) { continue; @@ -1135,6 +1139,64 @@ impl<'a> HtmlParser<'a> { } } +/// Text of a subtree flattened at `max_nesting_depth`, without the content of +/// [`NON_CONTENT`] elements (`el.text()` leaked `

    after

    {}", + "
    ".repeat(depth), + "
    ".repeat(depth) + ); + let md = convert(&html); + assert!(md.contains("ok") && md.contains("after"), "{depth}: {md}"); + for bad in [ + "SECRET_JS", + "color:red", + "tpl", + "", + ] { + assert!(!md.contains(bad), "{depth}: {bad} leaked: {md}"); + } + } +} + +#[test] +fn hidden_content_does_not_leak_past_max_depth() { + // Flattened deep subtrees honour the hidden-content default (S20). + let html = format!( + "{}

    ok

    SECRET_B

    SECRET_C

    {}", + "
    ".repeat(300), + "
    ".repeat(300) + ); + let md = convert(&html); + assert!(md.contains("ok"), "{md}"); + assert!(!md.contains("SECRET"), "{md}"); + let shown = markkit::Markkit::builder() + .include_hidden_content(true) + .build() + .extract_html_sync(&html) + .unwrap(); + assert!( + shown.contains("SECRET_A") && shown.contains("SECRET_C"), + "{shown}" + ); +} + #[test] fn very_long_text_node() { let text = "a".repeat(100_000); diff --git a/knowledge/formats/html-extraction.md b/knowledge/formats/html-extraction.md index f770e9d..8033ace 100644 --- a/knowledge/formats/html-extraction.md +++ b/knowledge/formats/html-extraction.md @@ -134,12 +134,28 @@ Trimming at render time: Limit: `limits.max_nesting_depth` (default 256, clamped to `HARD_MAX_NESTING_DEPTH` = 512) Behavior at depth limit: -- `convert_children`: extracts all descendant text via `.text().collect()`, trims, emits as single `Text` node -- `convert_element`: same -- flattens subtree to trimmed text -- `collect_inline`: same -- flattens subtree to trimmed text +- `convert_children`: flattens the subtree to its text (`flat_text`: iterative, skips the + content of non-content elements such as `script`, `style`, `template`, `form`, and of + hidden elements unless `include_hidden_content`), trims, + emits a single `Text` node +- `convert_element`, `collect_inline`: same - Empty text at depth limit -> elided -Prevents stack overflow on adversarial nesting. Tested with 1000 nested `
    ` elements. +Before html5ever runs, `html::nesting::cap_nesting` pre-scans the raw HTML and drops start +tags (and paired end tags) that would nest deeper than `max_nesting_depth + 8`, keeping their +text; non-content elements past the cap, and hidden ones (raw-attribute check with the +walker's T9 rules, entities not decoded) unless `include_hidden_content`, are dropped whole. html5ever's tree builder is +quadratic in depth (most start tags scan the open-element stack), so this keeps parsing +linear. Comments, quoted attributes and raw-text elements are skipped; void and optional-end +elements (`p`, `li`, `td`, ...) are not counted; table parts close implicitly. Input that +never reaches the cap is passed through unchanged (no copy). + +Tables use direct structure only: header = first `` of the first direct ``; body += direct `` children and `` children of direct `` sections. Rows of nested +tables stay in the nested table. + +Prevents stack overflow and super-linear time on adversarial nesting. Tested with 64 000 +nested `
    `s, 20 000 unclosed nested tables, and script/style past the limit. ## Exclude Selector Defaults diff --git a/knowledge/log.md b/knowledge/log.md index 1f1eb5e..17f7637 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -5,6 +5,7 @@ * **XLSB support (#11)**: Excel binary workbooks are read by a new bounds-checked BIFF12 reader (`spreadsheet/xlsb.rs`) instead of being rejected; calamine's XLSB reader was not used because it panics on malformed records. Same caps, hidden-sheet and macro rules as XLSX. See [Format Support](formats/format-support.md) and [Threat Model](security/threat-model.md) T6c. * **Over-escaping fix**: line-start `#`, `+` and ordered markers are escaped only when followed by whitespace or the end of the text run, so `4.6 out of 5` no longer renders `4\.6` (eval gap). `> - =` stay always escaped. See [Threat Model](security/threat-model.md) T1, [Evaluation Harness](operations/eval.md). * **MCP server**: `markkit mcp` serves `MarkkitTool` over MCP stdio (initialize, ping, tools/list, tools/call); line size capped from `--max-size`, tool failures as `isError` results. New [MCP Server](integrations/mcp-server.md) concept; MCP messages row in the [Threat Model](security/threat-model.md). +* **Nesting cost**: HTML parsing was quadratic in nesting depth (html5ever) and exponential in nested tables (descendant row selection duplicated inner rows); text flattened past `max_nesting_depth` leaked `