collective
Browse / github.com/storytold/pdfcraft / patch
M12: a dynamic XFA form whose data has a bare ampersand is still laid out
M12: a dynamic XFA form whose data has a bare ampersand is still laid out ## What this changes A dynamic XFA form whose data has a bare `&` was never laid out. Example: the filled IRCC form IMM 1344 (the `xfa_filled_imm1344e.pdf` sample in the pdf.js test corpus) has `Humanitarian & Compassionate` in its data. That is not well-formed XML, though Adobe's reader accepts it. The packet parse failed with `malformed entity reference`, so the form showed only its "Please wait... upgrade Adobe Reader" page instead of its 10 pages and saved values. A bare `&` (one that is not one of the five predefined entities or a numeric reference) is now escaped to `&` where the p
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- hikashop-nicolas (imported from pull request #4944, open)
- Base
1dd2737b47a9c1c9562f0af563053d1bf510e4bf- Size
- +111 −1 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
f3c0efa9a247caffce8357f1
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/tests.rs crates/xfa/src/packets.rs
@@ -5521,6 +5521,41 @@ fn dynamic_xfa_forms_open_with_the_values_and_rows_of_their_data() { assert!(!doc.dirty); } +/// A filled form whose data has a bare `&` ("Humanitarian & Compassionate" in an IRCC form) is not well-formed XML, though+/// Adobe's reader takes it: it laid out as nothing, and the form showed only its "Please wait" page.+#[test]+fn a_dynamic_xfa_form_whose_data_has_a_bare_ampersand_is_still_laid_out() {+ let data = "<form><head><familyName>Smith & Sons</familyName></head><table><row><what0>R&D</what0></row></table></form>";+ let bytes = Arc::new(pdfcraft_xfa::fixtures::shell(&pdfcraft_xfa::fixtures::template_with_data(1, data)));+ let mut s = Session::new().with_clock(|| 1_700_000_000);+ let id = s.open("form.pdf", None, bytes, None).expect("opens");+ let doc = s.get(id).unwrap();+ assert!(doc.xfa.is_some(), "laid out: {:?}", doc.xfa_warnings);+ let value = |n: &str| doc.form.iter().find(|f| f.name == n).unwrap_or_else(|| panic!("no field {n}")).value.clone();+ assert_eq!(value("familyName"), vec!["Smith & Sons".to_string()]);+ assert_eq!(value("what0"), vec!["R&D".to_string()]);+ // what is saved back is well-formed+ s.apply(id, Edit::SetFieldValue { name: "familyName".into(), value: FieldValue::Text("Jones & Co".into()) }).unwrap();+ let saved = s.save_bytes(id).unwrap();+ // the saved datasets stream itself, before any reader could repair it+ let cos = pdfcraft_cos::Document::open(saved).unwrap();+ let acro = cos.get(cos.root().unwrap()).as_dict().and_then(|c| c.get(b"AcroForm").map(|a| cos.resolve(a))).unwrap();+ let packets = acro.as_dict().and_then(|a| a.get(b"XFA").map(|x| cos.resolve(x))).unwrap();+ // one XDP stream, or an array of packets in which the datasets one is named+ let stream = match &*packets {+ pdfcraft_cos::Object::Array(items) => {+ let at = (0..items.len()).step_by(2).find(|&i| items[i].as_string().is_some_and(|s| s.to_text() == "datasets")).expect("a datasets packet");+ cos.resolve(&items[at + 1])+ }+ _ => packets.clone(),+ };+ let pdfcraft_cos::Object::Stream(stream) = &*stream else { panic!("a stream") };+ let text = String::from_utf8(stream.decoded().unwrap()).unwrap();+ assert!(!text.contains("Jones & Co") && text.contains("Jones & Co"), "{text}");+ let data = pdfcraft_xfa::parse_datasets(&text).expect("the saved datasets are XML");+ assert_eq!(data.text_at(&pdfcraft_xfa::som_to_path("form[0].head[0].familyName[0]")), Some("Jones & Co"));+}+ /// #3180: an XFA form with no fields whose configuration says `dynamicRender` is `forbidden` is a /// static form: its pages are the form. Opening it used to lay the template out into new pages /// and write them as a revision, so even a save with no change replaced the pages.@@ -44,6 +44,11 @@ fn utf16(bytes: &[u8], from: fn([u8; 2]) -> u16) -> Option<String> { /// anything else (invalid UTF-8, a legacy encoding, a broken surrogate), which can be read /// only approximately and so must not be written back. pub fn decode(bytes: &[u8]) -> Option<(String, Encoding)> {+ // a bare `&` is escaped on the way in, so the packet is XML for every reader and what is written back is too+ decode_exactly(bytes).map(|(text, encoding)| (repair_ampersands(text), encoding))+}++fn decode_exactly(bytes: &[u8]) -> Option<(String, Encoding)> { match bytes { [0xFF, 0xFE, rest @ ..] => utf16(rest, u16::from_le_bytes).map(|t| (t, Encoding::Utf16Le { bom: true })), [0xFE, 0xFF, rest @ ..] => utf16(rest, u16::from_be_bytes).map(|t| (t, Encoding::Utf16Be { bom: true })),@@ -97,6 +102,56 @@ fn to_text(bytes: &[u8]) -> String { } } +/// A packet whose text has a bare `&` ("Humanitarian & Compassionate" in a filled form's data) is not well-formed XML,+/// though Adobe's own reader takes it: such a `&` becomes `&`, leaving CDATA and comments as they are.+pub(crate) fn repair_ampersands(text: String) -> String {+ // only the five predefined entities and numeric references are references; anything else is text+ let valid_reference = |rest: &str| -> bool {+ // the `;` must come within the longest name (32), so a text full of `&` is not searched to its end each time+ let Some(end) = rest.char_indices().take(34).find(|(_, c)| *c == ';').map(|(i, _)| i).filter(|e| *e > 1 && *e <= 32) else { return false };+ let name = &rest[1..end];+ match name.strip_prefix('#') {+ Some(num) => match num.strip_prefix(['x', 'X']) {+ Some(hex) => !hex.is_empty() && hex.chars().all(|c| c.is_ascii_hexdigit()),+ None => !num.is_empty() && num.chars().all(|c| c.is_ascii_digit()),+ },+ None => matches!(name, "lt" | "gt" | "amp" | "quot" | "apos"),+ }+ };+ if !text.contains('&') {+ return text;+ }+ let mut out = String::with_capacity(text.len() + 16);+ let mut rest = text.as_str();+ let mut changed = false;+ while let Some(at) = rest.find(['&', '<']) {+ out.push_str(&rest[..at]);+ rest = &rest[at..];+ if let Some(inner) = rest.strip_prefix("<![CDATA[") {+ let len = inner.find("]]>").map_or(rest.len(), |e| e + "<![CDATA[".len() + 3);+ out.push_str(&rest[..len]);+ rest = &rest[len..];+ } else if rest.starts_with("<!--") {+ let len = rest.find("-->").map_or(rest.len(), |e| e + 3);+ out.push_str(&rest[..len]);+ rest = &rest[len..];+ } else if rest.starts_with('&') {+ if valid_reference(rest) {+ out.push('&');+ } else {+ out.push_str("&");+ changed = true;+ }+ rest = &rest[1..];+ } else {+ out.push('<');+ rest = &rest[1..];+ }+ }+ out.push_str(rest);+ if changed { out } else { text }+}+ /// Read the XFA packets, if the document has any. pub fn read_packets(doc: &Document) -> Result<Option<Packets>, XfaError> { let Some(root) = doc.root() else { return Ok(None) };@@ -127,7 +182,7 @@ pub fn read_packets(doc: &Document) -> Result<Option<Packets>, XfaError> { if total > MAX_XDP { return Err(XfaError::TooLarge("the XFA packets are larger than 64 MB".into())); }- Ok(Some(to_text(&bytes)))+ Ok(Some(repair_ampersands(to_text(&bytes)))) }; let mut xdp = String::new(); match &*xfa {@@ -163,3 +218,23 @@ pub fn read_packets(doc: &Document) -> Result<Option<Packets>, XfaError> { } Ok(Some(Packets { xdp, needs_rendering: needs_rendering.unwrap_or(false), has_fields })) }++#[cfg(test)]+mod repair_tests {+ use super::repair_ampersands;++ #[test]+ fn a_bare_ampersand_is_escaped_and_the_rest_is_left_alone() {+ assert_eq!(repair_ampersands("<a>Humanitarian & Compassionate</a>".into()), "<a>Humanitarian & Compassionate</a>");+ assert_eq!(repair_ampersands("<a>x & y 
 <</a>".into()), "<a>x & y 
 <</a>");+ assert_eq!(+ repair_ampersands("<a><![CDATA[ a & b ]]> c & d <!-- e & f --></a>".into()),+ "<a><![CDATA[ a & b ]]> c & d <!-- e & f --></a>"+ );+ assert_eq!(repair_ampersands("<a>AT&T; R&D, &, & "</a>".into()), "<a>AT&T; R&D, &, & &nbsp; "</a>");+ assert_eq!(repair_ampersands("no entities".into()), "no entities");+ // an unterminated CDATA section or comment runs to the end and stays as it is+ assert_eq!(repair_ampersands("<a><![CDATA[ a & b".into()), "<a><![CDATA[ a & b");+ assert_eq!(repair_ampersands("<a><!-- a & b".into()), "<a><!-- a & b");+ }+}