M8: exact xpacket target, spaced xmlns declarations, XMP decoded within its cap (#2448)

Review follow-up on #2539:
- only a processing instruction whose target is exactly `xpacket` is
  skipped, so `<?xpacketAudit …?>` 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 <noreply@anthropic.com>
This commit is contained in:
mtlcl410andClaude Opus 5.5 committed 2026-10-30 13:58:49 +08:00
1 parent c405d280c6
commit ca0cc61b2d
2 files changed
+22 -9

No files matched your search

+18 -7
View File
@@ -155,7 +155,7 @@ pub(crate) fn xml_values(xml: &str) -> Vec<String> {
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")) {
if !(start == "<?" && text.split_whitespace().next() == Some("xpacket")) {
push(text.to_string());
}
rest = body.get(end.saturating_add(stop.len())..).unwrap_or("");
@@ -164,7 +164,10 @@ pub(crate) fn xml_values(xml: &str) -> Vec<String> {
// 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<String> {
}
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<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;
}
// 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))
}
+4 -2
View File
@@ -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#"<?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>"#,
r#"<?xpacket begin="" id="W5M0MpCehiHzreSzNTczkc9d"?><?audit James Bond?><?xpacketAudit SECRET?><x:xmpmeta xmlns:x="adobe:ns:meta/" xmlns="http://example.org/" xmlns:y = "adobe:ns:y/" b = "spaced"><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>"]);
// 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&eacute; ", "<raw>"]);
}
/// XMP is decoded from UTF-8 or UTF-16, the latter also without a byte-order mark at the start