From 8f82494fb1d323ef24d5a068c789262c9c510209 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 5 Jul 2026 10:41:53 +0200 Subject: [PATCH 1/4] PDF: render /Link annotations as overlays (URI + internal GoTo) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A page's /Link annotations (ISO 32000-1 12.5.6.5) now become absolutely positioned overlays: - a /URI action -> external link (href attribute-escaped, opened in _blank via the document target); - a /GoTo action or a direct /Dest -> internal "#pN" link; each page div gains a matching id="pN". Destinations resolve a page reference (first element of a [page ...] array) to its index, including named destinations via the catalog /Dests dictionary and the /Names /Dests name tree (depth-guarded). The /Rect is mapped through the page transform to page-box points. Resolved entirely in the HTML layer from the parser's raw annotation dictionaries — no parser/IR change — and emitted by both the dual-layer and single-layer render paths. Also: - Fix parse_page crashing (map::at) on a page with no /Contents — a blank page carrying only annotations is valid; guard the lookup. - Elevate attribute escaping to html::escape_attribute in common.{hpp,cpp} (next to escape_text), replacing the local helper. Adds an end-to-end PdfFile test rendering a mini-PDF with a /URI, a direct /Dest, and a named /GoTo destination, asserting the anchors and page ids in both render modes. Co-Authored-By: Claude Opus 4.8 --- src/odr/internal/html/common.cpp | 8 + src/odr/internal/html/common.hpp | 4 + src/odr/internal/html/pdf_file.cpp | 220 ++++++++++++++++++- src/odr/internal/pdf/AGENTS.md | 23 +- src/odr/internal/pdf/pdf_document_parser.cpp | 23 +- test/src/internal/pdf/pdf_file.cpp | 63 ++++++ 6 files changed, 323 insertions(+), 18 deletions(-) diff --git a/src/odr/internal/html/common.cpp b/src/odr/internal/html/common.cpp index 2092e60c8..fb4d62181 100644 --- a/src/odr/internal/html/common.cpp +++ b/src/odr/internal/html/common.cpp @@ -37,6 +37,14 @@ std::string html::escape_text(std::string text) { return text; } +std::string html::escape_attribute(std::string value) { + util::string::replace_all(value, "&", "&"); + util::string::replace_all(value, "\"", """); + util::string::replace_all(value, "<", "<"); + util::string::replace_all(value, ">", ">"); + return value; +} + std::string html::color(const Color &color) { std::stringstream ss; ss << "#"; diff --git a/src/odr/internal/html/common.hpp b/src/odr/internal/html/common.hpp index 188dbefe0..711b193da 100644 --- a/src/odr/internal/html/common.hpp +++ b/src/odr/internal/html/common.hpp @@ -35,6 +35,10 @@ struct WritingState { std::string escape_text(std::string text); +/// Escape a string for use as an HTML double-quoted attribute value (`&`, `"`, +/// `<`, `>`). Unlike `escape_text`, it leaves leading/trailing spaces intact. +std::string escape_attribute(std::string value); + std::string color(const Color &color); std::string file_to_url(const std::string &file, const std::string &mime_type); diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index 4089769c9..5293df875 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -57,6 +57,198 @@ std::string svg_matrix(const util::math::Transform2D &m) { return std::move(f).str(); } +/// One resolved link annotation, positioned in page-box points (y-down). +struct LinkOut { + double left{0}; + double top{0}; + double width{0}; + double height{0}; + std::string href; ///< external URI or internal "#pN"; already attr-escaped +}; + +/// Resolves a link annotation's destination to a page: a `page-object -> +/// 0-based index` map plus the catalog's named-destination table (`/Dests` +/// dictionary and the `/Names /Dests` name tree, ISO 32000-1 12.3.2.3). +struct LinkResolver { + pdf::DocumentParser &parser; + std::map page_index; + std::map named_dests; ///< name -> raw dest value + + /// A destination (a name/string, a `[page …]` array, or a dict with `/D`) + /// to its target page index, if it resolves to a known page. + [[nodiscard]] std::optional resolve_dest_page(pdf::Object dest) { + dest = parser.resolve_object_copy(std::move(dest)); + if (dest.is_string() || dest.is_name()) { + const std::string name = + dest.is_string() ? dest.as_string() : dest.as_name(); + const auto it = named_dests.find(name); + if (it == named_dests.end()) { + return std::nullopt; + } + dest = parser.resolve_object_copy(it->second); + } + if (dest.is_dictionary() && dest.as_dictionary().has_value("D")) { + dest = parser.resolve_object_copy(dest.as_dictionary().get("D")); + } + if (!dest.is_array() || dest.as_array().empty()) { + return std::nullopt; + } + const pdf::Object &target = dest.as_array()[0]; + if (!target.is_reference()) { + return std::nullopt; + } + const auto it = page_index.find(target.as_reference()); + return it != page_index.end() ? std::optional(it->second) + : std::nullopt; + } +}; + +/// Walk a destination name tree (`Names` leaf pairs, `Kids` intermediates), +/// depth-guarded, collecting `name -> dest` into `out`. +void collect_dest_name_tree(pdf::DocumentParser &parser, + const pdf::Object &node_ref, + std::map &out, + const int depth) { + if (depth > 50) { + return; + } + const pdf::Object node = parser.resolve_object_copy(node_ref); + if (!node.is_dictionary()) { + return; + } + const pdf::Dictionary &dictionary = node.as_dictionary(); + if (dictionary.has_value("Names")) { + const pdf::Array names = + parser.resolve_object_copy(dictionary.get("Names")).as_array(); + for (std::size_t i = 0; i + 1 < names.size(); i += 2) { + const pdf::Object key = parser.resolve_object_copy(names[i]); + if (key.is_string()) { + out.emplace(key.as_string(), names[i + 1]); + } + } + } + if (dictionary.has_value("Kids")) { + const pdf::Array kids = + parser.resolve_object_copy(dictionary.get("Kids")).as_array(); + for (const pdf::Object &kid : kids) { + collect_dest_name_tree(parser, kid, out, depth + 1); + } + } +} + +/// Build the link resolver once per document: the page-index map and the +/// catalog's named destinations. +LinkResolver build_link_resolver(pdf::DocumentParser &parser, + const pdf::Document &document, + const std::vector &pages) { + LinkResolver resolver{parser, {}, {}}; + for (std::size_t i = 0; i < pages.size(); ++i) { + resolver.page_index.emplace(pages[i]->object_reference, i); + } + if (document.catalog != nullptr && document.catalog->object.is_dictionary()) { + const pdf::Dictionary &catalog = document.catalog->object.as_dictionary(); + if (catalog.has_value("Dests")) { + const pdf::Object dests = + parser.resolve_object_copy(catalog.get("Dests")); + if (dests.is_dictionary()) { + for (const auto &[key, value] : dests.as_dictionary()) { + resolver.named_dests.emplace(key, value); + } + } + } + if (catalog.has_value("Names")) { + const pdf::Object names = + parser.resolve_object_copy(catalog.get("Names")); + if (names.is_dictionary() && names.as_dictionary().has_value("Dests")) { + collect_dest_name_tree(parser, names.as_dictionary().get("Dests"), + resolver.named_dests, 0); + } + } + } + return resolver; +} + +/// Resolve a page's `/Link` annotations (ISO 32000-1 12.5.6.5) to positioned +/// overlays: a `/URI` action becomes an external link, a `/GoTo` action or a +/// direct `/Dest` an internal `#pN` link. `to_box` maps PDF user space to the +/// page box (points, y-down). +std::vector collect_page_links(const pdf::Page &page, + const util::math::Transform2D &to_box, + LinkResolver &resolver) { + std::vector links; + pdf::DocumentParser &parser = resolver.parser; + for (const pdf::Annotation *annotation : page.annotations) { + if (annotation == nullptr || !annotation->object.is_dictionary()) { + continue; + } + const pdf::Dictionary &dictionary = annotation->object.as_dictionary(); + const pdf::Object subtype = + parser.resolve_object_copy(dictionary.get("Subtype")); + if (!subtype.is_name() || subtype.as_name() != "Link") { + continue; + } + const pdf::Object rect = parser.resolve_object_copy(dictionary.get("Rect")); + if (!rect.is_array() || rect.as_array().size() < 4) { + continue; + } + const std::vector r = rect.as_reals(); + + std::string href; + if (dictionary.has_value("A")) { + const pdf::Object action = + parser.resolve_object_copy(dictionary.get("A")); + if (action.is_dictionary()) { + const pdf::Dictionary &a = action.as_dictionary(); + const pdf::Object s = parser.resolve_object_copy(a.get("S")); + const std::string kind = s.is_name() ? s.as_name() : ""; + if (kind == "URI" && a.has_value("URI")) { + const pdf::Object uri = parser.resolve_object_copy(a.get("URI")); + if (uri.is_string()) { + href = uri.as_string(); + } + } else if (kind == "GoTo" && a.has_value("D")) { + if (const auto index = resolver.resolve_dest_page(a.get("D"))) { + href = "#p" + std::to_string(*index + 1); + } + } + } + } + if (href.empty() && dictionary.has_value("Dest")) { + if (const auto index = + resolver.resolve_dest_page(dictionary.get("Dest"))) { + href = "#p" + std::to_string(*index + 1); + } + } + if (href.empty()) { + continue; + } + + const std::array p0 = to_box.apply(r[0], r[1]); + const std::array p1 = to_box.apply(r[2], r[3]); + LinkOut link; + link.left = std::min(p0[0], p1[0]); + link.top = std::min(p0[1], p1[1]); + link.width = std::abs(p1[0] - p0[0]); + link.height = std::abs(p1[1] - p0[1]); + link.href = escape_attribute(std::move(href)); + links.push_back(std::move(link)); + } + return links; +} + +/// Write a page's link overlays as absolutely-positioned `` elements (in +/// page-box points, matching the text layer's unit). +void write_page_links(HtmlWriter &out, const std::vector &links) { + for (const LinkOut &link : links) { + std::ostringstream a; + a << ""; + out.write_raw(std::move(a).str()); + } +} + /// Clamp a colour component in [0, 1] to an 8-bit channel value. std::int32_t to255(const double v) { return static_cast( @@ -1006,6 +1198,7 @@ class HtmlServiceImpl final : public HtmlService { std::vector vis_items; std::vector sel_lines; std::string clip_defs; + std::vector links; }; HtmlResources write_document_dual_layer(HtmlWriter &out) const { @@ -1016,6 +1209,7 @@ class HtmlServiceImpl final : public HtmlService { pdf::DocumentParser parser = pdf_file.create_parser(*m_logger); const std::unique_ptr document = parser.parse_document(); const std::vector pages = document->collect_pages(); + LinkResolver link_resolver = build_link_resolver(parser, *document, pages); AtomicStyles styles; std::vector pages_out; @@ -1072,6 +1266,7 @@ class HtmlServiceImpl final : public HtmlService { page_out.classes = pb.classes; page_out.width = width; page_out.height = height; + page_out.links = collect_page_links(*page, to_box, link_resolver); std::string stream; for (const auto &ref : page->contents_reference) { @@ -1436,9 +1631,13 @@ class HtmlServiceImpl final : public HtmlService { }; out.write_body_begin(); + std::size_t page_number = 0; for (const DualPageOut &page : pages_out) { - out.write_element_begin("div", - HtmlElementOptions().set_class(page.classes)); + out.write_element_begin( + "div", + HtmlElementOptions() + .set_class(page.classes) + .set_extra(R"(id="p)" + std::to_string(++page_number) + R"(")")); // Visual layer: paint-order graphics and unselectable glyphs. out.write_element_begin("div", @@ -1455,6 +1654,9 @@ class HtmlServiceImpl final : public HtmlService { } out.write_element_end("div"); // .sel + // Link overlays on top so they stay clickable. + write_page_links(out, page.links); + out.write_element_end("div"); // .p } out.write_body_end(); @@ -1514,6 +1716,7 @@ class HtmlServiceImpl final : public HtmlService { double height{0}; std::vector items; std::string clip_defs; + std::vector links; }; HtmlResources write_document_single_layer(HtmlWriter &out) const { @@ -1524,6 +1727,7 @@ class HtmlServiceImpl final : public HtmlService { pdf::DocumentParser parser = pdf_file.create_parser(*m_logger); const std::unique_ptr document = parser.parse_document(); const std::vector pages = document->collect_pages(); + LinkResolver link_resolver = build_link_resolver(parser, *document, pages); // ---- Font registration ------------------------------------------------ // A real-Unicode scalar gets a cmap entry only inside the BMP and outside @@ -1662,6 +1866,7 @@ class HtmlServiceImpl final : public HtmlService { page_out.classes = pb.classes; page_out.width = width; page_out.height = height; + page_out.links = collect_page_links(page, to_box, link_resolver); ClipRegistry clips(static_cast(pages_out.size())); GradientRegistry gradients(static_cast(pages_out.size())); @@ -1950,11 +2155,16 @@ class HtmlServiceImpl final : public HtmlService { }; out.write_body_begin(); + std::size_t page_number = 0; for (const SinglePageOut &page : pages_out) { - out.write_element_begin("div", - HtmlElementOptions().set_class(page.classes)); + out.write_element_begin( + "div", + HtmlElementOptions() + .set_class(page.classes) + .set_extra(R"(id="p)" + std::to_string(++page_number) + R"(")")); write_page_items(out, page.clip_defs, page.items, page.width, page.height, write_line); + write_page_links(out, page.links); out.write_element_end("div"); } out.write_body_end(); @@ -2195,6 +2405,8 @@ class HtmlServiceImpl final : public HtmlService { // SVG overlay covering the page box (visual graphics layer). out.out() << ".s{position:absolute;left:0;top:0;width:100%;height:100%;" "overflow:hidden;pointer-events:none}"; + // Link annotation overlays (absolutely positioned in page-box points). + out.out() << ".lk{position:absolute;transform-origin:0 0}"; out.out() << font_faces; out.out() << font_styles; styles.write_rules(out.out()); diff --git a/src/odr/internal/pdf/AGENTS.md b/src/odr/internal/pdf/AGENTS.md index 195f05fd2..6bfd5b02b 100644 --- a/src/odr/internal/pdf/AGENTS.md +++ b/src/odr/internal/pdf/AGENTS.md @@ -274,6 +274,19 @@ inline SVG per page. Experimental and not production-quality. matches rendering) is unsettled — currently `/Ascent` wins. The ascent is per-font, not per-CID. No focused unit test yet (verified visually on `style-various-1.pdf`); the reference-output snapshot test is the gate. +- **Link annotations** (`html/pdf_file.cpp`): a page's `/Link` annotations + (ISO 32000-1 12.5.6.5) become absolutely-positioned `` overlays. A `/URI` + action is an external link (attribute-escaped `href`, opened in `_blank` via + the document target); a `/GoTo` action or a direct `/Dest` is an internal + `#pN` link — each page `div` carries a matching `id="pN"`. Destinations + resolve a page *reference* (the first element of a `[page …]` array) to its + index, including named destinations via the catalog `/Dests` dictionary and + the `/Names /Dests` name tree (depth-guarded). The `/Rect` is mapped through + the page transform to page-box points. Resolved entirely in the HTML layer + from the parser's raw annotation dictionaries — no parser/IR change. Both + render modes emit them. Deferred: remote/launch actions (`/GoToR`, `/Launch`), + the destination's scroll position/zoom (only the target page is used), and + non-`/Link` annotation appearances (see *Interaction & navigation*). ## Graphics, images & transparency @@ -632,12 +645,13 @@ design before implementation. ## Interaction & navigation The next feature cluster — needs destinations from the page tree, little else. +Link annotations (`/URI` + internal `/GoTo`) already land — see *What works*. -- **Links**: URI actions and internal `GoTo` destinations (incl. named) as `` - overlays. - **Annotation appearances**: render `/AP` appearance streams (form XObjects again) for highlights, stamps, form-field appearances; AcroForm *interactivity* stays out of scope (read-only). +- **Remote/launch link actions** (`/GoToR`, `/Launch`) and destination scroll + position/zoom (the internal-link handler uses only the target page). - **Document outline** (`/Outlines`) → navigation anchors/sidebar. - **Optional content groups** (layers): honor default visibility; no toggle UI. - **Output scaling**: monolithic HTML vs. per-page lazy loading for large @@ -694,8 +708,9 @@ The next feature cluster — needs destinations from the page tree, little else. `/W2`/`/DW2` vertical metrics and a perpendicular pen advance, which the horizontal-only `extract_text` and space inference assume away). No corpus fixture needs either yet; revisit when one does. -- **Annotations** are collected but their content is not interpreted - (*Interaction & navigation*). +- **Annotations**: `/Link` annotations render as `` overlays (see *What + works*); other annotation types are collected but their appearance streams are + not interpreted (*Interaction & navigation*). - **XMP metadata** (`/Metadata` XML packet) is not parsed; `file_meta()`'s `document_meta` comes from the `/Info` dictionary only. XMP would supplement or override `/Info` for producers that write it. diff --git a/src/odr/internal/pdf/pdf_document_parser.cpp b/src/odr/internal/pdf/pdf_document_parser.cpp index 3550aa79c..badddca17 100644 --- a/src/odr/internal/pdf/pdf_document_parser.cpp +++ b/src/odr/internal/pdf/pdf_document_parser.cpp @@ -1192,17 +1192,20 @@ Page *parse_page(State &state, const ObjectReference &reference, Pages *parent, page->resources = parse_resources(state, resources); // /Contents is a content stream or an array of them, supplied directly or - // through an indirect reference (7.7.3.3). Resolve a reference first so that - // a reference to an array is expanded into its stream references rather than - // mistaken for a single stream. - const Object &contents = dictionary["Contents"]; - const Object resolved_contents = parser.resolve_object_copy(contents); - if (resolved_contents.is_array()) { - for (const Object &e : resolved_contents.as_array()) { - page->contents_reference.push_back(e.as_reference()); + // through an indirect reference (7.7.3.3). It is optional — a page may have + // none (e.g. a blank page carrying only annotations). Resolve a reference + // first so that a reference to an array is expanded into its stream + // references rather than mistaken for a single stream. + if (dictionary.has_key("Contents")) { + const Object &contents = dictionary["Contents"]; + const Object resolved_contents = parser.resolve_object_copy(contents); + if (resolved_contents.is_array()) { + for (const Object &e : resolved_contents.as_array()) { + page->contents_reference.push_back(e.as_reference()); + } + } else if (contents.is_reference()) { + page->contents_reference = {contents.as_reference()}; } - } else if (contents.is_reference()) { - page->contents_reference = {contents.as_reference()}; } if (dictionary.has_key("Annots")) { diff --git a/test/src/internal/pdf/pdf_file.cpp b/test/src/internal/pdf/pdf_file.cpp index 0871556d9..8c61dbf51 100644 --- a/test/src/internal/pdf/pdf_file.cpp +++ b/test/src/internal/pdf/pdf_file.cpp @@ -1,11 +1,15 @@ #include +#include +#include #include +#include #include #include #include +#include #include #include @@ -20,6 +24,44 @@ std::shared_ptr open_pdf(const std::string &bytes) { return std::make_shared(std::make_shared(bytes)); } +std::string render_html(const std::string &bytes, const PdfTextMode mode) { + const odr::PdfFile file(open_pdf(bytes)); + HtmlConfig config; + config.pdf_text_mode = mode; + const std::shared_ptr logger = Logger::create_null(); + const HtmlService service = + internal::html::create_pdf_service(file, "", config, logger); + std::ostringstream out; + service.write("document.html", out); + return std::move(out).str(); +} + +bool contains(const std::string &haystack, const std::string &needle) { + return haystack.find(needle) != std::string::npos; +} + +/// A three-page mini-PDF whose first page carries three `/Link` annotations: a +/// `/URI` action, a direct `/Dest` array to page 2, and a `/GoTo` action to a +/// named destination (`chap3` → page 3, via the catalog `/Dests`). +std::string link_annotations_mini_pdf() { + PdfFileBuilder builder; + builder + .object("<< /Type /Catalog /Pages 2 0 R " + "/Dests << /chap3 [5 0 R /Fit] >> >>") + .object("<< /Type /Pages /Kids [3 0 R 4 0 R 5 0 R] /Count 3 >>") + .object("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] " + "/Annots [6 0 R 7 0 R 8 0 R] >>") + .object("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>") + .object("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>") + .object("<< /Type /Annot /Subtype /Link /Rect [100 700 200 720] " + "/A << /S /URI /URI (http://example.com/?a=1&b=2) >> >>") + .object("<< /Type /Annot /Subtype /Link /Rect [100 600 200 620] " + "/Dest [4 0 R /Fit] >>") + .object("<< /Type /Annot /Subtype /Link /Rect [100 500 200 520] " + "/A << /S /GoTo /D (chap3) >> >>"); + return builder.trailer("/Root 1 0 R").build_classic(); +} + /// A two-page mini-PDF with an `/Info` dictionary. `/Title` is a UTF-16BE /// (BOM) hex string ("HI"), the rest literal (PDFDocEncoding) strings. std::string info_mini_pdf() { @@ -78,3 +120,24 @@ TEST(PdfFile, file_meta_without_info) { EXPECT_FALSE(meta.document_meta->title.has_value()); EXPECT_FALSE(meta.document_meta->author.has_value()); } + +// `/Link` annotations render as `` overlays: a `/URI` action → external href +// (with `&` attr-escaped), a direct `/Dest` and a named `/GoTo` → internal +// `#pN` anchors; each page div carries a matching `id`. +TEST(PdfFile, link_annotations_render_as_anchors) { + const std::string pdf = link_annotations_mini_pdf(); + for (const PdfTextMode mode : + {PdfTextMode::dual_layer, PdfTextMode::single_layer}) { + const std::string html = render_html(pdf, mode); + EXPECT_TRUE(contains(html, R"(id="p1")")) + << "mode " << static_cast(mode); + EXPECT_TRUE(contains(html, R"(id="p3")")) + << "mode " << static_cast(mode); + EXPECT_TRUE(contains(html, R"(href="http://example.com/?a=1&b=2")")) + << "mode " << static_cast(mode); + EXPECT_TRUE(contains(html, R"(href="#p2")")) + << "mode " << static_cast(mode); + EXPECT_TRUE(contains(html, R"(href="#p3")")) + << "mode " << static_cast(mode); + } +} From 780d66f63f28e13960ab8e0758aa860f5698b1c0 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 5 Jul 2026 12:21:23 +0200 Subject: [PATCH 2/4] PDF: sanitize /URI link schemes and keep internal links in-tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Codex review on #591: - Reject active URI schemes (javascript:/data:/…) in /Link /URI actions; only navigable schemes and relative refs become live hrefs. - Emit target="_self" on internal #pN anchors so they override the document's and scroll in place. Co-Authored-By: Claude Opus 4.8 --- src/odr/internal/html/pdf_file.cpp | 52 +++++++++++++++++++++++++++--- test/src/internal/pdf/pdf_file.cpp | 24 +++++++++----- 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index 5293df875..2e4ba1931 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -64,6 +64,7 @@ struct LinkOut { double width{0}; double height{0}; std::string href; ///< external URI or internal "#pN"; already attr-escaped + bool internal{false}; ///< true for a `#pN` target (needs `target="_self"`) }; /// Resolves a link annotation's destination to a page: a `page-object -> @@ -168,6 +169,41 @@ LinkResolver build_link_resolver(pdf::DocumentParser &parser, return resolver; } +/// Whether a `/URI` action target is safe to emit as an `href`. A PDF is +/// untrusted input, so active schemes (`javascript:`, `data:`, `vbscript:`, …) +/// must not become a clickable link that executes in the generated document. +/// We allow only the common navigable schemes plus scheme-less (relative) +/// references. Embedded ASCII whitespace/control bytes are ignored when reading +/// the scheme, matching browsers that strip them before dispatch (so +/// `java\tscript:` cannot slip through). +bool is_safe_uri(std::string_view uri) { + std::string scheme; + for (const char ch : uri) { + const auto c = static_cast(ch); + if (ch == ':') { + for (char &s : scheme) { + s = static_cast(std::tolower(static_cast(s))); + } + static constexpr std::string_view allowed[] = {"http", "https", "mailto", + "ftp", "ftps", "tel"}; + return std::find(std::begin(allowed), std::end(allowed), scheme) != + std::end(allowed); + } + if (ch == '/' || ch == '?' || ch == '#') { + return true; // path/query/fragment reached first -> relative reference + } + if (c <= 0x20) { + continue; // browsers strip embedded whitespace/control bytes + } + if (std::isalnum(c) != 0 || ch == '+' || ch == '-' || ch == '.') { + scheme.push_back(ch); + continue; + } + return true; // not a valid scheme character -> relative reference + } + return true; // no ':' -> relative reference +} + /// Resolve a page's `/Link` annotations (ISO 32000-1 12.5.6.5) to positioned /// overlays: a `/URI` action becomes an external link, a `/GoTo` action or a /// direct `/Dest` an internal `#pN` link. `to_box` maps PDF user space to the @@ -194,6 +230,7 @@ std::vector collect_page_links(const pdf::Page &page, const std::vector r = rect.as_reals(); std::string href; + bool internal = false; if (dictionary.has_value("A")) { const pdf::Object action = parser.resolve_object_copy(dictionary.get("A")); @@ -203,12 +240,13 @@ std::vector collect_page_links(const pdf::Page &page, const std::string kind = s.is_name() ? s.as_name() : ""; if (kind == "URI" && a.has_value("URI")) { const pdf::Object uri = parser.resolve_object_copy(a.get("URI")); - if (uri.is_string()) { + if (uri.is_string() && is_safe_uri(uri.as_string())) { href = uri.as_string(); } } else if (kind == "GoTo" && a.has_value("D")) { if (const auto index = resolver.resolve_dest_page(a.get("D"))) { href = "#p" + std::to_string(*index + 1); + internal = true; } } } @@ -217,6 +255,7 @@ std::vector collect_page_links(const pdf::Page &page, if (const auto index = resolver.resolve_dest_page(dictionary.get("Dest"))) { href = "#p" + std::to_string(*index + 1); + internal = true; } } if (href.empty()) { @@ -231,6 +270,7 @@ std::vector collect_page_links(const pdf::Page &page, link.width = std::abs(p1[0] - p0[0]); link.height = std::abs(p1[1] - p0[1]); link.href = escape_attribute(std::move(href)); + link.internal = internal; links.push_back(std::move(link)); } return links; @@ -241,9 +281,13 @@ std::vector collect_page_links(const pdf::Page &page, void write_page_links(HtmlWriter &out, const std::vector &links) { for (const LinkOut &link : links) { std::ostringstream a; - a << "` so they scroll within the rendered PDF instead of + // opening a new copy. + a << ""; out.write_raw(std::move(a).str()); } diff --git a/test/src/internal/pdf/pdf_file.cpp b/test/src/internal/pdf/pdf_file.cpp index 8c61dbf51..11b45fe9a 100644 --- a/test/src/internal/pdf/pdf_file.cpp +++ b/test/src/internal/pdf/pdf_file.cpp @@ -40,9 +40,10 @@ bool contains(const std::string &haystack, const std::string &needle) { return haystack.find(needle) != std::string::npos; } -/// A three-page mini-PDF whose first page carries three `/Link` annotations: a -/// `/URI` action, a direct `/Dest` array to page 2, and a `/GoTo` action to a -/// named destination (`chap3` → page 3, via the catalog `/Dests`). +/// A three-page mini-PDF whose first page carries four `/Link` annotations: a +/// `/URI` action, a direct `/Dest` array to page 2, a `/GoTo` action to a named +/// destination (`chap3` → page 3, via the catalog `/Dests`), and a `/URI` +/// action with a `javascript:` scheme (which must not become a live link). std::string link_annotations_mini_pdf() { PdfFileBuilder builder; builder @@ -50,7 +51,7 @@ std::string link_annotations_mini_pdf() { "/Dests << /chap3 [5 0 R /Fit] >> >>") .object("<< /Type /Pages /Kids [3 0 R 4 0 R 5 0 R] /Count 3 >>") .object("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] " - "/Annots [6 0 R 7 0 R 8 0 R] >>") + "/Annots [6 0 R 7 0 R 8 0 R 9 0 R] >>") .object("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>") .object("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>") .object("<< /Type /Annot /Subtype /Link /Rect [100 700 200 720] " @@ -58,7 +59,9 @@ std::string link_annotations_mini_pdf() { .object("<< /Type /Annot /Subtype /Link /Rect [100 600 200 620] " "/Dest [4 0 R /Fit] >>") .object("<< /Type /Annot /Subtype /Link /Rect [100 500 200 520] " - "/A << /S /GoTo /D (chap3) >> >>"); + "/A << /S /GoTo /D (chap3) >> >>") + .object("<< /Type /Annot /Subtype /Link /Rect [100 400 200 420] " + "/A << /S /URI /URI (javascript:alert\\(1\\)) >> >>"); return builder.trailer("/Root 1 0 R").build_classic(); } @@ -123,7 +126,9 @@ TEST(PdfFile, file_meta_without_info) { // `/Link` annotations render as `` overlays: a `/URI` action → external href // (with `&` attr-escaped), a direct `/Dest` and a named `/GoTo` → internal -// `#pN` anchors; each page div carries a matching `id`. +// `#pN` anchors that carry `target="_self"` (overriding ``); each page div carries a matching `id`. An active-scheme +// `/URI` is dropped. TEST(PdfFile, link_annotations_render_as_anchors) { const std::string pdf = link_annotations_mini_pdf(); for (const PdfTextMode mode : @@ -135,9 +140,12 @@ TEST(PdfFile, link_annotations_render_as_anchors) { << "mode " << static_cast(mode); EXPECT_TRUE(contains(html, R"(href="http://example.com/?a=1&b=2")")) << "mode " << static_cast(mode); - EXPECT_TRUE(contains(html, R"(href="#p2")")) + EXPECT_TRUE(contains(html, R"(href="#p2" target="_self")")) << "mode " << static_cast(mode); - EXPECT_TRUE(contains(html, R"(href="#p3")")) + EXPECT_TRUE(contains(html, R"(href="#p3" target="_self")")) + << "mode " << static_cast(mode); + // The `javascript:` action is not emitted as a link. + EXPECT_FALSE(contains(html, "javascript:alert")) << "mode " << static_cast(mode); } } From 0e5fec724570be844e4f759bee1dc41cc677189a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 5 Jul 2026 14:54:14 +0200 Subject: [PATCH 3/4] PDF docs: record link-overlay vs. text-selection trade-off and options Co-Authored-By: Claude Opus 4.8 --- src/odr/internal/pdf/AGENTS.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/odr/internal/pdf/AGENTS.md b/src/odr/internal/pdf/AGENTS.md index 6bfd5b02b..99f365b7b 100644 --- a/src/odr/internal/pdf/AGENTS.md +++ b/src/odr/internal/pdf/AGENTS.md @@ -652,6 +652,17 @@ Link annotations (`/URI` + internal `/GoTo`) already land — see *What works*. *interactivity* stays out of scope (read-only). - **Remote/launch link actions** (`/GoToR`, `/Launch`) and destination scroll position/zoom (the internal-link handler uses only the target page). +- **Link overlays vs. text selection**: the `` overlays sit above the `.sel` + text, so a mousedown over a link starts navigation instead of a selection — + links are clickable but text under them cannot be selected, and there is no + hover cursor issue since native `:hover` applies. A JS workaround existed + (overlays `pointer-events:none` so mousedown falls through to `.sel`; a + once-per-document click handler re-enabled hit-testing via an `.lkhit` class, + `elementFromPoint`, and forwarded the click; a rAF-throttled `mousemove` + restored the pointer cursor via an `.lkc` class) but was rolled back as too + involved (reverted commit `5cfa8a09`, reachable for later). Options to + revisit: (a) reinstate that JS approach; (b) a CSS-only route if one exists; + (c) leave overlays clickable-only and accept no under-link selection. - **Document outline** (`/Outlines`) → navigation anchors/sidebar. - **Optional content groups** (layers): honor default visibility; no toggle UI. - **Output scaling**: monolithic HTML vs. per-page lazy loading for large From ddd9a3b7eb8750ef473f5243a975d74d500fa7ee Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 5 Jul 2026 14:56:55 +0200 Subject: [PATCH 4/4] update refs --- test/data/reference-output/odr-private | 2 +- test/data/reference-output/odr-public | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data/reference-output/odr-private b/test/data/reference-output/odr-private index 6be5d3572..10c55b6b6 160000 --- a/test/data/reference-output/odr-private +++ b/test/data/reference-output/odr-private @@ -1 +1 @@ -Subproject commit 6be5d35729848e19f1c298cec06232c9b5159023 +Subproject commit 10c55b6b637ba681f37804d8b12432fa04cc1688 diff --git a/test/data/reference-output/odr-public b/test/data/reference-output/odr-public index 7d8dfe1ef..1f2618261 160000 --- a/test/data/reference-output/odr-public +++ b/test/data/reference-output/odr-public @@ -1 +1 @@ -Subproject commit 7d8dfe1ef79e768a59950f1767bfaab4665f2c27 +Subproject commit 1f2618261533da094670d02413ca3ec02618c2f4