From e0c02d5e893ba915e2ed7a5fcbefe0f79dea902b Mon Sep 17 00:00:00 2001
From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com>
Date: Sat, 17 Oct 2026 13:12:52 +0200
Subject: [PATCH] Strip XML 1.0-forbidden characters from XMP sidecar output

Fixes #2131

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
---
 crates/meta/src/xmp.rs | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/crates/meta/src/xmp.rs b/crates/meta/src/xmp.rs
index c27fe4ae2..9a54301ee 100644
--- a/crates/meta/src/xmp.rs
+++ b/crates/meta/src/xmp.rs
@@ -185,11 +185,22 @@ fn entity(name: &str) -> Option<char> {
                 Some(h) => u32::from_str_radix(h, 16).ok()?,
                 None => n.parse().ok()?,
             };
-            char::from_u32(v)?
+            char::from_u32(v).filter(|&c| is_xml_char(c))?
         }
     })
 }
 
+/// The XML 1.0 `Char` production (the surrogate range can't occur in a `char`).
+fn is_xml_char(c: char) -> bool {
+    matches!(c, '\t' | '\n' | '\r') || (c >= '\u{20}' && c != '\u{FFFE}' && c != '\u{FFFF}')
+}
+
+/// Drop the characters XML 1.0 forbids, which strict parsers (Adobe XMP Toolkit, expat, libxml2) reject the
+/// whole document for.
+fn xml_safe(s: &str) -> String {
+    s.chars().filter(|&c| is_xml_char(c)).collect()
+}
+
 /// Unescape attribute values (predefined + numeric references; unknown references kept verbatim).
 fn unescape_attr(s: &str) -> String {
     let mut out = String::with_capacity(s.len());
@@ -623,12 +634,12 @@ pub fn write_xmp_lc(meta: &Metadata, lc: &[(&str, &str)]) -> String {
     }
     x.push_str(">\n");
     for (k, v) in &simple {
-        x.push_str(&format!("   <{k}>{}</{k}>\n", escape(v.as_str())));
+        x.push_str(&format!("   <{k}>{}</{k}>\n", escape(xml_safe(v))));
     }
     for (k, v) in lc {
         if !k.is_empty() && k.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') {
             // element content: quotes needn't be escaped, which keeps the JSON readable
-            x.push_str(&format!("   <lc:{k}>{}</lc:{k}>\n", partial_escape(*v)));
+            x.push_str(&format!("   <lc:{k}>{}</lc:{k}>\n", partial_escape(xml_safe(v))));
         }
     }
     let array = |x: &mut String, k: &str, kind: &str, items: &[String], lang: bool| {
@@ -638,9 +649,9 @@ pub fn write_xmp_lc(meta: &Metadata, lc: &[(&str, &str)]) -> String {
         x.push_str(&format!("   <{k}>\n    <rdf:{kind}>\n"));
         for it in items {
             if lang {
-                x.push_str(&format!("     <rdf:li xml:lang=\"x-default\">{}</rdf:li>\n", escape(it.as_str())));
+                x.push_str(&format!("     <rdf:li xml:lang=\"x-default\">{}</rdf:li>\n", escape(xml_safe(it))));
             } else {
-                x.push_str(&format!("     <rdf:li>{}</rdf:li>\n", escape(it.as_str())));
+                x.push_str(&format!("     <rdf:li>{}</rdf:li>\n", escape(xml_safe(it))));
             }
         }
         x.push_str(&format!("    </rdf:{kind}>\n   </{k}>\n"));
@@ -1039,4 +1050,24 @@ mod tests {
         a.overlay_user_fields(&b);
         assert_eq!(a.regions, vec![with_region]);
     }
+
+    #[test]
+    fn writer_drops_characters_xml_1_0_forbids() {
+        let bad = "a\u{1}b\u{0}c\u{FFFE}d\u{FFFF}e\u{1F}\tf\ng\rh";
+        let m = Metadata { make: Some(bad.into()), caption: Some(bad.into()), keywords: vec![bad.into()], ..Default::default() };
+        let x = write_xmp_lc(&m, &[("settings", bad)]);
+        let illegal: Vec<char> =
+            x.chars().filter(|&c| (c < '\u{20}' && !matches!(c, '\t' | '\n' | '\r')) || c == '\u{FFFE}' || c == '\u{FFFF}').collect();
+        assert!(illegal.is_empty(), "illegal XML chars in output: {illegal:?}");
+        assert!(x.contains(">abcde\tf\ng\rh<"), "allowed characters must survive: {x}");
+    }
+
+    #[test]
+    fn numeric_references_to_forbidden_characters_are_not_decoded() {
+        assert_eq!(entity("#1"), None);
+        assert_eq!(entity("#0"), None);
+        assert_eq!(entity("#xFFFE"), None);
+        assert_eq!(entity("#x41"), Some('A'));
+        assert_eq!(entity("#9"), Some('\t'));
+    }
 }