From fb59c587dfb1e26f385ca385cd6acf5f8b8d572c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 02:06:11 +0000 Subject: [PATCH] print: respell refuses a nested redeclaration; test the vendor-element case for real Review of #307: - respell rewrites the root's namespace declarations, so an element below the root that declares either prefix of a rename again (ns0000 or brpsk) would make a name resolve to another namespace once the prefixes change. It now returns None for such a ticket, and a test covers both prefixes. - The vendor-element test replaced only an opening tag, left the closing tag behind and so never parsed: it passed because respell refused the broken XML, not because of what it was meant to check. Every refusal fixture is now asserted well-formed, and the element is replaced whole. Doing so showed the element guard was redundant: an element or attribute name in a renamed namespace leaves an undeclared prefix, which the final parse of the result already refuses. The guard is removed and the test now exercises that check. - Doc comments on the new constants and tests. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01FQ5PzHVUcntEHU97znim5u --- crates/print/src/ticket.rs | 58 +++++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/crates/print/src/ticket.rs b/crates/print/src/ticket.rs index 955ed343..4f385a20 100644 --- a/crates/print/src/ticket.rs +++ b/crates/print/src/ticket.rs @@ -206,8 +206,11 @@ pub(crate) fn verify_job_options(xml: &str, job: &Job, pages: usize) -> Result<( Ok(()) } +/// XML Schema instance namespace (`xsi:type`). const NS_XSI: &str = "http://www.w3.org/2001/XMLSchema-instance"; +/// XML Schema datatypes namespace (`xsd:integer`, `xsd:QName`). const NS_XSD: &str = "http://www.w3.org/2001/XMLSchema"; +/// The namespace XML itself binds to the `xml` prefix. const NS_XML: &str = "http://www.w3.org/XML/1998/namespace"; /// A PrintTicket that sets nothing. Merged and validated by a driver it comes back as the @@ -286,7 +289,8 @@ fn declared_prefixes(xml: &str, start: usize) -> Vec<(usize, String)> { /// The prefix means nothing in XML, but some drivers' print pipelines read it as text: the job is /// accepted, then every page fails in the XPS print API (`0x80040003`, "No such interface /// supported"). A Brother driver names its namespace `brpsk` and fails with `ns0000`. Only the -/// declarations and the vendor names in `name`/`constrained` attributes and QName values change. +/// declarations and the vendor names in `name`/`constrained` attributes and QName values change; +/// a ticket that redeclares one of the prefixes below its root is left as it is. pub(crate) fn respell(ticket: &str, driver: &str) -> Option { let document = roxmltree::Document::parse(ticket).ok()?; let own = vendor_prefixes(&roxmltree::Document::parse(driver).ok()?); @@ -308,6 +312,16 @@ pub(crate) fn respell(ticket: &str, driver: &str) -> Option { if after.len() != declared_count { return None; } + // Only the root's declarations are rewritten: an element below it that declares either prefix + // again would make a name mean something else once the prefixes change. + let touched = |prefix: &str| renames.iter().any(|(old, new)| prefix == *old || prefix == *new); + let redeclared = document + .descendants() + .filter(|n| n.is_element() && *n != document.root_element()) + .any(|n| declared_prefixes(ticket, n.range().start).iter().any(|(_, prefix)| touched(prefix))); + if redeclared { + return None; + } let respelled = |at: usize| { let rest = ticket.get(at..)?; renames.iter().find(|(old, _)| rest.strip_prefix(old).is_some_and(|r| r.starts_with(':'))).map(|(old, new)| (at, at + old.len(), *new)) @@ -318,13 +332,8 @@ pub(crate) fn respell(ticket: &str, driver: &str) -> Option { edits.push((at, at + prefix.len(), new)); } } - let uses_renamed = |ns: Option<&str>| ns.is_some_and(|ns| declared.iter().any(|(old, uri)| uri == ns && renamed(old).is_some())); for node in document.descendants() { if node.is_element() { - // A vendor namespace used for an element or attribute is not re-spelled here. - if uses_renamed(node.tag_name().namespace()) || node.attributes().any(|a| uses_renamed(a.namespace())) { - return None; - } for a in node.attributes().filter(|a| (a.namespace().is_none() && matches!(a.name(), "name" | "constrained")) || a.namespace() == Some(NS_XSI)) { @@ -343,7 +352,8 @@ pub(crate) fn respell(ticket: &str, driver: &str) -> Option { copied = end; } out.push_str(ticket.get(copied..)?); - // The result must declare each namespace under the prefix wanted and resolve every prefix. + // The result must parse (a vendor element or attribute name would be left with an undeclared + // prefix), declare each namespace under the prefix wanted and resolve every prefix. let check = roxmltree::Document::parse(&out).ok()?; let mut now = vendor_prefixes(&check); let mut wanted: Vec<(String, String)> = declared.iter().map(|(old, uri)| (renamed(old).unwrap_or(old).to_string(), uri.clone())).collect(); @@ -443,6 +453,7 @@ mod tests { assert!(verify_ticket("", ""); - assert_eq!(respell(&element, &brother("brpsk")), None); + let feature = ""; + assert!(merged.contains(feature)); + assert_eq!(refused(&merged.replace(feature, "")), None); // A name whose prefix is not declared stays refused after the change. - let dangling = merged.replace("name=\"ns0000:Mode\"", "name=\"ns0001:Mode\""); - assert_eq!(respell(&dangling, &brother("brpsk")), None); + assert_eq!(refused(&merged.replace("name=\"ns0000:Mode\"", "name=\"ns0001:Mode\"")), None); + // A nested element that declares either prefix again would change what a name resolves to. + for redeclared in ["ns0000", "brpsk"] { + let nested = merged.replace( + "", + &format!(""), + ); + assert_ne!(nested, merged); + assert_eq!(refused(&nested), None, "{redeclared} declared again below the root"); + } + // Control: the same ticket without the nested declaration is respelled. + assert!(refused(&merged).is_some()); } #[test]