diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b98d58..d5743df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Fixed +- Prevent raw HTML escaping code blocks inside task lists and nested definitions. + Task bodies now start below their checkbox; definitions render as GFM lists + below each term, preserving code blocks without requiring a definition-list extension. + Nested lists beginning with an empty item are separated from preceding text. Hard + breaks inside headings and link labels are flattened so they cannot close the + container before later inline code. + - PDF stream guards honor escaped dictionary keys and bound compressed prefixes before image filters, including when OCR is disabled. diff --git a/crates/markkit/fuzz/seeds/markdown/nested-definition-code.md b/crates/markkit/fuzz/seeds/markdown/nested-definition-code.md new file mode 100644 index 0000000..5988df7 Binary files /dev/null and b/crates/markkit/fuzz/seeds/markdown/nested-definition-code.md differ diff --git a/crates/markkit/fuzz/seeds/render_ir/empty-leading-list-item.bin b/crates/markkit/fuzz/seeds/render_ir/empty-leading-list-item.bin new file mode 100644 index 0000000..83f4f2a Binary files /dev/null and b/crates/markkit/fuzz/seeds/render_ir/empty-leading-list-item.bin differ diff --git a/crates/markkit/fuzz/seeds/render_ir/heading-link-hard-break.bin b/crates/markkit/fuzz/seeds/render_ir/heading-link-hard-break.bin new file mode 100644 index 0000000..ce18746 Binary files /dev/null and b/crates/markkit/fuzz/seeds/render_ir/heading-link-hard-break.bin differ diff --git a/crates/markkit/fuzz/seeds/render_ir/task-leading-code.bin b/crates/markkit/fuzz/seeds/render_ir/task-leading-code.bin new file mode 100644 index 0000000..fad85e3 Binary files /dev/null and b/crates/markkit/fuzz/seeds/render_ir/task-leading-code.bin differ diff --git a/crates/markkit/src/markdown.rs b/crates/markkit/src/markdown.rs index c04e9e8..6c60e52 100644 --- a/crates/markkit/src/markdown.rs +++ b/crates/markkit/src/markdown.rs @@ -582,7 +582,7 @@ mod tests { fn table_and_tasks() { assert_eq!( md("| a | b |\n|---|---|\n| 1 | 2 |\n\n- [x] done\n- [ ] todo\n"), - "| a | b |\n| --- | --- |\n| 1 | 2 |\n\n- [x] done\n- [ ] todo\n" + "| a | b |\n| --- | --- |\n| 1 | 2 |\n\n- [x]\n done\n- [ ]\n todo\n" ); } @@ -616,7 +616,8 @@ mod tests { ); let out = crate::render::MarkdownRenderer::new().render(&p.doc); assert!(out.contains("| :--- | :---: | ---: |"), "{out}"); - assert!(out.contains("- [x] done"), "{out}"); + assert!(out.contains("- [x]\n done"), "{out}"); + assert!(out.contains("Term\n\n- Definition"), "{out}"); assert!(out.contains("Note[^n]."), "{out}"); assert!(out.contains("[^n]:\n\n The note."), "{out}"); } diff --git a/crates/markkit/src/render/markdown.rs b/crates/markkit/src/render/markdown.rs index 9be3c9b..eee091f 100644 --- a/crates/markkit/src/render/markdown.rs +++ b/crates/markkit/src/render/markdown.rs @@ -1,10 +1,10 @@ //! IR → Markdown renderer. //! //! Decisions: -//! - GFM is the target. Constructs GFM lacks use the de facto extensions most -//! Markdown tools read: footnotes `[^label]` / `[^label]: body`, definition -//! lists `Term` + `: definition` (PHP Markdown Extra / Pandoc), task items -//! `- [x]`. Figures render their content plus an italic caption paragraph. +//! - GFM is the target. Definition lists use term paragraphs and ordinary list +//! items so their block bodies stay safe without a definition-list extension. +//! Footnotes use `[^label]` / `[^label]: body`; task bodies start below `- [x]`. +//! Figures render their content plus an italic caption paragraph. //! - Table spans are laid out on a grid (GFM has no spans): a spanned cell's //! content goes in its top-left slot, covered slots stay empty. Layout is //! bounded by `MAX_TABLE_GRID_CELLS`; a table whose spans would exceed it is @@ -23,7 +23,7 @@ use std::time::Instant; use super::escape::{self, Position, UrlKind}; -use crate::ir::{Alignment, DefinitionItem, Document, ListItem, Node, TableCell}; +use crate::ir::{Alignment, DefinitionItem, Document, Node, TableCell}; /// Grid budget for span layout (rows × columns). const MAX_TABLE_GRID_CELLS: usize = 1_000_000; @@ -51,6 +51,8 @@ pub struct MarkdownRenderer { /// Rendering link text: nested links render as text (CommonMark drops the /// outer link otherwise). in_link: bool, + /// Rendering inline content that must stay on one Markdown line. + single_line: bool, } impl MarkdownRenderer { @@ -63,6 +65,7 @@ impl MarkdownRenderer { line_start: false, in_table: false, in_link: false, + single_line: false, } } @@ -72,6 +75,7 @@ impl MarkdownRenderer { self.line_start = false; self.in_table = false; self.in_link = false; + self.single_line = false; self.render_nodes(&doc.nodes, 0); normalize_output(&self.buf) } @@ -166,7 +170,7 @@ impl MarkdownRenderer { self.buf.push(' '); let start = self.buf.len(); self.line_start = true; - self.render_inline(children); + self.render_inline_single_line(children); trim_trailing_inline(&mut self.buf, start); self.buf.push('\n'); } @@ -185,7 +189,13 @@ impl MarkdownRenderer { items, .. } => { - self.render_list(*ordered, *start, items); + self.render_list( + *ordered, + *start, + items + .iter() + .map(|item| (item.checked, item.children.as_slice())), + ); } Node::BlockQuote { children, .. } => { @@ -234,7 +244,7 @@ impl MarkdownRenderer { self.buf.push_str("---\n"); } - Node::DefinitionList { items, .. } => self.render_definition_list(items, depth), + Node::DefinitionList { items, .. } => self.render_definition_list(items), Node::Figure { children, caption, .. @@ -297,8 +307,9 @@ impl MarkdownRenderer { } } - /// Render a definition list: `Term` line, then `: definition` per definition. - fn render_definition_list(&mut self, items: &[DefinitionItem], depth: usize) { + /// Use GFM lists for definitions so nested blocks stay structured in readers + /// without the definition-list extension (S17). + fn render_definition_list(&mut self, items: &[DefinitionItem]) { for (i, item) in items.iter().enumerate() { if self.over_limit() { return; @@ -310,20 +321,25 @@ impl MarkdownRenderer { self.line_start = true; self.render_inline(&item.term); trim_trailing_inline(&mut self.buf, start); - self.buf.push('\n'); - for def in &item.definitions { - let inner = self.render_blocks(def, depth); - self.buf.push(':'); - push_hanging(&mut self.buf, &inner, " ", " "); - } + self.buf.push_str("\n\n"); + self.render_list( + false, + None, + item.definitions.iter().map(|def| (None, def.as_slice())), + ); } } /// Render a list. Each item body is rendered on its own and indented by /// the marker width, so nested blocks stay inside the item for any marker. - fn render_list(&mut self, ordered: bool, start: Option, items: &[ListItem]) { + fn render_list<'a>( + &mut self, + ordered: bool, + start: Option, + items: impl IntoIterator, &'a [Node])>, + ) { let start_num = start.unwrap_or(1); - for (i, item) in items.iter().enumerate() { + for (i, (checked, children)) in items.into_iter().enumerate() { if self.over_limit() { return; } @@ -335,17 +351,22 @@ impl MarkdownRenderer { let pad = " ".repeat(marker.len()); let body = { let mut sub = self.sub(); - sub.render_item_children(&item.children); + sub.render_item_children(children); self.absorb(&sub); normalize_output(&sub.buf) }; let mut lines = body.lines(); self.buf.push_str(&marker); - match item.checked { + match checked { Some(true) => self.buf.push_str("[x] "), Some(false) => self.buf.push_str("[ ] "), None => {} } + if checked.is_some() { + // The checkbox is a paragraph; a following block needs its own line. + self.buf.push('\n'); + self.buf.push_str(&pad); + } self.buf.push_str(lines.next().unwrap_or("")); self.buf.push('\n'); for line in lines { @@ -358,39 +379,63 @@ impl MarkdownRenderer { } } - /// Like [`Self::render_nodes`], but keeps the common "text, then nested list" - /// item tight (no blank line) when the list can interrupt a paragraph. + /// Keep nested lists tight only when their rendered first item can interrupt + /// the preceding paragraph; empty items require a blank line (S17). fn render_item_children(&mut self, nodes: &[Node]) { for (i, node) in nodes.iter().enumerate() { if self.over_limit() { return; } let block = node.is_block(); - if i > 0 { - let prev = &nodes[i - 1]; - let tight = matches!(prev, Node::Paragraph { .. } | Node::Heading { .. }) - || prev.is_inline(); - let interrupts = match node { - Node::List { ordered, start, .. } => { - !ordered || matches!(start, None | Some(1)) - } - _ => false, - }; - if tight && interrupts { - if !self.buf.ends_with('\n') { - self.buf.push('\n'); - } - } else if block || prev.is_block() { - if !self.buf.ends_with('\n') { - self.buf.push('\n'); + let tight = i > 0 + && (matches!(nodes[i - 1], Node::Paragraph { .. } | Node::Heading { .. }) + || nodes[i - 1].is_inline()); + let candidate = matches!( + node, + Node::List { ordered: false, .. } + | Node::List { + ordered: true, + start: None | Some(1), + .. } + ); + let list_body = if tight && candidate { + // Inspect actual output: empty paragraphs/inline nodes can leave + // the first marker empty. Render once to avoid recursive rework. + let mut sub = self.sub(); + sub.max_output = self.max_output.saturating_sub(self.buf.len()); + sub.render_node(node, 0); + sub.over_limit(); + self.absorb(&sub); + if self.tripped.is_some() { + return; + } + Some(normalize_output(&sub.buf)) + } else { + None + }; + let interrupts = list_body.as_ref().is_some_and(|body| { + body.lines() + .next() + .and_then(|line| line.split_once(' ')) + .is_some_and(|(_, content)| !content.is_empty()) + }); + if i > 0 && (block || nodes[i - 1].is_block()) { + if !self.buf.ends_with('\n') { + self.buf.push('\n'); + } + if !interrupts { self.buf.push('\n'); } } if !block && (self.buf.is_empty() || self.buf.ends_with('\n')) { self.line_start = true; } - self.render_node(node, 0); + if let Some(body) = list_body { + self.buf.push_str(&body); + } else { + self.render_node(node, 0); + } } } @@ -401,6 +446,14 @@ impl MarkdownRenderer { } } + /// Render inline content without allowing it to close its enclosing syntax. + fn render_inline_single_line(&mut self, nodes: &[Node]) { + let previous = self.single_line; + self.single_line = true; + self.render_inline(nodes); + self.single_line = previous; + } + /// Render a single inline node. fn render_inline_node(&mut self, node: &Node) { match node { @@ -431,7 +484,7 @@ impl MarkdownRenderer { self.buf.push('['); self.line_start = false; self.in_link = true; - self.render_inline(children); + self.render_inline_single_line(children); self.in_link = false; self.buf.push_str("]("); self.buf.push_str(&dest); @@ -443,7 +496,7 @@ impl MarkdownRenderer { self.buf.push(')'); } // Disallowed scheme or nested link: keep the text, drop the link. - None => self.render_inline(children), + None => self.render_inline_single_line(children), }, Node::Image { src, alt } => match escape::safe_destination(src, UrlKind::Image) { Some(mut dest) => { @@ -470,8 +523,9 @@ impl MarkdownRenderer { // paragraph (and any open link) early. Node::LineBreak if self.line_start => {} Node::LineBreak => { - if self.in_table { + if self.in_table || self.single_line { self.buf.push(' '); + self.line_start = false; } else { self.buf.push_str(" \n"); self.line_start = true; @@ -508,6 +562,8 @@ impl MarkdownRenderer { /// Render a block node as single-line inline content. fn render_block_inline(&mut self, node: &Node) { + let previous_single_line = self.single_line; + self.single_line = true; let sep = |r: &mut Self| { // No space right after an opening delimiter (`**`, `~~`, `[`). if !r.buf.is_empty() && !r.buf.ends_with([' ', '\n', '*', '~', '[']) { @@ -566,6 +622,7 @@ impl MarkdownRenderer { Node::ThematicBreak => sep(self), inline => self.render_inline_node(inline), } + self.single_line = previous_single_line; } /// Render a GFM table, laying spans out on a grid. diff --git a/crates/markkit/tests/html_integration.rs b/crates/markkit/tests/html_integration.rs index c35f625..9fa5d84 100644 --- a/crates/markkit/tests/html_integration.rs +++ b/crates/markkit/tests/html_integration.rs @@ -765,13 +765,13 @@ fn task_list_github_markup() {
  • Todo
  • Plain
    • "#; - assert_eq!(convert(html), "- [x] Done\n- [ ] Todo\n- Plain\n"); + assert_eq!(convert(html), "- [x]\n Done\n- [ ]\n Todo\n- Plain\n"); } #[test] fn task_list_checkbox_inside_paragraph() { let html = r#"

    1. Ship it

    "#; - assert_eq!(convert(html), "1. [x] Ship it\n"); + assert_eq!(convert(html), "1. [x]\n Ship it\n"); } #[test] @@ -792,14 +792,14 @@ fn definition_list() {
    Go

    Another.

    Second para.

    "; assert_eq!( convert(html), - "Rust\n: A language.\n: Also a fungus.\n\nGo\n: Another.\n\n Second para.\n" + "Rust\n\n- A language.\n- Also a fungus.\n\nGo\n\n- Another.\n\n Second para.\n" ); } #[test] fn definition_list_div_wrappers_and_shared_terms() { let html = "
    A
    B
    Both
    "; - assert_eq!(convert(html), "A \nB\n: Both\n"); + assert_eq!(convert(html), "A \nB\n\n- Both\n"); } #[test] diff --git a/crates/markkit/tests/limits.rs b/crates/markkit/tests/limits.rs index 742e396..b8e2d46 100644 --- a/crates/markkit/tests/limits.rs +++ b/crates/markkit/tests/limits.rs @@ -90,6 +90,22 @@ mod html { assert!(m.extract_html_sync(&html).is_ok()); } + #[test] + fn nested_list_rendering_respects_remaining_output_budget() { + let html = format!( + "
    • {}
      2. <tag>{}
    ", + "parent".repeat(8), + "code".repeat(64), + ); + let bounded = Markkit::builder().max_output_size(100).build(); + assert_eq!( + limit(bounded.extract_html_sync(&html)), + LimitKind::MaxOutputSize + ); + let roomy = Markkit::builder().max_output_size(1000).build(); + assert!(roomy.extract_html_sync(&html).unwrap().contains("")); + } + #[test] fn output_cap_stops_table_amplification() { // One wide row pads every other row: output ~ rows x cols. diff --git a/crates/markkit/tests/output_safety.rs b/crates/markkit/tests/output_safety.rs index feb0f60..d1800c2 100644 --- a/crates/markkit/tests/output_safety.rs +++ b/crates/markkit/tests/output_safety.rs @@ -7,7 +7,7 @@ //! inspects the resulting events. use markkit::MarkdownRenderer; -use markkit::ir::{Document, ListItem, Node, TableCell}; +use markkit::ir::{DefinitionItem, Document, ListItem, Node, TableCell}; use proptest::prelude::*; use pulldown_cmark::{Event, Options, Parser, Tag, TagEnd}; @@ -566,6 +566,57 @@ fn blocks_inside_heading_stay_on_one_line() { assert_eq!(visible_text(&md), "a b c ", "{md}"); } +#[test] +fn blockquote_heading_inline_blocks_cannot_expose_html() { + let md = render(vec![Node::BlockQuote { + source: None, + children: vec![Node::Heading { + source: None, + level: 6, + children: vec![ + Node::Link { + url: "alert(1)\n"; + let md = render(vec![Node::List { + ordered, + start: Some(10), + items: vec![ListItem::task( + vec![ + para(vec![]), + Node::CodeBlock { + language: Some("html".into()), + code: code.into(), + source: None, + }, + ], + checked, + )], + source: None, + }]); + assert_safe(&md); + assert!( + events(&md) + .iter() + .any(|e| matches!(e, Event::TaskListMarker(value) if *value == checked)), + "{md}" + ); + assert!( + events(&md) + .iter() + .any(|e| matches!(e, Event::Start(Tag::CodeBlock(_)))), + "{md}" + ); + assert_eq!(visible_text(&md), code, "{md} {:?}", events(&md)); + } + } +} + +#[test] +fn task_list_leading_nested_blocks_keep_code_safe() { + let code = || Node::CodeBlock { + language: Some("