mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-28 03:58:45 -06:00
find_urls case-folded a copy of the page text and then sliced the original characters with offsets taken from that copy. Case folding can change a string's length — U+0130 (İ) lowercases to "i" plus a combining dot — so the offsets drifted past the end of the source and `chars[i..j]` panicked. The CLI exited 101 before it could create the link; MCP reported an internal tool error but kept serving. Match the ASCII prefixes case-insensitively against the original characters instead, so every offset stays in the source's own index space, and use get() so no lookup can index past the end. A new test reproduces the reported panic on the old code (range start index 41 out of range for slice of length 39) and passes with the fix. pdfcraft-annot's tests, clippy and rustfmt are green. Fixes #816. Co-authored-by: Big Pickle <noreply@opencode.ai>
This commit is contained in:
+14
-12
@@ -299,27 +299,29 @@ pub fn remove_all(doc: &mut Document, pages: Option<&[usize]>) -> Result<usize,
|
||||
/// Find web addresses in text: `http(s)://…`, `www.…` and bare e-mail addresses become
|
||||
/// `mailto:`. Returns character ranges and the URI for each.
|
||||
pub fn find_urls(chars: &[char]) -> Vec<(std::ops::Range<usize>, String)> {
|
||||
let text: String = chars.iter().collect();
|
||||
let lower = text.to_lowercase();
|
||||
let lc: Vec<char> = lower.chars().collect();
|
||||
let mut out = Vec::new();
|
||||
let url_char = |c: char| !c.is_whitespace() && !"<>\"'()[]{}".contains(c);
|
||||
// Case-insensitive prefix test that never steps past the end. Comparing against the original
|
||||
// characters keeps every offset in `chars`'s own index space: case-folding a copy can change
|
||||
// its length (U+0130 becomes "i" plus a combining dot), so a folded copy must never supply
|
||||
// offsets into the source (that mismatch panicked; #816).
|
||||
let starts_with =
|
||||
|start: usize, pat: &str| pat.chars().enumerate().all(|(k, p)| chars.get(start + k).is_some_and(|c| c.eq_ignore_ascii_case(&p)));
|
||||
let mut out = Vec::new();
|
||||
let mut i = 0;
|
||||
while i < lc.len() {
|
||||
let at = |s: &str| lc[i..].iter().take(s.chars().count()).copied().eq(s.chars());
|
||||
let boundary = i == 0 || !lc[i - 1].is_alphanumeric();
|
||||
if boundary && (at("http://") || at("https://") || at("www.")) {
|
||||
while i < chars.len() {
|
||||
let boundary = i == 0 || chars.get(i - 1).is_some_and(|c| !c.is_alphanumeric());
|
||||
if boundary && (starts_with(i, "http://") || starts_with(i, "https://") || starts_with(i, "www.")) {
|
||||
let mut j = i;
|
||||
while j < chars.len() && url_char(chars[j]) {
|
||||
while chars.get(j).is_some_and(|c| url_char(*c)) {
|
||||
j += 1;
|
||||
}
|
||||
// Trailing punctuation belongs to the sentence.
|
||||
while j > i && ".,;:!?".contains(chars[j - 1]) {
|
||||
while j > i && chars.get(j - 1).is_some_and(|c| ".,;:!?".contains(*c)) {
|
||||
j -= 1;
|
||||
}
|
||||
let s: String = chars[i..j].iter().collect();
|
||||
let s: String = chars.get(i..j).map(|c| c.iter().collect()).unwrap_or_default();
|
||||
if s.len() > 6 && s.contains('.') {
|
||||
let uri = if s.to_lowercase().starts_with("www.") { format!("http://{s}") } else { s };
|
||||
let uri = if s.get(..4).is_some_and(|p| p.eq_ignore_ascii_case("www.")) { format!("http://{s}") } else { s };
|
||||
out.push((i..j, uri));
|
||||
i = j;
|
||||
continue;
|
||||
|
||||
@@ -537,6 +537,31 @@ fn urls_are_found_in_text() {
|
||||
assert_eq!(found, ["https://example.org/a?b=1", "http://www.rust-lang.org"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn urls_are_found_after_length_changing_lowercase() {
|
||||
// U+0130 (İ) lowercases to "i" plus a combining dot above, so a case-folded copy is longer
|
||||
// than the source. The old matcher took the address offset from that copy and sliced the
|
||||
// source with it, panicking at `chars[i..j]` (#816).
|
||||
let mut text = "\u{130}".repeat(20);
|
||||
text.push_str(" http://example.com");
|
||||
let chars: Vec<char> = text.chars().collect();
|
||||
let found = crate::links::find_urls(&chars);
|
||||
assert_eq!(found.len(), 1, "{found:?}");
|
||||
let (range, uri) = found.into_iter().next().unwrap();
|
||||
assert_eq!(uri, "http://example.com");
|
||||
assert_eq!(range, 21..39, "range is in source-character indices");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn url_scheme_and_www_match_case_insensitively() {
|
||||
let t: Vec<char> = "HTTP://Example.COM".chars().collect();
|
||||
let found: Vec<String> = crate::links::find_urls(&t).into_iter().map(|(_, u)| u).collect();
|
||||
assert_eq!(found, ["HTTP://Example.COM"]);
|
||||
let t: Vec<char> = "\u{130} www.Example.org".chars().collect();
|
||||
let found: Vec<String> = crate::links::find_urls(&t).into_iter().map(|(_, u)| u).collect();
|
||||
assert_eq!(found, ["http://www.Example.org"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn polygons_clouds_connected_lines_callouts_and_carets_are_drawn() {
|
||||
let mut doc = fixture();
|
||||
|
||||
Reference in New Issue
Block a user