From 378ad5eafc28928b13c02aff5f80b5235ecb0b46 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:19:42 +0800 Subject: [PATCH 1/3] M3: relative links resolve against the document's base URI; a link is active only in its QuadPoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR's five commits (base URI resolution, QuadPoints hit areas, their budget, the /Rect gate, zero-area quads, f64 containment), ported onto main, where inspect.rs, canvas.rs and the comments tool were split into modules: DocInfo::uri_base in inspect.rs, Link::quads and Link::contains in inspect/model.rs, the reading in inspect/core.rs and inspect/annotations.rs, their tests in inspect/tests.rs, and PageXform::link_contains in canvas/xform.rs. ISO 32000-2 §12.6.4.8 Table 211 (/URI /Base) and §12.5.6.5 Table 176 (/QuadPoints). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- Cargo.lock | 1 + crates/automation/src/links.rs | 35 ++- crates/automation/src/tools.rs | 2 +- crates/automation/tests/it/automation.rs | 46 +++ crates/render/Cargo.toml | 3 + crates/render/src/inspect.rs | 3 + crates/render/src/inspect/annotations.rs | 46 ++- crates/render/src/inspect/core.rs | 4 + crates/render/src/inspect/model.rs | 26 ++ crates/render/src/inspect/tests.rs | 140 ++++++++++ crates/render/src/lib.rs | 2 + crates/render/src/link_area.rs | 310 +++++++++++++++++++++ crates/ui-egui/src/canvas.rs | 4 +- crates/ui-egui/src/canvas/xform.rs | 13 + crates/ui-egui/src/tools/comments/input.rs | 2 +- crates/ui-egui/tests/it/document_links.rs | 58 ++++ parity/acrobat-features.toml | 4 +- 17 files changed, 681 insertions(+), 18 deletions(-) create mode 100644 crates/render/src/link_area.rs diff --git a/Cargo.lock b/Cargo.lock index cfcb9dc362..70ba94a1ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4604,6 +4604,7 @@ dependencies = [ "serde", "skrifa 0.44.0", "thiserror 2.0.21", + "url", ] [[package]] diff --git a/crates/automation/src/links.rs b/crates/automation/src/links.rs index 41fda9b7db..3344bd7bb5 100644 --- a/crates/automation/src/links.rs +++ b/crates/automation/src/links.rs @@ -65,7 +65,8 @@ impl Automation { pub(crate) fn link_list(&self, a: &Args) -> Result { let doc = self.doc(a)?; - let targets: std::collections::HashMap<(usize, usize), &LinkTarget> = doc.info.links.iter().map(|t| ((t.page, t.index), &t.target)).collect(); + let viewed: std::collections::HashMap<(usize, usize), &pdfcraft_render::Link> = + doc.info.links.iter().map(|t| ((t.page, t.index), t)).collect(); let items: Vec = doc .links() .iter() @@ -73,14 +74,42 @@ impl Automation { // The link's page may be one the viewer could not read (#2660): rect unchanged then. let rect = crate::view_rect(&doc.info, l.page, l.rect.map(|x| x as f32)); let mut v = json!({ "page": l.page + 1, "index": l.index + 1, "rect": rect, "visible": l.style.visible }); + let link = viewed.get(&(l.page, l.index)); + // Where the link is active when that isn't its whole rect: its /QuadPoints + // (ISO 32000-2 Table 176), as the bounding box of each quadrilateral. + if let Some(link) = link.filter(|k| !k.quads.is_empty()) { + let areas: Vec = link + .quads + .iter() + .map(|q| { + let (xs, ys) = ([q[0], q[2], q[4], q[6]], [q[1], q[3], q[5], q[7]]); + let fold = |v: [f32; 4], f: fn(f32, f32) -> f32, start: f32| v.into_iter().fold(start, f); + let box_ = [ + fold(xs, f32::min, f32::MAX), + fold(ys, f32::min, f32::MAX), + fold(xs, f32::max, f32::MIN), + fold(ys, f32::max, f32::MIN), + ]; + json!(crate::view_rect(&doc.info, l.page, box_)) + }) + .collect(); + v["active_areas"] = json!(areas); + } // Where an in-document link goes, as the viewer resolves it: named destinations // included, with the position on the page. - let target = targets.get(&(l.page, l.index)).and_then(|t| match t { + let target = link.and_then(|t| match &t.target { LinkTarget::Page(page, view) => Some((*page, view)), _ => None, }); match (&l.action, target) { - (LinkAction::Uri(u), _) => v["url"] = json!(u), + (LinkAction::Uri(u), _) => { + v["url"] = json!(u); + // A relative address, resolved against the catalog's /URI /Base + // (ISO 32000-2 Table 211): what the viewer offers to open. + if let Some(resolved) = pdfcraft_render::resolve_uri(doc.info.uri_base.as_deref(), u) { + v["resolved_url"] = json!(resolved); + } + } (action, Some((page, view))) => { v["to_page"] = json!(page + 1); if let Some(d) = doc.info.pages.get(page).and_then(|p| crate::destination(view, p)) { diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index c6c200cece..42ece3e010 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -685,7 +685,7 @@ pub fn tools() -> Vec { t("content_delete", "Delete added content", "Delete an added item (page, index from content_list). Undoable.") .destructive() .with(schema(json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "index": { "type": "integer", "minimum": 1 } }), &["doc", "page", "index"])), - t("link_list", "List links", "Every link: page, 1-based index (for link_edit/link_delete), rect (points from the top-left of the page), and where it goes (url or to_page).") + t("link_list", "List links", "Every link: page, 1-based index (for link_edit/link_delete), rect (points from the top-left of the page), and where it goes (url or to_page). A relative url also has resolved_url, the address resolved against the document's base URI (catalog /URI /Base); a link active only in its /QuadPoints has active_areas, one rect per quadrilateral.") .ro() .with(schema(json!({ "doc": doc() }), &["doc"])), t("link_open", "Open a linked PDF", "Follow a local GoToR link in PdfCraft: open the referenced PDF as a new document, respecting --root. page and index use link_list numbering. Named destinations are reported but not applied. Absolute file specifications require allow_absolute: true after reviewing the target; relative links need no opt-in.") diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 6a5765b911..8f350b930c 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -2049,6 +2049,52 @@ fn destinations_are_listed_with_their_position() { assert_eq!((links["links"][2]["url"].clone(), links["links"][2].get("destination")), (json!("https://example.com"), None)); } +/// A relative URI action resolves against the catalog's `/URI /Base` (ISO 32000-2 §12.6.4.8, +/// Table 211), and a link with `/QuadPoints` is active only there (§12.5.6.5, Table 176): both as +/// link_list and doc_info report them. +#[test] +fn links_resolve_relative_addresses_and_report_their_quad_points() { + let dir = workdir("link-base-quads"); + let objs: Vec<&str> = vec![ + "<< /Type /Catalog /Pages 2 0 R /URI << /Base (https://example.com/base/) >> >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>", + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R] >>", + "<< /Type /Annot /Subtype /Link /Rect [72 700 300 716] /Border [0 0 0] /A << /S /URI /URI (page.html) >> >>", + "<< /Type /Annot /Subtype /Link /Rect [72 670 300 686] /QuadPoints [72 670 120 670 120 686 72 686] /Border [0 0 0] /A << /S /URI /URI (https://example.org/q) >> >>", + "<< /Type /Annot /Subtype /Link /Rect [72 640 300 656] /Border [0 0 0] /A << /S /URI /URI (javascript:alert\\(1\\)) >> >>", + ]; + let mut pdf = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(pdf.len()); + pdf.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = pdf.len(); + pdf.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + pdf.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + pdf.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + std::fs::write(dir.join("links.pdf"), pdf).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "links.pdf" }))["doc"].as_u64().unwrap(); + let links = ok(&mut a, "link_list", json!({ "doc": doc })); + let [relative, quads, script] = [&links["links"][0], &links["links"][1], &links["links"][2]]; + assert_eq!( + (relative["url"].clone(), relative["resolved_url"].clone()), + (json!("page.html"), json!("https://example.com/base/page.html")), + "{links}" + ); + assert_eq!(relative.get("active_areas"), None, "{links}"); + // 792 − 686 = 106 points from the top: only the first word, not the whole rect. + assert_eq!(quads["active_areas"], json!([[72.0, 106.0, 120.0, 122.0]]), "{links}"); + assert_eq!(quads.get("resolved_url"), None, "an absolute address is not resolved"); + // A base never turns a script into a web address. + assert_eq!((script["url"].clone(), script.get("resolved_url")), (json!("javascript:alert(1)"), None), "{links}"); + let info = ok(&mut a, "doc_info", json!({ "doc": doc })); + assert_eq!(info["links"][0]["target"]["uri"], "https://example.com/base/page.html", "{info}"); +} + #[test] fn bookmarks_from_structure_through_tools() { let dir = workdir("bookmarks-structure"); diff --git a/crates/render/Cargo.toml b/crates/render/Cargo.toml index 28202a38b4..e0582ab9cd 100644 --- a/crates/render/Cargo.toml +++ b/crates/render/Cargo.toml @@ -21,6 +21,9 @@ log = { workspace = true } serde = { workspace = true } # Reads rendered pixel words as bytes; already in the tree through hayro. bytemuck = { version = "1" } +# Resolves relative URI actions against the catalog's /URI /Base, by the rules the browser uses; +# already in the tree (pdfcraft-engine checks document links with it). +url = "2.5" # lopdf (bootstrap) draws random bytes for encryption; on the web they come from the browser. [target.'cfg(target_arch = "wasm32")'.dependencies] diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index 0cf287b3f1..d4417a2c33 100644 --- a/crates/render/src/inspect.rs +++ b/crates/render/src/inspect.rs @@ -70,6 +70,9 @@ pub struct DocInfo { pub annotations: Vec, pub fields: Vec, pub links: Vec, + /// The catalog's `/URI /Base` (ISO 32000-2 Table 211), the base for relative URI actions, when + /// it is a string of at most [`crate::MAX_URI_BASE_BYTES`]. + pub uri_base: Option, pub layers: Vec, /// Radio-button layer groups (`/OCProperties /D /RBGroups`): turning one layer of a group on /// turns the others off. diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index 603aa88110..b48c8537a6 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -15,11 +15,13 @@ const MAX_REMOTE_DESTINATION_NAME_BYTES: usize = 1024; /// are not listed: real comments stay far below, many small ones sharing one long string do not. pub(super) const MAX_ANNOTATION_BYTES: usize = 64 << 20; -/// Web-address text the link listing, and name, description and embedded-file key text the -/// attachment listing, keep together. Many links or attachments can share one long string, which -/// each would otherwise copy. Past it, a link to a web address lists as a URI action without its -/// address, an attachment under its generic name without a description, and an embedded file whose -/// key does not fit is not listed. +/// Web-address text and `/QuadPoints` geometry the link listing, and name, description and +/// embedded-file key text the attachment listing, keep together. Many links or attachments can +/// share one long string or array, which each would otherwise copy. Past it, a link to a web +/// address lists as a URI action without its address (or keeps a relative address unresolved +/// against the document's base), a link is active over its whole `/Rect`, an attachment is listed +/// under its generic name without a description, and an embedded file whose key does not fit is +/// not listed. pub(super) const MAX_LINK_ATTACHMENT_TEXT_BYTES: usize = 16 << 20; /// The bytes a listed annotation keeps. @@ -101,7 +103,7 @@ impl<'a> Inspector<'a> { info.attachments.push(self.attachment(fs, AttachmentSource::Annotation { page, index }, text_budget, fallback)); } if subtype == "Link" { - if let Some(link) = self.link(page, index, d, text_budget) { + if let Some(link) = self.link(page, index, d, info.uri_base.as_deref(), text_budget) { if matches!(&link.target, LinkTarget::Other(s) if s == "JavaScript") { info.has_javascript = true; } @@ -192,7 +194,8 @@ impl<'a> Inspector<'a> { link_replies(&mut info.annotations); } - fn link(&self, page: usize, index: usize, d: &Dictionary, text_budget: &mut usize) -> Option { + /// `base` is the catalog's `/URI /Base`, which relative URI actions are resolved against. + fn link(&self, page: usize, index: usize, d: &Dictionary, base: Option<&str>, text_budget: &mut usize) -> Option { let rect = rect4(self.resolve(d.get(b"Rect").ok()?)); let target = if let Ok(dest) = d.get(b"Dest") { let (page, view) = self.dest(dest, 0)?; @@ -207,7 +210,15 @@ impl<'a> Inspector<'a> { Some("URI") => { let uri = a.get(b"URI").ok().and_then(|u| self.resolve(u).as_str().ok())?; match spend(uri, text_budget, |b| String::from_utf8_lossy(b).into_owned()) { - Some(uri) => LinkTarget::Uri(uri), + Some(uri) => match crate::resolve_uri(base, &uri) { + // The raw address is spent already. What the base adds is spent too, or + // the link keeps the raw address. + Some(resolved) if resolved.len().saturating_sub(uri.len()) <= *text_budget => { + *text_budget = text_budget.saturating_sub(resolved.len().saturating_sub(uri.len())); + LinkTarget::Uri(resolved) + } + _ => LinkTarget::Uri(uri), + }, None => LinkTarget::Other("URI".into()), } } @@ -218,7 +229,24 @@ impl<'a> Inspector<'a> { None => return None, } }; - Some(Link { page, index, rect, target }) + let quads = match d.get(b"QuadPoints").map(|o| self.resolve(o)) { + // Read no further than one value past the cap: a longer array isn't used. + Ok(Object::Array(q)) if q.len() <= crate::MAX_LINK_QUADS.saturating_mul(8) => { + let values: Vec> = q.iter().map(|o| self.resolve(o).as_float().ok()).collect(); + let quads = crate::link_quads(&values, rect); + // Many links can share one long array, which each would otherwise copy. Past the + // budget a link is active over its whole `/Rect`. + let bytes = std::mem::size_of_val(quads.as_slice()); + if bytes <= *text_budget { + *text_budget = text_budget.saturating_sub(bytes); + quads + } else { + Vec::new() + } + } + _ => Vec::new(), + }; + Some(Link { page, index, rect, quads, target }) } fn remote_go_to(&self, action: &Dictionary) -> Option { diff --git a/crates/render/src/inspect/core.rs b/crates/render/src/inspect/core.rs index c190dc565b..518f4f3792 100644 --- a/crates/render/src/inspect/core.rs +++ b/crates/render/src/inspect/core.rs @@ -207,6 +207,10 @@ impl<'a> Inspector<'a> { if let Some(first) = catalog.get(b"Outlines").ok().and_then(|o| self.dict(o)).and_then(|d| d.get(b"First").ok()) { (info.outline, info.outline_more) = self.outline_window(first); } + info.uri_base = catalog.get(b"URI").ok().and_then(|o| self.dict(o)).and_then(|d| match d.get(b"Base").map(|o| self.resolve(o)) { + Ok(Object::String(bytes, _)) if bytes.len() <= crate::MAX_URI_BASE_BYTES => std::str::from_utf8(bytes).ok().map(str::to_owned), + _ => None, + }); let mut text_budget = MAX_LINK_ATTACHMENT_TEXT_BYTES; self.annotations(info, &mut text_budget); self.transitions(info); diff --git a/crates/render/src/inspect/model.rs b/crates/render/src/inspect/model.rs index 490897b60f..6cfff7c0ab 100644 --- a/crates/render/src/inspect/model.rs +++ b/crates/render/src/inspect/model.rs @@ -196,13 +196,39 @@ pub struct Link { /// Zero-based position in the page's raw `/Annots` array. pub index: usize, pub rect: [f32; 4], + /// Where the link is active when that isn't the whole of `rect`: its `/QuadPoints`, 8 numbers + /// each in user space (ISO 32000-2 Table 176). Empty when the link is active over `rect` + /// ([`crate::link_quads`]). + pub quads: Vec<[f32; 8]>, pub target: LinkTarget, } +impl Link { + /// Whether the user-space point `(x, y)` activates the link: inside one of its quadrilaterals, + /// or inside its rectangle when it has none. + pub fn contains(&self, x: f32, y: f32) -> bool { + let [x0, y0, x1, y1] = self.rect; + if self.quads.is_empty() { + return x >= x0 && x <= x1 && y >= y0 && y <= y1; + } + // Every kept quadrilateral lies within `/Rect` give or take the slack `link_quads` + // accepts, so a point outside that is in none of them: skip the scan (up to + // `MAX_LINK_QUADS` per link, run per link on every pointer move). + let s = crate::link_area::RECT_SLACK; + if !(x >= x0 - s && x <= x1 + s && y >= y0 - s && y <= y1 + s) { + return false; + } + self.quads.iter().any(|q| crate::quad_contains(q, x, y)) + } +} + #[derive(Clone, Debug, PartialEq)] pub enum LinkTarget { /// A destination in this document: the 0-based page and where on it the view goes. Page(usize, DestView), + /// A URI action's address, resolved against the catalog's `/URI /Base` when it is a relative + /// reference ([`crate::resolve_uri`]); not yet checked, as the opener must + /// (`pdfcraft_engine::links::document_url`). Uri(String), /// A set-layer-visibility action (`SetOCGState`, ISO 32000-2 §12.6.4.13): each change in /// order, naming the layer by its optional content group. With `preserve_rb`, a layer turned diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index a5bfec474d..fe5468f461 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -946,6 +946,146 @@ fn a_named_attachment_leaves_its_contents_unspent() { assert!(matches!(&info.links[0].target, LinkTarget::Uri(u) if *u == uri)); } +#[test] +fn link_hit_test_rejects_points_outside_the_rect_before_scanning_quads() { + // A quadrilateral reaching past `/Rect` (hand-built: `link_quads` keeps none beyond the + // slack) shows whether the bounds gate runs before the quadrilateral scan. + let link = Link { + page: 0, + index: 0, + rect: [0.0, 0.0, 10.0, 10.0], + quads: vec![[0.0, 0.0, 50.0, 0.0, 50.0, 10.0, 0.0, 10.0]], + target: LinkTarget::Other("URI".into()), + }; + assert!(link.contains(5.0, 5.0), "inside the rect and the quadrilateral"); + assert!(link.contains(10.005, 5.0), "within the slack `link_quads` accepts"); + assert!(!link.contains(30.0, 5.0), "outside the rect: no quadrilateral is scanned"); + assert!(!link.contains(f32::NAN, 5.0)); +} + +#[test] +fn a_link_whose_quads_have_no_area_is_active_over_its_rect() { + let rect = [10.0, 10.0, 200.0, 30.0]; + let link = |values: &[f32]| Link { + page: 0, + index: 0, + rect, + quads: crate::link_quads(&values.iter().map(|v| Some(*v)).collect::>(), rect), + target: LinkTarget::Other("URI".into()), + }; + let flat = [10.0, 20.0, 60.0, 20.0, 60.0, 20.0, 10.0, 20.0]; + assert!(link(&flat).contains(100.0, 25.0), "no usable quad: the whole /Rect"); + let good = [10.0, 10.0, 60.0, 10.0, 60.0, 30.0, 10.0, 30.0]; + let mixed: Vec = flat.iter().chain(good.iter()).copied().collect(); + assert!(link(&mixed).contains(30.0, 25.0), "inside the usable quad"); + assert!(!link(&mixed).contains(100.0, 25.0), "the usable quad still gates the /Rect"); +} + +fn links_of(catalog_extra: &str, annots: &str) -> DocInfo { + let pdf = format!( + "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R {catalog_extra} >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Annots [{annots}] >> endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF" + ); + let doc = Document::load_mem(pdf.as_bytes()).expect("loads"); + let page = PageInfo { width: 300.0, height: 300.0, crop: [0.0, 0.0, 300.0, 300.0], rotation: 0, label: String::new() }; + let mut info = DocInfo { pages: vec![page], ..Default::default() }; + Inspector::new(&doc).fill(&mut info); + info +} + +fn uris(info: &DocInfo) -> Vec { + info.links.iter().map(|l| if let LinkTarget::Uri(u) = &l.target { u.clone() } else { format!("{:?}", l.target) }).collect() +} + +/// ISO 32000-2 §12.6.4.8, Table 211: a relative URI action is resolved against the catalog's +/// `/URI /Base`; absolute addresses, and a base that isn't a web address, change nothing. +#[test] +fn relative_uri_actions_resolve_against_the_catalog_base() { + let annots = "<< /Subtype /Link /Rect [0 0 10 10] /A << /S /URI /URI (page.html) >> >> \ + << /Subtype /Link /Rect [0 0 10 10] /A << /S /URI /URI (javascript:alert(1)) >> >> \ + << /Subtype /Link /Rect [0 0 10 10] /A << /S /URI /URI (https://example.org/abs) >> >>"; + let info = links_of("/URI << /Base (https://example.com/base/) >>", annots); + assert_eq!(info.uri_base.as_deref(), Some("https://example.com/base/")); + assert_eq!(uris(&info), ["https://example.com/base/page.html", "javascript:alert(1)", "https://example.org/abs"]); + // An indirect URI dictionary is followed too. + let pdf_indirect = links_of("/URI 4 0 R >> endobj\n4 0 obj << /Base (https://example.com/b/) >> endobj\n5 0 obj <<", annots); + assert_eq!(uris(&pdf_indirect).first().map(String::as_str), Some("https://example.com/b/page.html")); + // Without a base, or with one that isn't a web address, the address stays as written. + assert_eq!(uris(&links_of("", annots)).first().map(String::as_str), Some("page.html")); + for base in ["/URI << /Base (file:///C:/docs/) >>", "/URI << /Base /NotAString >>", "/URI 7", "/URI << /Base (javascript:x//) >>"] { + assert_eq!(uris(&links_of(base, annots)).first().map(String::as_str), Some("page.html"), "{base}"); + } +} + +/// ISO 32000-2 §12.5.6.5, Table 176: a link is active in its `/QuadPoints`, or over its +/// `/Rect` when they are missing, malformed or reach outside it. +#[test] +fn links_are_active_in_their_quad_points() { + let link = |quads: &str| format!("<< /Subtype /Link /Rect [10 10 200 30] {quads} /A << /S /URI /URI (https://example.org/) >> >>"); + let info = links_of("", &link("/QuadPoints [10 10 60 10 60 30 10 30]")); + let l = &info.links[0]; + assert_eq!(l.quads, vec![[10.0, 10.0, 60.0, 10.0, 60.0, 30.0, 10.0, 30.0]]); + assert!(l.contains(30.0, 20.0), "the first word"); + assert!(!l.contains(150.0, 20.0), "inside /Rect but outside the quad"); + for fallback in [ + "", + "/QuadPoints [10 10 60 10 60 30 10]", + "/QuadPoints [10 10 60 10 60 30 10 (x)]", + "/QuadPoints [10 10 260 10 260 30 10 30]", + "/QuadPoints 5", + ] { + let info = links_of("", &link(fallback)); + let l = &info.links[0]; + assert!(l.quads.is_empty(), "{fallback}"); + assert!(l.contains(150.0, 20.0) && !l.contains(250.0, 20.0), "{fallback}: active over /Rect"); + } + let many = "10 10 60 10 60 30 10 30 ".repeat(crate::MAX_LINK_QUADS + 1); + assert!(links_of("", &link(&format!("/QuadPoints [{many}]"))).links[0].quads.is_empty(), "too many quads"); +} + +/// Links sharing one long `/QuadPoints` array, or a relative address resolved against a long +/// `/URI /Base`, keep their copies only within `MAX_LINK_ATTACHMENT_TEXT_BYTES`, and every +/// link is still listed. +#[test] +fn links_sharing_quad_points_or_a_long_base_keep_them_within_a_budget() { + let page = PageInfo { width: 200.0, height: 200.0, crop: [0.0, 0.0, 200.0, 200.0], rotation: 0, label: String::new() }; + let fill = |catalog_extra: &str, links: &str, object5: &str| { + let pdf = format!( + "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R {catalog_extra} >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Annots [{links}] >> endobj\n\ + 5 0 obj {object5} endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF" + ); + let doc = Document::load_mem(pdf.as_bytes()).expect("loads"); + let mut info = DocInfo { pages: vec![page.clone()], ..Default::default() }; + Inspector::new(&doc).fill(&mut info); + info + }; + + let count = MAX_LINK_ATTACHMENT_TEXT_BYTES / (crate::MAX_LINK_QUADS * std::mem::size_of::<[f32; 8]>()) + 20; + let quads = format!("[{}]", "10 10 60 10 60 30 10 30 ".repeat(crate::MAX_LINK_QUADS)); + let links = "<< /Subtype /Link /Rect [0 0 100 100] /QuadPoints 5 0 R /A << /S /URI /URI (https://example.org/) >> >> ".repeat(count); + let info = fill("", &links, &quads); + assert_eq!(info.links.len(), count); + let kept: usize = info.links.iter().map(|l| std::mem::size_of_val(l.quads.as_slice())).sum(); + assert!(kept > 0 && kept <= MAX_LINK_ATTACHMENT_TEXT_BYTES, "{kept} quad bytes kept"); + assert!(info.links.iter().any(|l| l.quads.is_empty()), "past the budget a link is active over its /Rect"); + + let base = format!("https://example.com/{}/", "b".repeat(crate::MAX_URI_BASE_BYTES - 32)); + let count = MAX_LINK_ATTACHMENT_TEXT_BYTES / base.len() + 20; + let links = "<< /Subtype /Link /Rect [0 0 1 1] /A 5 0 R >> ".repeat(count); + let info = fill(&format!("/URI << /Base ({base}) >>"), &links, "<< /S /URI /URI (p.html) >>"); + assert_eq!(info.links.len(), count); + let addresses = uris(&info); + let kept: usize = addresses.iter().map(String::len).sum(); + assert!(kept <= MAX_LINK_ATTACHMENT_TEXT_BYTES, "{kept} address bytes kept"); + assert_eq!(addresses.first(), Some(&format!("{base}p.html"))); + assert!(addresses.iter().any(|u| u == "p.html"), "past the budget a link keeps its raw address"); +} + /// Embedded files whose name-tree keys share one long string keep it, as key and as name, only /// within `MAX_LINK_ATTACHMENT_TEXT_BYTES`. #[test] diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 954823ef09..d6cfe9ef60 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -9,6 +9,7 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] mod inspect; +mod link_area; mod pixels; mod raster; mod reading; @@ -20,6 +21,7 @@ pub use inspect::{ Annotation, Attachment, AttachmentSource, DestView, DocInfo, Field, FieldKind, FontInfo, Layer, LayerOp, Link, LinkTarget, OutlineItem, PageInfo, PageTransition, RemoteDestination, TransitionStyle, Xfa, attachment_data, check_pages, inspect, link_replies, pretty_date, thread_tops, }; +pub use link_area::{MAX_LINK_QUADS, MAX_URI_BASE_BYTES, link_quads, quad_contains, resolve_uri}; pub use pixels::Pixels; pub use raster::{ MAX_PIXELS, MAX_SIDE, PRINT_TAG, PageRenderer, RenderConfig, RenderPool, RenderRequest, RenderStats, RenderWarning, RenderedPage, RequestKind, diff --git a/crates/render/src/link_area.rs b/crates/render/src/link_area.rs new file mode 100644 index 0000000000..de0c1839a8 --- /dev/null +++ b/crates/render/src/link_area.rs @@ -0,0 +1,310 @@ +//! Where a link annotation is active, and where its URI action goes. +//! +//! - ISO 32000-2 §12.5.6.5, Table 176: a link's `/QuadPoints` are "the region in which the link +//! should be activated"; when they are absent, not understood, or any coordinate lies outside +//! `/Rect`, the activation region is `/Rect`. +//! - ISO 32000-2 §12.6.4.8, Table 211: the catalog's `/URI /Base` is "the base URI that shall be +//! used in resolving relative URI references" in URI actions. + +/// The most quadrilaterals a link's `/QuadPoints` may give before they are not used (the link is +/// then active over its `/Rect`, as for an array that isn't understood). +pub const MAX_LINK_QUADS: usize = 1024; + +/// The longest `/URI /Base` that is used. A longer base would only produce an address the link +/// check refuses as too long, so it is ignored. +pub const MAX_URI_BASE_BYTES: usize = 8192; + +/// How far (in points) a quadrilateral's corner may lie outside `/Rect` and still count as inside: +/// rounding in the writer, not a different region. +pub(crate) const RECT_SLACK: f32 = 0.01; + +/// A link's quadrilaterals from its `/QuadPoints` values (`None` for an entry that isn't a +/// number), or an empty list when the link is active over its whole `/Rect`: no values, a count +/// that isn't a multiple of 8, more than [`MAX_LINK_QUADS`], a value that isn't a finite number, +/// a corner outside `rect` (normalized, `[x0, y0, x1, y1]`), or no quadrilateral that encloses +/// an area. +/// +/// A quadrilateral no thicker than [`RECT_SLACK`] (its corners on one line, give or take +/// rounding) is left out: it adds nothing to the region the others make, and kept alone it would +/// leave the link active nowhere. When every quadrilateral is like that, the array is not one +/// that can be used and the link is active over `/Rect`. +pub fn link_quads(values: &[Option], rect: [f32; 4]) -> Vec<[f32; 8]> { + if values.is_empty() || !values.len().is_multiple_of(8) || values.len() / 8 > MAX_LINK_QUADS { + return Vec::new(); + } + let mut numbers = Vec::with_capacity(values.len()); + for v in values { + match v { + Some(n) if n.is_finite() => numbers.push(*n), + _ => return Vec::new(), + } + } + let [x0, y0, x1, y1] = rect; + let inside = numbers + .as_chunks::<2>() + .0 + .iter() + .all(|[x, y]| *x >= x0 - RECT_SLACK && *x <= x1 + RECT_SLACK && *y >= y0 - RECT_SLACK && *y <= y1 + RECT_SLACK); + if !inside { + return Vec::new(); + } + numbers.as_chunks::<8>().0.iter().filter(|q| encloses_area(q)).copied().collect() +} + +/// Whether the quadrilateral `q` (`x1 y1 … x4 y4`, finite) is thicker than [`RECT_SLACK`]: one of +/// the four triangles its corners make (whose union [`quad_contains`] tests) is more than that +/// high over its longest side. Worked in `f64`, where products of finite `f32` values neither +/// overflow nor become NaN. +fn encloses_area(q: &[f32; 8]) -> bool { + let p = [[q[0], q[1]], [q[2], q[3]], [q[4], q[5]], [q[6], q[7]]].map(|[x, y]| [f64::from(x), f64::from(y)]); + let len = |u: [f64; 2], v: [f64; 2]| (v[0] - u[0]).hypot(v[1] - u[1]); + [[0, 1, 2], [0, 1, 3], [0, 2, 3], [1, 2, 3]].iter().any(|&[a, b, c]| match (p.get(a), p.get(b), p.get(c)) { + (Some(&a), Some(&b), Some(&c)) => { + // Twice the triangle's area is its longest side times its height over that side. + let twice_area = ((b[0] - a[0]) * (c[1] - a[1]) - (b[1] - a[1]) * (c[0] - a[0])).abs(); + let longest = len(a, b).max(len(b, c)).max(len(c, a)); + twice_area > f64::from(RECT_SLACK) * longest + } + _ => false, + }) +} + +/// Whether the point `(x, y)` lies in the quadrilateral `q` (`x1 y1 … x4 y4`), edges included. +/// Writers disagree on the corners' order (the specification says counterclockwise, text-markup +/// writers often go top-left, top-right, bottom-left, bottom-right), so the quadrilateral is taken +/// as the area its four corners enclose: the union of the four triangles they make. +pub fn quad_contains(q: &[f32; 8], x: f32, y: f32) -> bool { + if !(x.is_finite() && y.is_finite()) { + return false; + } + let p = [[q[0], q[1]], [q[2], q[3]], [q[4], q[5]], [q[6], q[7]]]; + [[0, 1, 2], [0, 1, 3], [0, 2, 3], [1, 2, 3]].iter().any(|&[a, b, c]| match (p.get(a), p.get(b), p.get(c)) { + (Some(a), Some(b), Some(c)) => triangle_contains(*a, *b, *c, [x, y]), + _ => false, + }) +} + +/// Worked in `f64`, like [`encloses_area`]: in `f32` the cross products of far-apart finite +/// corners overflow to infinity and their differences to NaN, which no sign test rejects. +fn triangle_contains(a: [f32; 2], b: [f32; 2], c: [f32; 2], p: [f32; 2]) -> bool { + let [a, b, c, p] = [a, b, c, p].map(|[x, y]| [f64::from(x), f64::from(y)]); + let cross = |o: [f64; 2], u: [f64; 2], v: [f64; 2]| (u[0] - o[0]) * (v[1] - o[1]) - (u[1] - o[1]) * (v[0] - o[0]); + // A triangle with no area holds no point (it would otherwise hold its whole line). + if cross(a, b, c) == 0.0 { + return false; + } + let (d1, d2, d3) = (cross(a, b, p), cross(b, c, p), cross(c, a, p)); + let negative = d1 < 0.0 || d2 < 0.0 || d3 < 0.0; + let positive = d1 > 0.0 || d2 > 0.0 || d3 > 0.0; + !(negative && positive) +} + +/// Whether `s` begins with an RFC 3986 scheme (`ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) ":"`), +/// so it is an absolute URI rather than a relative reference. +fn has_scheme(s: &str) -> bool { + s.split_once(':').is_some_and(|(scheme, _)| { + scheme.chars().next().is_some_and(|c| c.is_ascii_alphabetic()) + && scheme.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')) + }) +} + +/// The address a URI action goes to when the document's catalog gives `base` (`/URI /Base`): +/// a relative reference (`page.html`, `../a/`, `?q=1`, `//host/x`) resolved against it by +/// RFC 3986 §5 (the WHATWG URL rules, which the browser applies too). `None` when the address is +/// used as written: it already has a scheme (`https:`, `mailto:`, `javascript:`…), there is no +/// usable base, or the result wouldn't be a web address. +/// +/// Only a web (`http`, `https`) base is used, and the result must keep the base's scheme, so a +/// base never turns a relative link into a `file:`, `javascript:` or other address. An address +/// with control characters or whitespace other than a plain space is left as written, since the +/// URL rules would drop them and read what is left (`java\nscript:`) differently. Either way the +/// address still goes through the same check and confirmation as any other link. +pub fn resolve_uri(base: Option<&str>, raw: &str) -> Option { + let reference = raw.trim(); + let base = base?.trim(); + if reference.is_empty() || base.len() > MAX_URI_BASE_BYTES || has_scheme(reference) { + return None; + } + let hidden = |s: &str| s.chars().any(|c| c.is_control() || (c.is_whitespace() && c != ' ')); + // The base must name its host as written (`https://host/…`): the URL rules would also accept + // `https:///host/` or `https:host/`, which no link check reads as a web address. + let names_host = base.split_once("://").is_some_and(|(_, rest)| !rest.starts_with(['/', '\\'])); + if hidden(reference) || hidden(base) || !names_host { + return None; + } + let base = url::Url::parse(base).ok()?; + if !matches!(base.scheme(), "http" | "https") || base.host().is_none() { + return None; + } + let resolved = base.join(reference).ok()?; + (resolved.scheme() == base.scheme() && resolved.host().is_some()).then(|| resolved.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + const BASE: &str = "https://example.com/base/"; + + #[test] + fn relative_references_resolve_against_the_base() { + let r = |raw: &str| resolve_uri(Some(BASE), raw); + assert_eq!(r("page.html").as_deref(), Some("https://example.com/base/page.html")); + assert_eq!(r(" page.html ").as_deref(), Some("https://example.com/base/page.html"), "trimmed like any link"); + assert_eq!(r("../up.html").as_deref(), Some("https://example.com/up.html")); + assert_eq!(r("/root.html").as_deref(), Some("https://example.com/root.html")); + assert_eq!(r("?q=1").as_deref(), Some("https://example.com/base/?q=1")); + assert_eq!(r("#part").as_deref(), Some("https://example.com/base/#part")); + assert_eq!(r("a/b:c").as_deref(), Some("https://example.com/base/a/b:c"), "a colon after a slash is no scheme"); + // A network-path reference names its own host; the confirmation shows that host. + assert_eq!(r("//other.example/x").as_deref(), Some("https://other.example/x")); + assert_eq!(resolve_uri(Some("http://example.com/dir/file.pdf"), "next.pdf").as_deref(), Some("http://example.com/dir/next.pdf")); + } + + #[test] + fn absolute_addresses_and_missing_bases_are_used_as_written() { + for raw in ["https://example.org/x", "HTTP://example.org", "mailto:a@example.org", "javascript:alert(1)", "file:///etc/passwd", "C:\\x.pdf"] { + assert_eq!(resolve_uri(Some(BASE), raw), None, "{raw}"); + } + assert_eq!(resolve_uri(None, "page.html"), None); + assert_eq!(resolve_uri(Some(BASE), ""), None); + assert_eq!(resolve_uri(Some(BASE), " "), None); + } + + #[test] + fn a_base_that_is_not_a_web_address_is_not_used() { + for base in [ + "file:///C:/docs/", + "javascript:alert(1)//", + "data:text/html,x/", + "mailto:a@example.org", + "smb://server/share/", + "relative/base/", + "", + "https://", + "https:///nohost/", + "https:nohost/", + "https:\\\\nohost/", + "https://exa\nmple.com/", + ] { + assert_eq!(resolve_uri(Some(base), "page.html"), None, "{base:?}"); + } + let long = format!("https://example.com/{}/", "a".repeat(MAX_URI_BASE_BYTES)); + assert_eq!(resolve_uri(Some(&long), "page.html"), None); + } + + #[test] + fn hidden_characters_never_make_a_scheme() { + // The URL rules drop tabs and newlines, which would read these as `javascript:`. + for raw in ["java\nscript:alert(1)", "java\tscript:alert(1)", "\u{0}javascript:alert(1)", "page\u{2028}.html"] { + assert_eq!(resolve_uri(Some(BASE), raw), None, "{raw:?}"); + } + } + + #[test] + fn quads_inside_the_rect_are_used() { + let rect = [10.0, 10.0, 200.0, 30.0]; + let q = [10.0, 10.0, 60.0, 10.0, 60.0, 30.0, 10.0, 30.0].map(Some); + assert_eq!(link_quads(&q, rect), vec![[10.0, 10.0, 60.0, 10.0, 60.0, 30.0, 10.0, 30.0]]); + let two: Vec> = q.iter().chain(q.iter()).copied().collect(); + assert_eq!(link_quads(&two, rect).len(), 2); + } + + #[test] + fn quads_that_are_not_understood_fall_back_to_the_rect() { + let rect = [10.0, 10.0, 200.0, 30.0]; + let good = [10.0, 10.0, 60.0, 10.0, 60.0, 30.0, 10.0, 30.0]; + assert!(link_quads(&[], rect).is_empty()); + assert!(link_quads(&good[..7].iter().copied().map(Some).collect::>(), rect).is_empty(), "not a multiple of 8"); + let mut odd: Vec> = good.map(Some).to_vec(); + odd.push(Some(1.0)); + assert!(link_quads(&odd, rect).is_empty()); + let mut not_number = good.map(Some); + not_number[3] = None; + assert!(link_quads(¬_number, rect).is_empty()); + let mut nan = good.map(Some); + nan[0] = Some(f32::NAN); + assert!(link_quads(&nan, rect).is_empty()); + // A /Rect too large for f32 would hold an infinite corner. + let mut infinite = good.map(Some); + infinite[2] = Some(f32::INFINITY); + assert!(link_quads(&infinite, [f32::NEG_INFINITY, f32::NEG_INFINITY, f32::INFINITY, f32::INFINITY]).is_empty()); + let mut outside = good.map(Some); + outside[2] = Some(250.0); + assert!(link_quads(&outside, rect).is_empty(), "a corner outside /Rect"); + let mut below = good.map(Some); + below[1] = Some(9.0); + assert!(link_quads(&below, rect).is_empty(), "a corner below /Rect"); + let too_many: Vec> = std::iter::repeat_n(good.map(Some), MAX_LINK_QUADS + 1).flatten().collect(); + assert!(link_quads(&too_many, rect).is_empty()); + let most: Vec> = std::iter::repeat_n(good.map(Some), MAX_LINK_QUADS).flatten().collect(); + assert_eq!(link_quads(&most, rect).len(), MAX_LINK_QUADS); + } + + #[test] + fn quads_with_no_area_are_left_out() { + let rect = [10.0, 10.0, 200.0, 30.0]; + let good = [10.0, 10.0, 60.0, 10.0, 60.0, 30.0, 10.0, 30.0]; + // Corners on one line, all at one point, and a sliver thinner than the slack. + let line = [10.0, 20.0, 60.0, 20.0, 60.0, 20.0, 10.0, 20.0]; + let point = [30.0, 20.0, 30.0, 20.0, 30.0, 20.0, 30.0, 20.0]; + let sliver = [10.0, 20.0, 60.0, 20.0, 60.0, 20.005, 10.0, 20.005]; + for flat in [line, point, sliver] { + assert!(link_quads(&flat.map(Some), rect).is_empty(), "{flat:?} alone: active over /Rect"); + let mixed: Vec> = flat.iter().chain(good.iter()).map(|v| Some(*v)).collect(); + assert_eq!(link_quads(&mixed, rect), vec![good], "{flat:?} beside a usable quad"); + } + // Thicker than the slack is a region, however thin. + let thin = [10.0, 20.0, 60.0, 20.0, 60.0, 20.05, 10.0, 20.05]; + assert_eq!(link_quads(&thin.map(Some), rect), vec![thin]); + // A triangle (two corners shared) still encloses an area. + let triangle = [10.0, 10.0, 60.0, 10.0, 60.0, 30.0, 60.0, 30.0]; + assert_eq!(link_quads(&triangle.map(Some), rect), vec![triangle]); + // The largest finite values neither overflow nor turn to NaN. + let all = [f32::MIN, f32::MIN, f32::MAX, f32::MAX]; + let huge = [f32::MIN, f32::MIN, f32::MAX, f32::MIN, f32::MAX, f32::MAX, f32::MIN, f32::MAX]; + assert_eq!(link_quads(&huge.map(Some), all), vec![huge]); + let huge_line = [f32::MIN, f32::MIN, f32::MAX, f32::MAX, f32::MAX, f32::MAX, f32::MIN, f32::MIN]; + assert!(link_quads(&huge_line.map(Some), all).is_empty()); + } + + #[test] + fn a_quad_holds_its_area_in_any_corner_order() { + // Counterclockwise from the bottom left, as the specification writes them. + let ccw = [10.0, 10.0, 60.0, 10.0, 60.0, 30.0, 10.0, 30.0]; + // Top-left, top-right, bottom-left, bottom-right, as text-markup writers often do. + let z = [10.0, 30.0, 60.0, 30.0, 10.0, 10.0, 60.0, 10.0]; + for q in [ccw, z] { + assert!(quad_contains(&q, 30.0, 20.0)); + assert!(quad_contains(&q, 10.0, 10.0), "corners count"); + assert!(quad_contains(&q, 60.0, 20.0), "edges count"); + assert!(!quad_contains(&q, 100.0, 20.0), "right of the quad"); + assert!(!quad_contains(&q, 30.0, 31.0), "above the quad"); + assert!(!quad_contains(&q, 9.0, 20.0), "left of the quad"); + assert!(!quad_contains(&q, 30.0, 9.0), "below the quad"); + } + // A tilted quad: the corner of its bounding box is outside it. + let diamond = [50.0, 0.0, 100.0, 50.0, 50.0, 100.0, 0.0, 50.0]; + assert!(quad_contains(&diamond, 50.0, 50.0)); + assert!(!quad_contains(&diamond, 5.0, 5.0)); + // A quad with no area holds nothing. + let flat = [0.0, 0.0, 10.0, 0.0, 20.0, 0.0, 30.0, 0.0]; + assert!(!quad_contains(&flat, 5.0, 0.0)); + assert!(!quad_contains(&ccw, f32::NAN, 20.0), "a point that isn't a number is nowhere"); + } + + #[test] + fn a_quad_with_extreme_corners_holds_only_its_area() { + // In f32 the cross products overflow (inf, and inf - inf = NaN), which passed every sign + // test and put (1.5e20, 1.5e20), outside the triangle, inside it. + let q = [0.0, 0.0, 2e20, 0.0, 0.0, 2e20, 0.0, 0.0]; + assert_eq!(link_quads(&q.map(Some), [0.0, 0.0, 3e20, 3e20]), vec![q], "kept by the area check"); + assert!(!quad_contains(&q, 1.5e20, 1.5e20), "beyond the long edge"); + assert!(!quad_contains(&q, 3e20, 1.0), "right of the triangle"); + assert!(quad_contains(&q, 0.5e20, 0.5e20)); + assert!(quad_contains(&q, 1.0, 1.0)); + let max = [0.0, 0.0, f32::MAX, 0.0, 0.0, f32::MAX, 0.0, 0.0]; + assert!(!quad_contains(&max, f32::MAX, f32::MAX)); + assert!(quad_contains(&max, 1.0, 1.0)); + } +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 8b280ba75d..73c0dda9e0 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1548,7 +1548,7 @@ fn text_pointer_input(ui: &mut egui::Ui, view: &mut DocView, text: &PageText, po let TextPointer { page, xf, rect, info, tool, resp, ptr, pointer: screen_pointer } = pointer; let (vx, vy) = xf.screen_to_view(screen_pointer); let over_text = text.glyphs.iter().any(|glyph| vx >= glyph.rect[0] && vx <= glyph.rect[2] && vy >= glyph.rect[1] && vy <= glyph.rect[3]); - let over_link = info.links.iter().any(|link| link.page == page && xf.user_rect(info, page, link.rect).contains(screen_pointer)); + let over_link = info.links.iter().any(|link| link.page == page && xf.link_contains(info, page, link, screen_pointer)); let column = tool == QuickTool::ColumnSelect || ui.input(|input| input.modifiers.alt); let gesture = TextGesture { page, xf, rect, tool, ptr, resp, pointer: screen_pointer, vx, vy, over_text, over_link, column }; update_text_cursor(view, &gesture); @@ -1900,7 +1900,7 @@ fn draw_page(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_>, i /// The link under `p` on `page`: the hand, and where a click goes. Only the Select tool follows /// links and the Link tool edits them (`Layer::reached_by`). fn link_hit(info: &DocInfo, xf: &PageXform, page: usize, p: Pos2) -> Option { - let (index, l) = info.links.iter().enumerate().rfind(|(_, l)| l.page == page && xf.user_rect(info, page, l.rect).contains(p))?; + let (index, l) = info.links.iter().enumerate().rfind(|(_, l)| l.page == page && xf.link_contains(info, page, l, p))?; let label = match &l.target { LinkTarget::Page(n, _) => crate::i18n::fmt(tl!("Go to page {p}"), &[("p", info.pages.get(*n).map(|p| p.label.as_str()).unwrap_or("?"))]), LinkTarget::Uri(u) => u.clone(), diff --git a/crates/ui-egui/src/canvas/xform.rs b/crates/ui-egui/src/canvas/xform.rs index 91a3739d62..312eb1b121 100644 --- a/crates/ui-egui/src/canvas/xform.rs +++ b/crates/ui-egui/src/canvas/xform.rs @@ -128,6 +128,19 @@ impl PageXform { Rect::from_two_pos(self.norm_to_screen(a.0, a.1), self.norm_to_screen(b.0, b.1)) } + /// Whether the screen point `p` activates link `l` on `page`: inside one of its + /// `/QuadPoints` quadrilaterals, or its rectangle when it has none (ISO 32000-2 Table 176). + pub fn link_contains(&self, info: &DocInfo, page: usize, l: &pdfcraft_render::Link, p: Pos2) -> bool { + if l.quads.is_empty() { + return self.user_rect(info, page, l.rect).contains(p); + } + // The inverse of `user_rect`: screen → the displayed page's fraction → user space. + let Some(pg) = info.pages.get(page) else { return false }; + let (u, v) = self.screen_to_norm(p); + let [x, y] = pg.view_to_user(u * pg.width, v * pg.height); + l.contains(x, y) + } + /// This transform resized to a raster of `px` device pixels (width, height before the view /// rotation), so each of its texels covers one screen pixel. The rectangle's corner must /// already be on a whole pixel ([`snap_to_pixels`]). diff --git a/crates/ui-egui/src/tools/comments/input.rs b/crates/ui-egui/src/tools/comments/input.rs index 5e74fdf56e..0c1a0317ae 100644 --- a/crates/ui-egui/src/tools/comments/input.rs +++ b/crates/ui-egui/src/tools/comments/input.rs @@ -132,7 +132,7 @@ fn draw_grabs(cx: &PageCx<'_>, view: &DocView) -> bool { /// Whether `p` is on one of this page's links. fn on_link(cx: &PageCx<'_>, p: Pos2) -> bool { - cx.info.links.iter().any(|link| link.page == cx.page && cx.xf.user_rect(cx.info, cx.page, link.rect).contains(p)) + cx.info.links.iter().any(|link| link.page == cx.page && cx.xf.link_contains(cx.info, cx.page, link, p)) } /// The tip for the comment under the pointer, with any tool, beside that tool's own cursor: Acrobat diff --git a/crates/ui-egui/tests/it/document_links.rs b/crates/ui-egui/tests/it/document_links.rs index 8405446df2..d50020de67 100644 --- a/crates/ui-egui/tests/it/document_links.rs +++ b/crates/ui-egui/tests/it/document_links.rs @@ -760,3 +760,61 @@ fn highlighted_link_can_be_requested_again_after_cancel() { assert_eq!(h.state().last_opened_url, None); } } + +/// One 300×400 page with two links (ISO 32000-2 §12.5.6.5, §12.6.4.8): at y 300..350 a relative +/// address the catalog's `/URI /Base` resolves, and at y 200..250 a link whose `/Rect` spans the +/// page but whose `/QuadPoints` cover only x 50..120. +fn base_and_quads_fixture() -> Vec { + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /URI << /Base (https://example.org/base/) >> >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R] >>", + "<< /Type /Annot /Subtype /Link /Rect [50 300 250 350] /Border [0 0 0] /A << /S /URI /URI (page.html) >> >>", + "<< /Type /Annot /Subtype /Link /Rect [50 200 250 250] /QuadPoints [50 200 120 200 120 250 50 250] /Border [0 0 0] /A << /S /URI /URI (https://example.org/first-word) >> >>", + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offs = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offs.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let x = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offs { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{x}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +/// A relative link opens at the address the document's base gives it, through the usual +/// confirmation; a link is active only in its `/QuadPoints`, not over the rest of its `/Rect`. +#[test] +fn relative_links_use_the_base_and_quad_points_limit_where_a_link_is_active() { + let _gpu = gpu(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("base.pdf", None, base_and_quads_fixture()).unwrap(); + app.set_option("left", "closed").unwrap(); + app.set_option("panel", "none").unwrap(); + app.set_option("zoom", "150").unwrap(); + app + }); + h.run_steps(8); + click_link(&mut h); + assert_eq!(h.state().pending_link, pending("https://example.org/base/page.html", LinkOrigin::Link)); + h.get_by_label("Cancel").click(); + h.run_steps(4); + let point = |h: &Harness<'static, PdfCraftApp>, x: f32, y: f32| { + let r = h.state().views[0].page_screen_rect(0).expect("page 1 on screen"); + egui::pos2(r.min.x + r.width() * (x / 300.0), r.min.y + r.height() * (1.0 - y / 400.0)) + }; + // Inside /Rect, outside the quadrilateral: nothing. + let away = point(&h, 200.0, 225.0); + click_at(&mut h, away); + assert_eq!(h.state().pending_link, None, "the rest of /Rect is not the link"); + let on = point(&h, 80.0, 225.0); + click_at(&mut h, on); + assert_eq!(h.state().pending_link, pending("https://example.org/first-word", LinkOrigin::Link)); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 1404247cd7..f5dde406b9 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1190,8 +1190,8 @@ milestone = "M3" status = "shipped" commands = [] tools = ["link_open"] -evidence = ["crates/ui-egui/tests/it/view.rs::clicking_a_link_goes_to_its_destination", "crates/ui-egui/tests/it/view.rs::link_destinations_scroll_to_their_position", "crates/ui-egui/tests/it/view.rs::a_link_without_a_position_keeps_the_place_on_the_next_page", "crates/ui-egui/src/canvas/tests.rs::a_null_xyz_coordinate_keeps_the_readers_place_on_the_new_page", "crates/render/src/inspect/tests.rs::bookmarks_and_links_keep_their_destination_view", "crates/render/src/inspect/tests.rs::links_read_bounded_local_remote_go_to_actions", "crates/automation/tests/it/automation.rs::local_gotor_link_open_applies_destinations_and_honors_root", "crates/ui-egui/tests/it/document_links.rs::a_local_gotor_link_opens_in_a_tab_and_applies_its_destination", "crates/ui-egui/tests/it/document_links.rs::an_absolute_gotor_waits_for_consent_and_keeps_its_destination_across_tabs", "crates/ui-egui/tests/it/document_links.rs::cancelling_an_absolute_gotor_by_button_escape_or_dismissal_changes_nothing", "crates/automation/tests/it/automation.rs::absolute_gotor_consent_does_not_bypass_validation_or_passwords", "crates/render/src/inspect/tests.rs::launch_actions_that_name_a_pdf_open_it_like_a_remote_link", "crates/automation/tests/it/automation.rs::a_launch_link_to_a_pdf_opens_it_like_a_remote_link"] -notes = "GoTo and URI links plus local-PDF GoToR links: source-relative or local absolute /F paths, /UF preferred in file-spec dictionaries, explicit numeric page/view destinations, and internal tabs. Absolute local GoToR targets require desktop Allow/Cancel confirmation or an explicit allow_absolute opt-in in link_open; relative links open normally. link_open honors --root. Named remote destinations open at default view with a warning; /NewWindow is a tab hint. A Launch action whose /F names a .pdf file (and has no /Win launch parameters) opens that PDF the same way, backslash separators read as / (#4168); URL/network file systems, GoToE, other Launch actions and other actions are not executed. Same-document destinations retain their supported /XYZ, /Fit, /FitH, /FitV and /FitR views; /FitB* is read as whole-page fit. A null /XYZ left or top keeps the reader's place, as ISO 32000-2 §12.3.2.2 says and Acrobat Reader was observed to do (synthetic fixture): the user-space point at the window's top-left stays there on the target page at the same zoom, along each axis the page overflows; CAD exports (WSCAD) write every cross-reference this way. Null /FitH top and /FitV left still go to the page edge." +evidence = ["crates/ui-egui/tests/it/view.rs::clicking_a_link_goes_to_its_destination", "crates/ui-egui/tests/it/view.rs::link_destinations_scroll_to_their_position", "crates/ui-egui/tests/it/view.rs::a_link_without_a_position_keeps_the_place_on_the_next_page", "crates/ui-egui/src/canvas/tests.rs::a_null_xyz_coordinate_keeps_the_readers_place_on_the_new_page", "crates/render/src/inspect/tests.rs::bookmarks_and_links_keep_their_destination_view", "crates/render/src/inspect/tests.rs::links_read_bounded_local_remote_go_to_actions", "crates/automation/tests/it/automation.rs::local_gotor_link_open_applies_destinations_and_honors_root", "crates/ui-egui/tests/it/document_links.rs::a_local_gotor_link_opens_in_a_tab_and_applies_its_destination", "crates/ui-egui/tests/it/document_links.rs::an_absolute_gotor_waits_for_consent_and_keeps_its_destination_across_tabs", "crates/ui-egui/tests/it/document_links.rs::cancelling_an_absolute_gotor_by_button_escape_or_dismissal_changes_nothing", "crates/automation/tests/it/automation.rs::absolute_gotor_consent_does_not_bypass_validation_or_passwords", "crates/render/src/inspect/tests.rs::launch_actions_that_name_a_pdf_open_it_like_a_remote_link", "crates/automation/tests/it/automation.rs::a_launch_link_to_a_pdf_opens_it_like_a_remote_link", "crates/render/src/inspect/tests.rs::relative_uri_actions_resolve_against_the_catalog_base", "crates/render/src/inspect/tests.rs::links_are_active_in_their_quad_points", "crates/automation/tests/it/automation.rs::links_resolve_relative_addresses_and_report_their_quad_points"] +notes = "A relative URI action resolves against the catalog's /URI /Base (ISO 32000-2 Table 211) when that base is an http(s) address, then goes through the usual check and confirmation; without a base, a relative address is still refused (resolving against the document's own location is not done). A link with valid /QuadPoints is active only inside them, otherwise over /Rect (Table 176); link_list reports resolved_url and active_areas. IsMap is not supported. GoTo and URI links plus local-PDF GoToR links: source-relative or local absolute /F paths, /UF preferred in file-spec dictionaries, explicit numeric page/view destinations, and internal tabs. Absolute local GoToR targets require desktop Allow/Cancel confirmation or an explicit allow_absolute opt-in in link_open; relative links open normally. link_open honors --root. Named remote destinations open at default view with a warning; /NewWindow is a tab hint. A Launch action whose /F names a .pdf file (and has no /Win launch parameters) opens that PDF the same way, backslash separators read as / (#4168); URL/network file systems, GoToE, other Launch actions and other actions are not executed. Same-document destinations retain their supported /XYZ, /Fit, /FitH, /FitV and /FitR views; /FitB* is read as whole-page fit. A null /XYZ left or top keeps the reader's place, as ISO 32000-2 §12.3.2.2 says and Acrobat Reader was observed to do (synthetic fixture): the user-space point at the window's top-left stays there on the target page at the same zoom, along each axis the page overflows; CAD exports (WSCAD) write every cross-reference this way. Null /FitH top and /FitV left still go to the page edge." [[feature]] id = "view.annotation-hover" From 8a7394c57869d3d1171d07b8f2f4902711316781 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:19:43 +0800 Subject: [PATCH 2/3] M3: link_list reports the address the viewer opens, not a second resolution Once inspection's text budget is spent, a relative URI keeps its raw address, and that is what the viewer opens. link_list resolved it afresh against /URI /Base and reported an address nobody opens. It now reads resolved_url from the inspected target, and resolves only for a link inspection did not list. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/automation/src/links.rs | 13 ++++++-- crates/automation/tests/it/automation.rs | 41 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/crates/automation/src/links.rs b/crates/automation/src/links.rs index 3344bd7bb5..59e20f36e7 100644 --- a/crates/automation/src/links.rs +++ b/crates/automation/src/links.rs @@ -105,9 +105,16 @@ impl Automation { (LinkAction::Uri(u), _) => { v["url"] = json!(u); // A relative address, resolved against the catalog's /URI /Base - // (ISO 32000-2 Table 211): what the viewer offers to open. - if let Some(resolved) = pdfcraft_render::resolve_uri(doc.info.uri_base.as_deref(), u) { - v["resolved_url"] = json!(resolved); + // (ISO 32000-2 Table 211): what the viewer offers to open. That is the + // address inspection kept, which stays raw once its text budget is spent; + // resolve afresh only for a link inspection did not list. + let opened = match link.map(|t| &t.target) { + Some(LinkTarget::Uri(kept)) => Some(kept.clone()), + Some(_) => None, + None => pdfcraft_render::resolve_uri(doc.info.uri_base.as_deref(), u), + }; + if let Some(opened) = opened.filter(|o| o != u) { + v["resolved_url"] = json!(opened); } } (action, Some((page, view))) => { diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 8f350b930c..461f5c2d22 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -2095,6 +2095,47 @@ fn links_resolve_relative_addresses_and_report_their_quad_points() { assert_eq!(info["links"][0]["target"]["uri"], "https://example.com/base/page.html", "{info}"); } +/// Once inspection's text budget is spent, a relative address stays raw, and the viewer opens it +/// as written: `link_list` reports that address, not a second resolution against `/URI /Base`. +#[test] +fn link_list_reports_the_address_the_viewer_opens_past_the_text_budget() { + let dir = workdir("link-base-budget"); + // Each link spends what the base adds to its address; well past 16 MiB of it in all. + let base = format!("https://example.com/{}/", "b".repeat(8000)); + let count = (16 << 20) / base.len() + 20; + let annots = "<< /Type /Annot /Subtype /Link /Rect [0 0 10 10] /Border [0 0 0] /A 4 0 R >> ".repeat(count); + let catalog = format!("<< /Type /Catalog /Pages 2 0 R /URI << /Base ({base}) >> >>"); + let page = format!("<< /Type /Page /Parent 2 0 R /Annots [{annots}] >>"); + let objs = [catalog.as_str(), "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>", page.as_str(), "<< /S /URI /URI (p.html) >>"]; + let mut pdf = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(pdf.len()); + pdf.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = pdf.len(); + pdf.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + pdf.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + pdf.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + std::fs::write(dir.join("links.pdf"), pdf).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "links.pdf" }))["doc"].as_u64().unwrap(); + let listed = ok(&mut a, "link_list", json!({ "doc": doc })); + let links = listed["links"].as_array().unwrap(); + assert_eq!(links.len(), count); + let info = ok(&mut a, "doc_info", json!({ "doc": doc })); + let opened = info["links"].as_array().unwrap(); + assert_eq!(opened.len(), count); + let resolved = format!("{base}p.html"); + assert_eq!(links[0]["resolved_url"], json!(resolved), "the first links resolve"); + assert_eq!(opened[0]["target"]["uri"], json!(resolved)); + // Past the budget the viewer keeps (and opens) the raw address; the listing says so. + let last = count - 1; + assert_eq!(opened[last]["target"]["uri"], json!("p.html")); + assert_eq!((links[last]["url"].clone(), links[last].get("resolved_url")), (json!("p.html"), None)); +} #[test] fn bookmarks_from_structure_through_tools() { let dir = workdir("bookmarks-structure"); From 582d491b2f8ae5fdc819f1649d716efa6095cbab Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 08:07:02 +0800 Subject: [PATCH 3/3] M3: a shared /QuadPoints array that no longer fits the budget is not read again Review on #4474: past the text budget every link still resolved and parsed its whole /QuadPoints array before dropping the result, so many links sharing one long array cost its length each. Now no array is read once less than one quadrilateral's bytes is left, and an indirect array that did not fit is remembered and skipped for later links (the budget only shrinks). A guard on the budget being exactly zero alone would rarely trigger, since URIs spend from the same budget. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/render/src/inspect/annotations.rs | 34 +++++++++++++++++++++--- crates/render/src/inspect/tests.rs | 6 +++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index b48c8537a6..e0acc5f2a4 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -62,6 +62,9 @@ impl<'a> Inspector<'a> { /// `text_budget` is what is left of [`MAX_LINK_ATTACHMENT_TEXT_BYTES`]. pub(super) fn annotations(&self, info: &mut DocInfo, text_budget: &mut usize) { + // Indirect `/QuadPoints` arrays whose quadrilaterals did not fit what was left of the budget: + // the budget only shrinks, so they are not read again (review on #4474). + let mut unfit_quads: HashSet = HashSet::new(); let mut pages: Vec<(ObjectId, usize)> = self.page_index.iter().map(|(&id, &page)| (id, page)).collect(); pages.sort_unstable_by_key(|&(_, page)| page); // An annotation, or a whole `/Annots` array, that several entries or pages list is on the @@ -103,7 +106,7 @@ impl<'a> Inspector<'a> { info.attachments.push(self.attachment(fs, AttachmentSource::Annotation { page, index }, text_budget, fallback)); } if subtype == "Link" { - if let Some(link) = self.link(page, index, d, info.uri_base.as_deref(), text_budget) { + if let Some(link) = self.link(page, index, d, info.uri_base.as_deref(), text_budget, &mut unfit_quads) { if matches!(&link.target, LinkTarget::Other(s) if s == "JavaScript") { info.has_javascript = true; } @@ -195,7 +198,16 @@ impl<'a> Inspector<'a> { } /// `base` is the catalog's `/URI /Base`, which relative URI actions are resolved against. - fn link(&self, page: usize, index: usize, d: &Dictionary, base: Option<&str>, text_budget: &mut usize) -> Option { + /// `unfit_quads` holds the indirect `/QuadPoints` arrays already found too large for the budget. + fn link( + &self, + page: usize, + index: usize, + d: &Dictionary, + base: Option<&str>, + text_budget: &mut usize, + unfit_quads: &mut HashSet, + ) -> Option { let rect = rect4(self.resolve(d.get(b"Rect").ok()?)); let target = if let Ok(dest) = d.get(b"Dest") { let (page, view) = self.dest(dest, 0)?; @@ -229,9 +241,16 @@ impl<'a> Inspector<'a> { None => return None, } }; + // Not read at all once no quadrilateral would fit what is left of the budget, or when this + // shared array already did not fit: many links can share one long array, and reading it + // again for each would cost its length every time (review on #4474). + let shared = d.get(b"QuadPoints").ok().and_then(|o| o.as_reference().ok()); + let readable = *text_budget >= std::mem::size_of::<[f32; 8]>() && shared.is_none_or(|id| !unfit_quads.contains(&id)); let quads = match d.get(b"QuadPoints").map(|o| self.resolve(o)) { // Read no further than one value past the cap: a longer array isn't used. - Ok(Object::Array(q)) if q.len() <= crate::MAX_LINK_QUADS.saturating_mul(8) => { + Ok(Object::Array(q)) if readable && q.len() <= crate::MAX_LINK_QUADS.saturating_mul(8) => { + #[cfg(test)] + QUAD_ARRAYS_READ.with(|n| n.set(n.get() + 1)); let values: Vec> = q.iter().map(|o| self.resolve(o).as_float().ok()).collect(); let quads = crate::link_quads(&values, rect); // Many links can share one long array, which each would otherwise copy. Past the @@ -241,6 +260,9 @@ impl<'a> Inspector<'a> { *text_budget = text_budget.saturating_sub(bytes); quads } else { + if let Some(id) = shared { + unfit_quads.insert(id); + } Vec::new() } } @@ -500,3 +522,9 @@ fn rect4(o: &Object) -> [f32; 4] { _ => [0.0; 4], } } + +#[cfg(test)] +thread_local! { + /// `/QuadPoints` arrays whose values were read. + pub(super) static QUAD_ARRAYS_READ: std::cell::Cell = const { std::cell::Cell::new(0) }; +} diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index fe5468f461..13bd9bf267 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1068,11 +1068,17 @@ fn links_sharing_quad_points_or_a_long_base_keep_them_within_a_budget() { let count = MAX_LINK_ATTACHMENT_TEXT_BYTES / (crate::MAX_LINK_QUADS * std::mem::size_of::<[f32; 8]>()) + 20; let quads = format!("[{}]", "10 10 60 10 60 30 10 30 ".repeat(crate::MAX_LINK_QUADS)); let links = "<< /Subtype /Link /Rect [0 0 100 100] /QuadPoints 5 0 R /A << /S /URI /URI (https://example.org/) >> >> ".repeat(count); + super::annotations::QUAD_ARRAYS_READ.with(|n| n.set(0)); let info = fill("", &links, &quads); assert_eq!(info.links.len(), count); let kept: usize = info.links.iter().map(|l| std::mem::size_of_val(l.quads.as_slice())).sum(); assert!(kept > 0 && kept <= MAX_LINK_ATTACHMENT_TEXT_BYTES, "{kept} quad bytes kept"); assert!(info.links.iter().any(|l| l.quads.is_empty()), "past the budget a link is active over its /Rect"); + // Review on #4474: the shared array is read once for each link it fits, and once more to find + // that it no longer does; not again for every link after that. + let fitted = info.links.iter().filter(|l| !l.quads.is_empty()).count(); + let read = super::annotations::QUAD_ARRAYS_READ.with(std::cell::Cell::get); + assert_eq!(read, fitted + 1, "{read} reads for {fitted} links within the budget, of {count}"); let base = format!("https://example.com/{}/", "b".repeat(crate::MAX_URI_BASE_BYTES - 32)); let count = MAX_LINK_ATTACHMENT_TEXT_BYTES / base.len() + 20;