From 27aeab48e14952d4d36460f9c475d81a7640cef2 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Sun, 27 Sep 2026 18:23:36 -0700 Subject: [PATCH] gatewayd: links: url_len and closes as their comments say M5b review, findings 1 and 2. url_len ended a clean reply's URL at any `<` outside parentheses, not at a `<` before white space, a bracket, `<`, `>` or the end; closes used a count of characters as a byte offset, so a closer indented with a no-break space did not close its fence. Both bodies, and fence_end's (an index replaced by `get`), are the reference's again. tests/links_review.rs pins both cases with the reference's output. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/gatewayd/src/links/inline.rs | 47 ++++++++++----------------- crates/gatewayd/src/links/mod.rs | 29 +++++------------ crates/gatewayd/tests/links_review.rs | 29 +++++++++++++++++ 3 files changed, 55 insertions(+), 50 deletions(-) create mode 100644 crates/gatewayd/tests/links_review.rs diff --git a/crates/gatewayd/src/links/inline.rs b/crates/gatewayd/src/links/inline.rs index 6b4d457..4dae8e3 100644 --- a/crates/gatewayd/src/links/inline.rs +++ b/crates/gatewayd/src/links/inline.rs @@ -439,41 +439,28 @@ fn scheme_less(rest: &str) -> bool { /// out: it may start an autolink); unless `whole`, up to any `<`, `>`, `|` or backtick too. Then /// less the characters of `TRAILING` at its end. `whole` is the most marked may read as the URL. fn url_len(rest: &str, whole: bool) -> usize { - let mut depth = 0; - let mut gt_seen = false; + let mut depth = 0usize; let mut end = rest.len(); for (i, c) in rest.char_indices() { - if c.is_whitespace() { + let next = rest.get(i + 1..).and_then(|after| after.chars().next()); + let open = c == '<' && next.is_none_or(|n| n.is_whitespace() || "()<>".contains(n)); + let part = if whole { + !open + } else { + !matches!(c, '<' | '>' | '|' | '`') + }; + if c.is_whitespace() || !part || (c == ')' && depth == 0) { end = i; break; } - if !whole && (c == '<' || c == '>' || c == '|' || c == '`') { - end = i; - break; - } - match c { - '(' => depth += 1, - ')' if depth == 0 => { - end = i; - break; - } - ')' => depth -= 1, - '<' if depth == 0 && !gt_seen => { - end = i; - break; - } - '>' => gt_seen = true, - _ => {} + if c == '(' { + depth += 1; + } else if c == ')' { + depth -= 1; } } - while let Some(last) = rest - .get(..end) - .and_then(|prefix| prefix.chars().next_back()) - { - if !TRAILING.contains(&last) { - break; - } - end -= last.len_utf8(); - } - end + rest.get(..end) + .unwrap_or_default() + .trim_end_matches(TRAILING) + .len() } diff --git a/crates/gatewayd/src/links/mod.rs b/crates/gatewayd/src/links/mod.rs index 7cd3fa3..2b19b80 100644 --- a/crates/gatewayd/src/links/mod.rs +++ b/crates/gatewayd/src/links/mod.rs @@ -217,18 +217,15 @@ fn nested_start(line: &str) -> bool { /// `None` (it leaves a list item in CommonMark, and the block with it). No closer: `None`, since /// marked reads an unclosed fence as code only at times. fn fence_end(lines: &[&str], at: usize) -> Option { - let (indent, run) = opener(lines[at])?; - let mut i = at + 1; - while let Some(line) = lines.get(i) { + let (indent, run) = opener(lines.get(at)?)?; + for (i, line) in lines.iter().enumerate().skip(at + 1) { if closes(line, run) { return Some(i); } - if !content(line).is_empty() - && content(line).chars().take_while(|&c| c == ' ').count() < indent - { + let text = content(line); + if !text.is_empty() && text.len() - text.trim_start_matches(' ').len() < indent { return None; } - i += 1; } None } @@ -263,22 +260,14 @@ fn opener(line: &str) -> Option<(usize, &str)> { /// come only backticks and `~`, then only spaces, tabs or no-break spaces. fn closes(line: &str, run: &str) -> bool { let text = content(line); - let leading = text - .chars() - .take_while(|&c| c == ' ' || c == '\u{a0}') - .count(); - if leading > 3 { + let rest = text.trim_start_matches([' ', '\u{a0}']); + if text.chars().count() - rest.chars().count() > 3 { return false; } - let Some(after) = text.get(leading..) else { + let Some(tail) = rest.strip_prefix(run) else { return false; }; - if !after.starts_with(run) { - return false; - } - let rest = after.get(run.len()..).unwrap_or(""); - let after_ticks = rest.trim_start_matches(['`', '~']); - after_ticks + tail.trim_end_matches([' ', '\t', '\u{a0}']) .chars() - .all(|c| c == ' ' || c == '\t' || c == '\u{a0}') + .all(|c| c == '`' || c == '~') } diff --git a/crates/gatewayd/tests/links_review.rs b/crates/gatewayd/tests/links_review.rs new file mode 100644 index 0000000..b86a93f --- /dev/null +++ b/crates/gatewayd/tests/links_review.rs @@ -0,0 +1,29 @@ +//! Two cases the M5b review found where `links::disarm` did not follow the comments of its +//! helpers: a `<` inside a clean reply's URL, and a fence closed by a line indented with a no-break +//! space (M5b review, implementer log). Each expected text is the reference's. + +use gatewayd::links::disarm; + +#[test] +fn a_url_runs_on_past_a_less_than_sign_that_starts_no_autolink() { + // `url_len`: only a `<` before white space, a bracket, `<`, `>` or the end stops the URL of a + // clean reply, so the image after it is wrapped with the URL, whole. + let text = "see https://a.example/x