From ca0cc61b2dcb9077a25e21805f207e01457514cc Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 17 Oct 2026 02:14:24 +0800 Subject: [PATCH] M8: exact xpacket target, spaced xmlns declarations, XMP decoded within its cap (#2448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on #2539: - only a processing instruction whose target is exactly `xpacket` is skipped, so `` is searched like any other; - an attribute name is kept across whitespace before `=`, so `xmlns:x = "adobe:ns:meta/"` is still a declaration, not a value; - the XMP stream is decoded with decoded_within(MAX_XMP_BYTES), so a small stream that inflates hugely stops at the cap instead of after it. Co-Authored-By: Claude Opus 5.5 --- crates/redact/src/metadata.rs | 25 ++++++++++++++++++------- crates/redact/src/tests.rs | 6 ++++-- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index b6ef956192..84500ecc81 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -155,7 +155,7 @@ pub(crate) fn xml_values(xml: &str) -> Vec { 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 == " Vec { // 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(); + // The attribute name being read, and the last one read before whitespace: XML allows + // whitespace around `=` (`xmlns:x = "…"`). let mut name = String::new(); + let mut last = String::new(); while let Some((i, c)) = chars.next() { match c { '>' => { @@ -179,14 +182,24 @@ pub(crate) fn xml_values(xml: &str) -> Vec { } value.push(d); } - let declaration = name == "xmlns" || name.starts_with("xmlns:"); + let attr = if name.is_empty() { &last } else { &name }; + let declaration = attr == "xmlns" || attr.starts_with("xmlns:"); if !declaration { push(unescape(&value)); } name.clear(); + last.clear(); } '=' => {} - c if c.is_whitespace() || c == '/' => name.clear(), + '/' => { + name.clear(); + last.clear(); + } + c if c.is_whitespace() => { + if !name.is_empty() { + last = std::mem::take(&mut name); + } + } c => name.push(c), } } @@ -220,10 +233,8 @@ pub(crate) fn decode_xmp(bytes: &[u8]) -> Option { /// The text values of an XMP stream, or `None` when it can't be read. fn xmp_values(doc: &Document, o: &Object) -> Option> { let Object::Stream(s) = &*doc.resolve(o) else { return None }; - let bytes = s.decoded().ok()?; - if bytes.len() > MAX_XMP_BYTES { - return None; - } + // Decoding stops at the cap, so a small stream that inflates hugely costs no more memory. + let bytes = s.decoded_within(MAX_XMP_BYTES).ok()?; decode_xmp(&bytes).map(|t| xml_values(&t)) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 58194c5cac..19f553332f 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -333,9 +333,11 @@ fn metadata_terms_and_xmp_text() { // 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#"Résumé ]]>"#, + r#"Résumé ]]>"#, ); - assert_eq!(x, ["audit James Bond", "x & y", " hidden ", "Résumé ", ""]); + // Only the `xpacket` wrapper itself is skipped, and a declaration is one with spaces around + // `=` too. + assert_eq!(x, ["audit James Bond", "xpacketAudit SECRET", "spaced", "x & y", " hidden ", "Résumé ", ""]); } /// XMP is decoded from UTF-8 or UTF-16, the latter also without a byte-order mark at the start