M8: metadata check fails closed and finds letter-spaced and short XMP values (#2448)

Review follow-up on #2539:
- the glyph end includes character and word spacing, so letter-spaced
  redacted text is one word and still matches /Title (Bluefin);
- an XMP value wholly part of a removed run goes, as an /Info value does
  (J. Li), compared as whole words (True stays with "untrue");
- when the removed text was cut short (too many runs, a run too long, too
  many terms) every /Info string and XMP stream goes instead of searching
  what was kept;
- XMP that is not valid UTF-8 or UTF-16 (also UTF-16 without a leading
  mark) is not searched lossily but removed;
- namespace declarations and the xpacket wrapper are not searched;
  comments and other processing instructions are;
- the raw-byte assertion saves without object streams so it can see /Info.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/redact/README.md

@@ -31,12 +31,17 @@ clear_marks(&mut doc, None)?;                        // remove marks without app
 - the marks themselves, replaced by boxes in their fill colour (and overlay text) drawn into the
   page as a `/PCMark /Redaction` stream;
 - **metadata** that repeats the removed text (#2448): `/Info` entries containing it or wholly
-  part of it (three letters or digits at least), and the
-  catalog's and pages' XMP streams whose text (element content and attribute values, entities
-  decoded) contains it, removed whole; unreadable XMP goes too. The removed glyphs' text is
-  collected in runs by position, and each run (two or more letters or digits) and each of its
-  words of three or more are matched without regard to case or spacing. `Report::metadata`
-  counts what went.
+  part of it as whole words (three letters or digits at least), and the catalog's and pages'
+  XMP streams whose text (element content, attribute values other than `xmlns` declarations,
+  comments and processing instructions, entities decoded) contains it or has a value wholly
+  part of it, removed whole, together with any PDF/A identification in that packet. XMP that
+  isn't valid UTF-8 or UTF-16 goes too, and when the removed text was too much to collect
+  every `/Info` string and every XMP stream goes (fail-closed). The removed glyphs' text is
+  collected in runs by position (character and word spacing included, so letter-spaced text
+  stays one word), and each run (two or more letters or digits) and each of its words of three
+  or more are matched without regard to case or spacing. A value that shares only words of one
+  or two letters with the removed text, and isn't wholly part of it, stays: Sanitize's
+  metadata category removes the rest. `Report::metadata` counts what went.
 
 A verification pass re-reads each redacted page and fails the operation if any glyph or inline
 image is still under an area. It checks every content stream the page had, whatever its

crates/redact/src/interp.rs

@@ -129,6 +129,9 @@ pub(crate) struct Scope<'a> {
     /// glyphs (#2448), so the document's metadata can be checked for it. Call [`Scope::flush`]
     /// after processing to close the last run.
     pub removed_text: Vec<String>,
+    /// Some removed text was not kept (too many runs, or a run too long): the metadata check
+    /// can't search for it and must fail closed.
+    pub removed_cut: bool,
     /// The run being collected, and where its last glyph ended (device space) with its height.
     run: String,
     run_end: Option<((f64, f64), f64)>,
@@ -157,6 +160,7 @@ impl<'a> Scope<'a> {
             xobject_oc_cache: HashMap::new(),
             layer_blocks: 0,
             removed_text: Vec::new(),
+            removed_cut: false,
             run: String::new(),
             run_end: None,
             fonts: HashMap::new(),
@@ -170,8 +174,12 @@ impl<'a> Scope<'a> {
     pub fn flush(&mut self) {
         self.run_end = None;
         let run = std::mem::take(&mut self.run);
-        if !run.trim().is_empty() && self.removed_text.len() < MAX_RUNS {
-            self.removed_text.push(run);
+        if !run.trim().is_empty() {
+            if self.removed_text.len() < MAX_RUNS {
+                self.removed_text.push(run);
+            } else {
+                self.removed_cut = true;
+            }
         }
     }
 
@@ -191,6 +199,8 @@ impl<'a> Scope<'a> {
         }
         if self.run.len().saturating_add(text.len()) <= MAX_RUN_BYTES {
             self.run.push_str(text);
+        } else {
+            self.removed_cut = true;
         }
         self.run_end = Some((end, height));
     }
@@ -723,7 +733,10 @@ fn show(scope: &mut Scope<'_>, gs: &Gs, tm: &mut Matrix, items: &[Object]) -> (V
                     };
                     if scope.mode == Mode::Apply && scope.hidden_text.is_none() {
                         if hit {
-                            let end = trm.apply(w0, 0.0);
+                            // The glyph ends after its character and word spacing, in `trm`'s
+                            // units (the size is in `trm`), so letter-spaced text stays one word.
+                            let spaced = if size != 0.0 { w0 + spacing / size } else { w0 };
+                            let end = trm.apply(spaced, 0.0);
                             let top = trm.apply(0.0, 1.0);
                             let height = (top.0 - origin.0).hypot(top.1 - origin.1);
                             scope.removed(f.text_of(code).as_deref().unwrap_or(""), origin, end, height);

crates/redact/src/lib.rs

@@ -423,6 +423,7 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result<Report, Redac
         retired_properties: Vec::new(),
         widget_owner,
         removed_text: Vec::new(),
+        removed_cut: false,
         removed_everywhere: HashSet::new(),
     };
     for &(pi, page_marks) in &groups {
@@ -431,7 +432,7 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result<Report, Redac
         redact_page_content(doc, &mut applying, pi, &page, &rects)?;
         remove_page_annotations(doc, &mut applying, &page, &rects, page_marks)?;
     }
-    let Applying { mut report, doomed_fields, doomed_xobjects, retired_properties, mut removed_everywhere, removed_text, .. } = applying;
+    let Applying { mut report, doomed_fields, doomed_xobjects, retired_properties, mut removed_everywhere, removed_text, removed_cut, .. } = applying;
 
     // 2b. Replies and pop-ups of a removed annotation can be listed on another page than their
     // parent: their `/IRT` or `/Parent` would keep the removed comment reachable.
@@ -534,7 +535,7 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result<Report, Redac
         return Err(RedactError::Residue(residue));
     }
     // 6. The document's metadata must not keep what the pages no longer show (#2448).
-    report.metadata = metadata::scrub(doc, &removed_text)?;
+    report.metadata = metadata::scrub(doc, &removed_text, !removed_cut)?;
     // The previous revision still holds the removed content: the next save must rewrite.
     doc.require_full_save();
     Ok(report)
@@ -551,6 +552,8 @@ struct Applying {
     removed_everywhere: HashSet<ObjRef>,
     /// The text of the removed glyphs, for the metadata check at the end.
     removed_text: Vec<String>,
+    /// Some removed text was too long to keep whole for that check.
+    removed_cut: bool,
 }
 
 /// Step 1 for one page: rewrites its content so nothing readable stays under a mark, and cleans the tags it no longer shows.
@@ -561,7 +564,7 @@ fn redact_page_content(
     page: &pdfcraft_model::Page,
     rects: &[[f64; 4]],
 ) -> Result<(), RedactError> {
-    let Applying { report, doomed_xobjects, retired_properties, removed_text, .. } = applying;
+    let Applying { report, doomed_xobjects, retired_properties, removed_text, removed_cut, .. } = applying;
     // 1. Content.
     let mut budget = pdfcraft_cos::DecodeBudget::page();
     let (list, data) = page_streams_within(doc, &page.dict, pi, &mut budget)?;
@@ -571,6 +574,7 @@ fn redact_page_content(
         let out = process(doc, &mut scope, &data, &resources, pdfcraft_content::Matrix::IDENTITY);
         scope.flush();
         removed_text.append(&mut scope.removed_text);
+        *removed_cut |= scope.removed_cut;
         out
     };
     // Fail closed: the interpreter lays vertical-writing text out horizontally, so it cannot

crates/redact/src/metadata.rs

@@ -7,10 +7,15 @@
 //! An `/Info` entry whose text contains a redacted term is removed, and so is one whose whole
 //! value is part of a removed run (an author "J. Li" redacted from a sentence, whose words are too
 //! short to be terms; the reverse check #2598 also proposed). An XMP stream whose text
-//! (element content and attribute values, entities decoded) contains one is removed whole:
-//! editing the XML in place could leave it malformed or keep the term in another spelling.
-//! Unreadable XMP goes too, and so does text too large to search against every term
-//! (fail-closed). Terms are matched without regard to case or spacing.
+//! (element content, attribute values other than namespace declarations, comments and
+//! processing instructions, entities decoded) contains one, or has a value that is part of a
+//! removed run, is removed whole: editing the XML in place could leave it malformed or keep the
+//! term in another spelling. A PDF/A identification in that packet goes with it.
+//!
+//! Fail-closed: XMP that isn't UTF-8 or UTF-16 goes, text too large to search against every term
+//! counts as matching, and when the removed text itself was cut short (too many runs or terms,
+//! or a run too long to keep) every `/Info` string and every XMP stream goes, since what was
+//! never collected can't be searched for. Terms are matched without regard to case or spacing.
 
 use std::collections::HashSet;
 
@@ -36,23 +41,28 @@ fn significant(s: &str, min: usize) -> bool {
 
 /// The terms to look for: each removed run (at least two letters or digits), and each of its
 /// words with at least three, so "Smith, John" in `/Author` is caught when "John Smith" was
-/// redacted. Shorter pieces would match nearly any text.
-pub(crate) fn terms(removed: &[String]) -> Vec<String> {
+/// redacted. Shorter pieces would match nearly any text. `None` when a run is longer than
+/// [`MAX_TERM_CHARS`] or there are more than [`MAX_TERMS`] terms: the caller then can't search
+/// for all of it and must clear the metadata instead.
+pub(crate) fn terms(removed: &[String]) -> Option<Vec<String>> {
     let mut seen = HashSet::new();
     let mut out = Vec::new();
     for run in removed {
-        let run: String = normalize(run).chars().take(MAX_TERM_CHARS).collect();
+        let run = normalize(run);
+        if run.chars().nth(MAX_TERM_CHARS).is_some() {
+            return None;
+        }
         let words = run.split(|c: char| !c.is_alphanumeric()).filter(|w| significant(w, 3)).map(str::to_string).collect::<Vec<_>>();
         for t in std::iter::once(run.clone()).filter(|r| significant(r, 2)).chain(words) {
-            if out.len() >= MAX_TERMS {
-                return out;
-            }
             if seen.insert(t.clone()) {
+                if out.len() >= MAX_TERMS {
+                    return None;
+                }
                 out.push(t);
             }
         }
     }
-    out
+    Some(out)
 }
 
 fn mentions(text: &str, terms: &[String]) -> bool {
@@ -60,8 +70,9 @@ fn mentions(text: &str, terms: &[String]) -> bool {
     text.len().saturating_mul(terms.len()) > MAX_WORK || terms.iter().any(|t| text.contains(t.as_str()))
 }
 
-/// Is `value` itself part of one of the removed `runs` (normalized)? It needs three letters or
-/// digits, so an entry such as "by" doesn't go with every sentence that contains it. Too much
+/// Is `value` itself part of one of the removed `runs` (normalized), as whole words? It needs
+/// three letters or digits, so an entry such as "by" doesn't go with every sentence that
+/// contains it, and word boundaries, so an XMP value "True" doesn't go with "untrue". Too much
 /// text to search counts as part (fail-closed).
 fn within(value: &str, runs: &[String]) -> bool {
     let value = normalize(value);
@@ -69,7 +80,15 @@ fn within(value: &str, runs: &[String]) -> bool {
         return false;
     }
     let total = runs.iter().map(String::len).fold(0usize, usize::saturating_add);
-    value.len().saturating_mul(total) > MAX_WORK || runs.iter().any(|r| r.contains(value.as_str()))
+    let word = |c: Option<char>| c.is_some_and(char::is_alphanumeric);
+    let whole = |r: &String| {
+        r.match_indices(value.as_str()).any(|(at, m)| {
+            let before = r.get(..at).and_then(|s| s.chars().next_back());
+            let after = r.get(at.saturating_add(m.len())..).and_then(|s| s.chars().next());
+            !(word(before) && value.starts_with(char::is_alphanumeric)) && !(word(after) && value.ends_with(char::is_alphanumeric))
+        })
+    };
+    value.len().saturating_mul(total) > MAX_WORK || runs.iter().any(whole)
 }
 
 /// Decode the five predefined XML entities and character references; anything else stays.
@@ -110,34 +129,42 @@ fn unescape(s: &str) -> String {
     out
 }
 
-/// The text an XML document carries: element content and attribute values (XMP stores simple
-/// properties as attributes), each followed by a space, entities decoded. Names, comments and
-/// markup are left out, so a term can't match a tag such as `dc:title`.
-pub(crate) fn xml_text(xml: &str) -> String {
-    let mut out = String::new();
+/// The text values an XML document carries, entities decoded: element content, attribute values
+/// (XMP stores simple properties as attributes), CDATA, and the text of comments and processing
+/// instructions other than the `xpacket` wrapper, which a reader can still see. Names and markup are left out, so a term can't
+/// match a tag such as `dc:title`, and so are namespace declarations (`xmlns`), whose URIs
+/// would match words such as "adobe" or "application" in every packet.
+pub(crate) fn xml_values(xml: &str) -> Vec<String> {
+    let mut out = Vec::new();
+    let mut push = |s: String| {
+        if !s.trim().is_empty() {
+            out.push(s);
+        }
+    };
     let mut rest = xml;
     while !rest.is_empty() {
         let Some(open) = rest.find('<') else {
-            out.push_str(&unescape(rest));
+            push(unescape(rest));
             break;
         };
-        out.push_str(&unescape(rest.get(..open).unwrap_or("")));
-        out.push(' ');
+        push(unescape(rest.get(..open).unwrap_or("")));
         let tag = rest.get(open..).unwrap_or("");
-        if let Some(body) = tag.strip_prefix("<![CDATA[") {
-            let end = body.find("]]>").unwrap_or(body.len());
-            out.push_str(body.get(..end).unwrap_or(""));
-            out.push(' ');
-            rest = body.get(end + 3..).unwrap_or("");
-            continue;
-        }
-        if tag.starts_with("<!--") {
-            rest = tag.find("-->").and_then(|e| tag.get(e + 3..)).unwrap_or("");
+        let delimited = [("<![CDATA[", "]]>"), ("<!--", "-->"), ("<?", "?>")].into_iter().find(|(start, _)| tag.starts_with(start));
+        if let Some((start, stop)) = delimited {
+            let body = tag.get(start.len()..).unwrap_or("");
+            let end = body.find(stop).unwrap_or(body.len());
+            let text = body.get(..end).unwrap_or("");
+            // The packet wrapper's fixed `begin`/`id`/`end` would match words such as "end".
+            if !(start == "<?" && text.starts_with("xpacket")) {
+                push(text.to_string());
+            }
+            rest = body.get(end.saturating_add(stop.len())..).unwrap_or("");
             continue;
         }
         // Attribute values inside the tag, up to its closing `>` (which can't be inside quotes).
         let mut chars = tag.char_indices().skip(1);
         let mut end = tag.len();
+        let mut name = String::new();
         while let Some((i, c)) = chars.next() {
             match c {
                 '>' => {
@@ -152,10 +179,15 @@ pub(crate) fn xml_text(xml: &str) -> String {
                         }
                         value.push(d);
                     }
-                    out.push_str(&unescape(&value));
-                    out.push(' ');
+                    let declaration = name == "xmlns" || name.starts_with("xmlns:");
+                    if !declaration {
+                        push(unescape(&value));
+                    }
+                    name.clear();
                 }
-                _ => {}
+                '=' => {}
+                c if c.is_whitespace() || c == '/' => name.clear(),
+                c => name.push(c),
             }
         }
         rest = tag.get(end..).unwrap_or("");
@@ -163,48 +195,57 @@ pub(crate) fn xml_text(xml: &str) -> String {
     out
 }
 
-/// The decoded text of an XMP stream (UTF-8, or UTF-16 with a byte-order mark), or `None` when it
-/// can't be read.
-fn xmp_text(doc: &Document, o: &Object) -> Option<String> {
+/// Decode an XMP packet: UTF-8 (with or without a byte-order mark), or UTF-16 told by its mark
+/// or, as a packet usually begins `<?xpacket` with the mark inside the attribute, by the zero
+/// byte of its first character. `None` for anything else or any invalid sequence, so text that
+/// can't be read faithfully is never searched lossily (the caller removes it).
+pub(crate) fn decode_xmp(bytes: &[u8]) -> Option<String> {
+    let utf16 = |body: &[u8], be: bool| -> Option<String> {
+        let (units, rest) = body.as_chunks::<2>();
+        if !rest.is_empty() {
+            return None;
+        }
+        char::decode_utf16(units.iter().map(|&p| if be { u16::from_be_bytes(p) } else { u16::from_le_bytes(p) })).collect::<Result<String, _>>().ok()
+    };
+    match bytes {
+        [0xEF, 0xBB, 0xBF, rest @ ..] => std::str::from_utf8(rest).ok().map(str::to_string),
+        [0xFE, 0xFF, rest @ ..] => utf16(rest, true),
+        [0xFF, 0xFE, rest @ ..] => utf16(rest, false),
+        [0, a, ..] if *a != 0 => utf16(bytes, true),
+        [a, 0, ..] if *a != 0 => utf16(bytes, false),
+        _ => std::str::from_utf8(bytes).ok().map(str::to_string),
+    }
+}
+
+/// The text values of an XMP stream, or `None` when it can't be read.
+fn xmp_values(doc: &Document, o: &Object) -> Option<Vec<String>> {
     let Object::Stream(s) = &*doc.resolve(o) else { return None };
     let bytes = s.decoded().ok()?;
     if bytes.len() > MAX_XMP_BYTES {
         return None;
     }
-    let units = |be: bool| {
-        bytes.get(2..).unwrap_or(&[]).as_chunks::<2>().0.iter().map(move |&p| if be { u16::from_be_bytes(p) } else { u16::from_le_bytes(p) })
-    };
-    let text = match bytes.get(..2) {
-        Some([0xFE, 0xFF]) => char::decode_utf16(units(true)).map(|c| c.unwrap_or('\u{FFFD}')).collect(),
-        Some([0xFF, 0xFE]) => char::decode_utf16(units(false)).map(|c| c.unwrap_or('\u{FFFD}')).collect(),
-        _ => String::from_utf8_lossy(&bytes).into_owned(),
-    };
-    Some(xml_text(&text))
+    decode_xmp(&bytes).map(|t| xml_values(&t))
 }
 
 /// Remove the metadata that still holds `removed` text: matching `/Info` entries, and the
-/// catalog's and pages' XMP streams that mention it. Returns how many entries and streams went.
-pub(crate) fn scrub(doc: &mut Document, removed: &[String]) -> Result<usize, pdfcraft_cos::CosError> {
-    let terms = terms(removed);
-    if terms.is_empty() {
-        return Ok(0);
-    }
-    let runs: Vec<String> = removed.iter().take(MAX_TERMS).map(|r| normalize(r).chars().take(MAX_TERM_CHARS).collect()).collect();
+/// catalog's and pages' XMP streams that mention it. `complete` is false when the collection of
+/// removed text was cut short; then, as when the terms themselves exceed their limits, every
+/// `/Info` string and every XMP stream goes. Returns how many entries and streams went.
+pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> Result<usize, pdfcraft_cos::CosError> {
+    // `None` matches everything (fail-closed).
+    let search = match terms(removed).filter(|_| complete) {
+        Some(t) if t.is_empty() => return Ok(0),
+        Some(t) => Some((t, removed.iter().map(|r| normalize(r)).collect::<Vec<String>>())),
+        None => None,
+    };
+    let hit = |text: &str| search.as_ref().is_none_or(|(terms, runs)| mentions(text, terms) || within(text, runs));
     let mut n = 0;
 
     // The information dictionary, direct in the trailer or an object of its own.
     let info_obj = doc.trailer().get(b"Info").cloned();
     if let Some(info) = info_obj.as_ref().and_then(|i| doc.resolve(i).as_dict().cloned()) {
-        let doomed: Vec<Vec<u8>> = info
-            .iter()
-            .filter(|(_, v)| {
-                doc.resolve(v).as_string().is_some_and(|s| {
-                    let s = s.to_text();
-                    mentions(&s, &terms) || within(&s, &runs)
-                })
-            })
-            .map(|(k, _)| k.to_vec())
-            .collect();
+        let doomed: Vec<Vec<u8>> =
+            info.iter().filter(|(_, v)| doc.resolve(v).as_string().is_some_and(|s| hit(&s.to_text()))).map(|(k, _)| k.to_vec()).collect();
         if !doomed.is_empty() {
             n += doomed.len();
             let mut kept = info.clone();
@@ -223,7 +264,15 @@ pub(crate) fn scrub(doc: &mut Document, removed: &[String]) -> Result<usize, pdf
     holders.extend(pdfcraft_model::pages(doc).iter().map(|p| p.obj));
     for holder in holders {
         let Some(xmp) = doc.get(holder).as_dict().and_then(|d| d.get(b"Metadata").cloned()) else { continue };
-        if xmp_text(doc, &xmp).is_none_or(|t| mentions(&t, &terms)) {
+        let doomed = match xmp_values(doc, &xmp) {
+            None => true,
+            Some(values) => {
+                let words = |(terms, _): &(Vec<String>, Vec<String>)| mentions(&values.join(" "), terms);
+                let part = |(_, runs): &(Vec<String>, Vec<String>)| values.iter().any(|v| within(v, runs));
+                search.as_ref().is_none_or(|s| words(s) || part(s))
+            }
+        };
+        if doomed {
             doc.update_dict(holder, |d| {
                 d.remove(b"Metadata");
             })?;

crates/redact/src/tests.rs

@@ -136,8 +136,10 @@ fn redacted_text_does_not_survive_in_info_or_xmp() {
     mark(&mut doc, 0, &[[55.0, 95.0, 90.0, 110.0]], "");
     let r = apply(&mut doc, None).unwrap();
     assert_eq!((r.glyphs, r.metadata), (7, 3), "{r:?}");
-    let bytes = write_full(&doc, &SaveOptions::default()).unwrap();
+    // Without object streams, so the byte search below can see the `/Info` dictionary.
+    let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap();
     let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w.eq_ignore_ascii_case(needle));
+    assert!(has(b"Ada Lovelace"), "the kept /Info entry is visible to the search");
     assert!(!has(b"bluefin") && !has(b"BlueFin"), "the redacted word survived in the metadata");
     let saved = Document::open(Arc::new(bytes)).unwrap();
     let info = saved.trailer().get(b"Info").and_then(|i| saved.resolve(i).as_dict().cloned()).unwrap();
@@ -171,6 +173,90 @@ fn an_info_value_inside_the_redacted_text_goes() {
     assert!(info.contains(b"Subject") && info.contains(b"Producer"));
 }
 
+/// One page showing `content`, with `/Info` `info` and catalog XMP whose `rdf:RDF` holds
+/// `xmp_body`.
+fn with_metadata(content: &[u8], info: &str, xmp_body: &str) -> Document {
+    let xmp = format!(
+        r#"<?xpacket begin="" id="W5M0MpCehiHzreSzNTczkc9d"?><x:xmpmeta xmlns:x="adobe:ns:meta/"><rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">{xmp_body}</rdf:RDF></x:xmpmeta><?xpacket end="w"?>"#
+    );
+    let objs: Vec<Vec<u8>> = vec![
+        b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 0 R >>".to_vec(),
+        b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(),
+        b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> >>".to_vec(),
+        stream("", content),
+        FONT.replace("95 0 R", "6 0 R").into_bytes(),
+        widths(),
+        stream("/Type /Metadata /Subtype /XML", xmp.as_bytes()),
+        info.as_bytes().to_vec(),
+    ];
+    Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 8 0 R").build())).unwrap()
+}
+
+fn info_keys(doc: &Document) -> Vec<String> {
+    let info = doc.trailer().get(b"Info").and_then(|i| doc.resolve(i).as_dict().cloned()).unwrap();
+    info.iter().map(|(k, _)| String::from_utf8_lossy(k).into_owned()).collect()
+}
+
+fn has_xmp(doc: &Document) -> bool {
+    doc.get(doc.root().unwrap()).as_dict().is_some_and(|c| c.contains(b"Metadata"))
+}
+
+/// Letter-spaced text (character spacing wider than a word gap between plain glyphs) is still
+/// one word, so `/Title (Bluefin)` goes when "B L U E F I N" is redacted (#2539 review).
+#[test]
+fn letter_spaced_redacted_text_still_matches_metadata() {
+    let body = r#"<rdf:Description rdf:about="" xmlns:dc="http://purl.org/dc/elements/1.1/"><dc:title>Bluefin</dc:title></rdf:Description>"#;
+    let mut doc = with_metadata(b"BT /F1 10 Tf 3 Tc 10 100 Td (BLUEFIN) Tj ET", "<< /Title (Bluefin) /Producer (Test) >>", body);
+    mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], "");
+    let r = apply(&mut doc, None).unwrap();
+    assert_eq!(r.metadata, 2, "{r:?}");
+    assert_eq!(info_keys(&doc), ["Producer"]);
+    assert!(!has_xmp(&doc));
+}
+
+/// An XMP value that is part of the removed text goes, as an `/Info` value does, though its
+/// words are too short to be terms ("J. Li"). Values are compared as whole words, so "True"
+/// doesn't go with "untrue", and namespace URIs and the `xpacket` wrapper aren't searched, so
+/// redacting "Adobe … end" keeps a packet declared under `adobe:ns:meta/` and closed by
+/// `<?xpacket end="w"?>`.
+#[test]
+fn xmp_values_inside_the_redacted_text_go_as_whole_words() {
+    let creator = r#"<rdf:Description rdf:about="" xmlns:dc="http://purl.org/dc/elements/1.1/"><dc:creator>J. Li</dc:creator></rdf:Description>"#;
+    let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (Prepared by J. Li) Tj ET", "<< /Producer (Test) >>", creator);
+    mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], "");
+    let r = apply(&mut doc, None).unwrap();
+    assert_eq!(r.metadata, 1, "{r:?}");
+    assert!(!has_xmp(&doc), "the packet named J. Li");
+
+    let trapped = r#"<rdf:Description rdf:about="" xmlns:pdf="http://ns.adobe.com/pdf/1.3/" pdf:Trapped="True"/>"#;
+    let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (Adobe said untrue at the end) Tj ET", "<< /Subject (True) /Producer (Test) >>", trapped);
+    mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], "");
+    let r = apply(&mut doc, None).unwrap();
+    assert_eq!(r.metadata, 0, "{r:?}");
+    assert!(has_xmp(&doc));
+    assert_eq!(info_keys(&doc), ["Subject", "Producer"]);
+}
+
+/// When the removed text was too much to collect, what wasn't collected can't be searched for:
+/// every `/Info` string and the XMP go (fail-closed), even though the runs that were kept match
+/// nothing.
+#[test]
+fn metadata_is_cleared_when_the_removed_text_was_cut_short() {
+    // More separate runs of one letter than are kept: each `A` is too far from the last to join.
+    let mut content = b"BT /F1 10 Tf 10 100 Td".to_vec();
+    for _ in 0..50_001 {
+        content.extend_from_slice(b" (A) Tj 30 0 Td (A) Tj -30 0 Td");
+    }
+    content.extend_from_slice(b" ET");
+    let body = r#"<rdf:Description rdf:about="" xmlns:dc="http://purl.org/dc/elements/1.1/"><dc:title>Annual figures</dc:title></rdf:Description>"#;
+    let mut doc = with_metadata(&content, "<< /Title (Annual figures) /Producer (Test) /Trapped /False >>", body);
+    mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], "");
+    let r = apply(&mut doc, None).unwrap();
+    assert_eq!(r.metadata, 3, "{r:?}");
+    assert_eq!(info_keys(&doc), ["Trapped"], "strings go, the name stays");
+    assert!(!has_xmp(&doc));
+}
+
 /// A value shorter than the redacted sentence goes from XMP too, in a custom schema's element or
 /// attribute, as from `/Info` (the regression #2598 suggested keeping): the packets holding it are
 /// removed, one that doesn't stays.
@@ -241,12 +327,29 @@ fn metadata_terms_and_xmp_text() {
     // Runs of two letters or digits or more are matched whole, and by their words of three or
     // more, ignoring case and spacing.
     let t = metadata::terms(&["John  Smith".into(), "7".into(), "of".into(), "A-12".into()]);
-    assert_eq!(t, ["john smith", "john", "smith", "of", "a-12"]);
-    // XMP text is element content and attribute values, entities decoded; not names or comments.
-    let x = metadata::xml_text(r#"<dc:title a="x &amp; y"><!-- hidden --><rdf:li>R&#233;sum&eacute; <![CDATA[<raw>]]></rdf:li></dc:title>"#);
-    let words: Vec<&str> = x.split_whitespace().collect();
-    assert_eq!(words, ["x", "&", "y", "Résum&eacute;", "<raw>"]);
-    assert!(!x.contains("title") && !x.contains("hidden"));
+    assert_eq!(t.as_deref(), Some(&["john smith", "john", "smith", "of", "a-12"].map(String::from)[..]));
+    // A run too long to search for whole can't be matched: the caller clears everything.
+    assert_eq!(metadata::terms(&["x".repeat(5000)]), None);
+    // XMP values: element content, attribute values, CDATA, comments and processing instructions,
+    // entities decoded; not names, and not namespace declarations.
+    let x = metadata::xml_values(
+        r#"<?xpacket begin="" id="W5M0MpCehiHzreSzNTczkc9d"?><?audit James Bond?><x:xmpmeta xmlns:x="adobe:ns:meta/" xmlns="http://example.org/"><dc:title a="x &amp; y"><!-- hidden --><rdf:li>R&#233;sum&eacute; <![CDATA[<raw>]]></rdf:li></dc:title></x:xmpmeta>"#,
+    );
+    assert_eq!(x, ["audit James Bond", "x & y", " hidden ", "Résum&eacute; ", "<raw>"]);
+}
+
+/// XMP is decoded from UTF-8 or UTF-16, the latter also without a byte-order mark at the start
+/// (a packet's mark sits inside `<?xpacket begin="…"`); anything that isn't valid is not read.
+#[test]
+fn xmp_packets_decode_or_fail_closed() {
+    let text = "<?xpacket begin=\"\u{feff}\"?><a>Bond</a>";
+    let utf16 = |be: bool| text.encode_utf16().flat_map(|u| if be { u.to_be_bytes() } else { u.to_le_bytes() }).collect::<Vec<u8>>();
+    assert_eq!(metadata::decode_xmp(text.as_bytes()).as_deref(), Some(text));
+    assert_eq!(metadata::decode_xmp(&utf16(true)).as_deref(), Some(text));
+    assert_eq!(metadata::decode_xmp(&utf16(false)).as_deref(), Some(text));
+    assert_eq!(metadata::decode_xmp(&[[0xFF, 0xFE].as_slice(), &utf16(false)].concat()).as_deref(), Some(text));
+    assert_eq!(metadata::decode_xmp(b"<a>Bond \xE9</a>"), None, "Latin-1 isn't searched lossily");
+    assert_eq!(metadata::decode_xmp(&utf16(false)[1..]), None, "odd length");
 }
 
 #[test]

parity/acrobat-features.toml

@@ -5768,8 +5768,8 @@ milestone = "M8"
 status = "shipped"
 commands = ["redact.apply"]
 tools = ["redact_apply"]
-evidence = ["crates/ui-egui/tests/it/redact.rs::marking_applying_and_clearing", "crates/engine/src/tests.rs::redaction_marks_apply_for_good_and_undo", "crates/automation/tests/it/automation.rs::redacting_through_tools", "crates/engine/src/tests.rs::the_renderer_check_refuses_text_still_shown_under_a_mark", "crates/redact/src/tests.rs::redacted_xobjects_do_not_survive_through_inherited_or_shared_resources", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp"]
-notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more); other objects' XMP, bookmarks and attachments are not searched. Refused, with nothing changed, when the page still shows text under a mark afterwards: text in some predefined CMaps and rotated Type 3 fonts is not removed yet, so those documents cannot be redacted. Also refused on a page with an inline image the renderer reads no image for, since the operators after it can't be known. Removed images and forms are unlinked from page-tree resources (own, inherited or shared) and from surviving forms' resources, not yet from annotation appearances or patterns"
+evidence = ["crates/ui-egui/tests/it/redact.rs::marking_applying_and_clearing", "crates/engine/src/tests.rs::redaction_marks_apply_for_good_and_undo", "crates/automation/tests/it/automation.rs::redacting_through_tools", "crates/engine/src/tests.rs::the_renderer_check_refuses_text_still_shown_under_a_mark", "crates/redact/src/tests.rs::redacted_xobjects_do_not_survive_through_inherited_or_shared_resources", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp", "crates/redact/src/tests.rs::metadata_is_cleared_when_the_removed_text_was_cut_short"]
+notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more, and a value wholly part of a run as whole words; fail-closed when the removed text was too much to collect or the XMP can't be decoded); other objects' XMP, bookmarks and attachments are not searched. Refused, with nothing changed, when the page still shows text under a mark afterwards: text in some predefined CMaps and rotated Type 3 fonts is not removed yet, so those documents cannot be redacted. Also refused on a page with an inline image the renderer reads no image for, since the operators after it can't be known. Removed images and forms are unlinked from page-tree resources (own, inherited or shared) and from surviving forms' resources, not yet from annotation appearances or patterns"
 
 [[feature]]
 id = "protect.redact-glyph-removal"