Copy a subtree its document lost as an unlinked node (#356)

PyXmlSec_LxmlLivePathTo stops where getparent() returns None, but an
element removed from its tree is not a document root: lxml leaves such a
subtree pointing at the document it left, whose getroottree() still
serializes that document — now without the subtree in it. BeginDoc
therefore copied a document that does not hold the element, recorded
depth 0, and handed xmlsec the copy's *root* instead. Signing a template
taken out of sign1-in.xml (its URI="" reference then digests the
document without the signature in it, which is the point of taking it
out) succeeds on the raw path and failed with Error(1, 'failed to sign')
under the shadow; find_parent was worse, answering for the wrong tree
with no error at all.

The copy now reproduces the shape the raw path works on. The document is
copied as before, for the references, and the removed subtree is copied
into it as an unlinked node beside its tree (shadow.unlinked): that node
is what shadow.root / shadow.element and every path map between, what
the marking tags, what the reflection re-parses (xmlSaveTree of the
subtree — the document dump does not hold it) and what Discard frees,
since the document does not own it. Registered ids are replayed once per
live top, the subtree's own and the document's, because a #id reference
from the subtree into the document it left resolves on the raw path too.
Such a node cannot be replaced on either path — libxml2 needs a parent
to put the replacement in.

Begin's whole-document detour for internal-subset documents had the same
blind spot (the dump cannot hold the subtree, and the path re-rooted the
wrong node); it now takes that detour only when the element really hangs
under the document's root.

Also: test_encrypt_uri never called encrypt_uri — it called
encrypt_binary with the file:// URI — so the shadow path of encrypt_uri
had no successful test at all.

Verified byte-identical to the raw path for sign+verify of a removed
template, a removed ancestor, a #id reference into the document it left,
find_parent in both directions, decrypt, and an encrypt/decrypt
Type=Content round trip. Five new tests, each failing on the shadow path
before the fix and passing on the raw path. 323 passed / 6 skipped on
the mismatch build (also at PYXMLSEC_TEST_ITERATIONS=50), 335 / 6 on the
matched static wheel plain and with PYXMLSEC_FORCE_SHADOW=1; a 10k
removed-template sign+verify loop holds RSS at 24.3 → 26.1 MiB with a
stable digest.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: mxamin

developer.md

@@ -160,7 +160,20 @@ caller gets a new proxy object.
 `index`), serializes `element.getroottree()` — comments/PIs outside the root
 and the internal DTD subset survive — and hands back the copy's counterpart;
 `shadow.element` becomes the live *root*, which is where the reflection maps
-paths onto. `BeginNewDoc` creates an empty private document; `End` roots the
+paths onto.
+
+An element **removed from its document** is a shape of its own: lxml leaves
+such a subtree pointing at the document it left, and so does the raw path —
+xmlsec works on a node outside the tree whose `doc` still answers its `#id`
+references (a template taken out of a document and signed with `URI=""`
+digests that document, without itself in it). The copy holds both: the
+document is copied as always, and the removed subtree is copied into it as an
+*unlinked* node beside its tree (`shadow.unlinked`), which is then what
+`shadow.root` / `shadow.element` and every path map between. Registered IDs
+are replayed twice, once for each of the two live tops. Such a node cannot be
+replaced (libxml2 needs a parent to put the replacement in), on either path.
+
+`BeginNewDoc` creates an empty private document; `End` roots the
 detached result there, dumps it and returns it as a new detached lxml element
 (in a document of its own until grafted; lxml moves it when the caller
 appends it, like the raw path's detached node).
@@ -303,9 +316,9 @@ All invisible to the documented API:
 - a subtree of a document that declares entities in its internal subset
   (`resolve_entities=False`) is copied by copying the whole document and
   cutting it back to the element, since the subtree's `&name;` references
-  need their declarations; `encrypt_xml` templates are still serialized on
-  their own, so a *template* carrying unresolved entity references is not
-  supported (signing and encryption of such a document are refused on both
+  need their declarations; `encrypt_xml` templates and subtrees removed from
+  their document are still serialized on their own, so a *template* — or a
+  removed subtree — carrying unresolved entity references is not supported (signing and encryption of such a document are refused on both
   paths anyway — libxml2's c14n rejects entity-reference nodes);
 - operations that replace the **document root** (encrypting the root element
   with `Type=Element`, decrypting a root `EncryptedData`) morph the live root

src/lxml.c

@@ -17,6 +17,7 @@
 #include <libxml/xmlmemory.h>
 #include <libxml/parser.h>
 #include <libxml/dict.h>
+#include <libxml/xmlsave.h>
 
 #include <stdint.h>
 
@@ -435,18 +436,26 @@ static int PyXmlSec_LxmlShadowTagNodes(PyXmlSec_LxmlShadow* shadow, xmlNodePtr n
 // exception set.
 static int PyXmlSec_LxmlShadowMark(PyXmlSec_LxmlShadow* shadow) {
     int count = PyXmlSec_LxmlShadowCountNodes(shadow->doc->children, 0);
+    // The unlinked subtree is in the document but not in its tree, so the
+    // walk over the tree does not reach it; it is pre-existing all the same.
+    int unlinked = shadow->unlinked ? PyXmlSec_LxmlShadowCountNodes(shadow->root, 0) : 0;
+    int next;
 
-    if (count < 0) {
+    if (count < 0 || unlinked < 0) {
         PyErr_SetString(PyXmlSec_InternalError, "the document is nested too deeply.");
         return -1;
     }
+    count += unlinked;
     shadow->tags = (PyXmlSec_LxmlShadowTag*)PyMem_Malloc((count > 0 ? count : 1) * sizeof(*shadow->tags));
     if (shadow->tags == NULL) {
         PyErr_NoMemory();
         return -1;
     }
     shadow->ntags = count;
-    PyXmlSec_LxmlShadowTagNodes(shadow, shadow->doc->children, 0);
+    next = PyXmlSec_LxmlShadowTagNodes(shadow, shadow->doc->children, 0);
+    if (shadow->unlinked) {
+        PyXmlSec_LxmlShadowTagNodes(shadow, shadow->root, next);
+    }
     return 0;
 }
 
@@ -593,6 +602,33 @@ static PyObject* PyXmlSec_LxmlShadowDumpCopy(PyXmlSec_LxmlShadow* shadow) {
     xmlChar* dump = NULL;
     int dump_size = 0;
 
+    if (shadow->unlinked) {
+        // The paths the reflection carries are relative to the unlinked
+        // subtree, so that is what has to be re-parsed — the document dump
+        // does not hold it.
+        xmlBufferPtr buffer = xmlBufferCreate();
+        xmlSaveCtxtPtr save = buffer != NULL ? xmlSaveToBuffer(buffer, "UTF-8", XML_SAVE_NO_DECL) : NULL;
+        int failed = 1;
+
+        if (save != NULL) {
+            failed = xmlSaveTree(save, shadow->root) < 0;
+            failed = xmlSaveClose(save) < 0 || failed;
+        }
+        if (!failed) {
+            bytes = PyBytes_FromStringAndSize((const char*)xmlBufferContent(buffer), (Py_ssize_t)xmlBufferLength(buffer));
+            if (bytes != NULL) {
+                result = PyXmlSec_LxmlElementFromBytes(bytes);
+            }
+            Py_XDECREF(bytes);
+        } else {
+            PyErr_SetString(PyXmlSec_InternalError, "cannot serialize the private copy.");
+        }
+        if (buffer != NULL) {
+            xmlBufferFree(buffer);
+        }
+        return result;
+    }
+
     xmlDocDumpMemory(shadow->doc, &dump, &dump_size);
     if (dump == NULL || dump_size <= 0) {
         PyErr_SetString(PyXmlSec_InternalError, "cannot serialize the private copy.");
@@ -695,13 +731,15 @@ int PyXmlSec_LxmlShadowBegin(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElementPt
     int depth = 0;
     int dtd = 0;
     int extdtd = 0;
+    int whole = 0;
 
     shadow->element = element;
     shadow->owned = NULL;
     shadow->doc = NULL;
     shadow->root = NULL;
     shadow->tags = NULL;
     shadow->ntags = 0;
+    shadow->unlinked = 0;
 
     // Fast path: lxml links the same libxml2 (the import guard passed), so
     // xmlsec can mutate lxml's nodes directly and no copy is needed; End sees
@@ -725,12 +763,22 @@ int PyXmlSec_LxmlShadowBegin(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElementPt
         // in the document's internal subset — serializing the element alone
         // would leave them undefined and the parse would fail. Copy the whole
         // document, declarations included, and cut it back to the element.
+        // Unless the element was removed from that document: the dump would
+        // not hold it at all, and the element alone is all there is to copy.
         PyObject* live_top = NULL;
+        PyObject* live_root = PyObject_CallMethod(tree, "getroot", NULL);
+        if (live_root == NULL) {
+            goto ON_FAIL;
+        }
         depth = PyXmlSec_LxmlLivePathTo((PyObject*)element, path, &live_top);
+        whole = depth >= 0 && live_top == live_root;
         Py_XDECREF(live_top);
+        Py_DECREF(live_root);
         if (depth < 0) {
             goto ON_FAIL;
         }
+    }
+    if (whole) {
         bytes = PyObject_CallFunctionObjArgs(PyXmlSec_LxmlEtreeToString, tree, NULL);
     } else {
         bytes = PyXmlSec_LxmlElementToBytes((PyObject*)element);
@@ -745,7 +793,7 @@ int PyXmlSec_LxmlShadowBegin(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElementPt
     if (shadow->doc == NULL) {
         goto ON_FAIL;
     }
-    if (dtd && PyXmlSec_LxmlShadowReroot(shadow, path, depth) < 0) {
+    if (whole && PyXmlSec_LxmlShadowReroot(shadow, path, depth) < 0) {
         goto ON_FAIL;
     }
     shadow->root = xmlDocGetRootElement(shadow->doc);
@@ -769,6 +817,7 @@ xmlDocPtr PyXmlSec_LxmlShadowBeginNewDoc(PyXmlSec_LxmlShadow* shadow, PyXmlSec_L
     shadow->root = NULL;
     shadow->tags = NULL;
     shadow->ntags = 0;
+    shadow->unlinked = 0;
 
     // Fast path: allocate the detached subtree straight in the element's own
     // document, as the raw code always did.
@@ -785,9 +834,15 @@ xmlDocPtr PyXmlSec_LxmlShadowBeginNewDoc(PyXmlSec_LxmlShadow* shadow, PyXmlSec_L
 
 void PyXmlSec_LxmlShadowDiscard(PyXmlSec_LxmlShadow* shadow) {
     if (shadow->doc != NULL) {
+        if (shadow->unlinked && shadow->root != NULL) {
+            // Outside the document's tree: freeing the document does not
+            // reach it.
+            xmlFreeNode(shadow->root);
+        }
         xmlFreeDoc(shadow->doc);
         shadow->doc = NULL;
         shadow->root = NULL;
+        shadow->unlinked = 0;
     }
     PyMem_Free(shadow->tags);
     shadow->tags = NULL;
@@ -1523,8 +1578,10 @@ static void PyXmlSec_LxmlShadowApplyIdSpec(xmlDocPtr doc, xmlNodePtr node, const
 }
 
 // Replays the specs recorded for the shadow's live document onto the copy,
-// each at the copy's counterpart of the element it was registered for.
-static int PyXmlSec_LxmlShadowReplayIds(PyXmlSec_LxmlShadow* shadow) {
+// each at the copy's counterpart of the element it was registered for: a spec
+// recorded for an element under the live `live_top` is applied under its
+// counterpart `copy_top`. Returns 0, or -1 with an exception set.
+static int PyXmlSec_LxmlShadowReplayIds(PyXmlSec_LxmlShadow* shadow, PyObject* live_top, xmlNodePtr copy_top) {
     PyObject* key;
     PyObject* entry;
     PyObject* nodes;
@@ -1567,19 +1624,49 @@ static int PyXmlSec_LxmlShadowReplayIds(PyXmlSec_LxmlShadow* shadow) {
             return -1;
         }
         // lxml hands out one proxy per node, so identity settles whether the
-        // element still hangs under the root being copied; a registration for
-        // an element that has since left this tree applies to nothing here.
-        if (top == (PyObject*)shadow->element) {
-            PyXmlSec_LxmlShadowApplyIdSpec(shadow->doc, PyXmlSec_LxmlShadowWalkNode(shadow->root, path, depth),
+        // element still hangs under the tree being replayed; a registration
+        // for an element that has since left it applies to nothing here.
+        if (top == live_top) {
+            PyXmlSec_LxmlShadowApplyIdSpec(shadow->doc, PyXmlSec_LxmlShadowWalkNode(copy_top, path, depth),
                                            (const xmlChar*)name, (const xmlChar*)ns, subtree);
         }
         Py_DECREF(top);
     }
     return 0;
 }
 
+// Copies the live subtree `element` — removed from its document, so the copy
+// of that document does not hold it either — into the private document, as an
+// unlinked node beside its tree. That is the shape the raw path hands xmlsec:
+// a node outside the tree whose `doc` still answers `#id` references.
+// Returns 0, or -1 with an exception set.
+static int PyXmlSec_LxmlShadowUnlinkedCopy(PyXmlSec_LxmlShadow* shadow, PyObject* element) {
+    PyObject* bytes = PyXmlSec_LxmlElementToBytes(element);
+    xmlDocPtr tdoc;
+    xmlNodePtr copy;
+
+    if (bytes == NULL) {
+        return -1;
+    }
+    tdoc = PyXmlSec_LxmlShadowParse(bytes, NULL, 0, "cannot make a private copy of the element.");
+    Py_DECREF(bytes);
+    if (tdoc == NULL) {
+        return -1;
+    }
+    copy = xmlDocCopyNode(xmlDocGetRootElement(tdoc), shadow->doc, 1);
+    xmlFreeDoc(tdoc);
+    if (copy == NULL) {
+        PyErr_SetString(PyXmlSec_InternalError, "cannot make a private copy of the element.");
+        return -1;
+    }
+    shadow->root = copy;
+    shadow->unlinked = 1;
+    return 0;
+}
+
 int PyXmlSec_LxmlShadowBeginDoc(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElementPtr element, xmlNodePtr* target) {
     PyObject* cur = NULL;
+    PyObject* live_root = NULL;
     PyObject* tree = NULL;
     PyObject* bytes = NULL;
     PyObject* url_holder = NULL;
@@ -1595,6 +1682,7 @@ int PyXmlSec_LxmlShadowBeginDoc(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElemen
     shadow->root = NULL;
     shadow->tags = NULL;
     shadow->ntags = 0;
+    shadow->unlinked = 0;
     *target = NULL;
 
     if (!PyXmlSec_LxmlShadowActive) {
@@ -1617,6 +1705,13 @@ int PyXmlSec_LxmlShadowBeginDoc(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElemen
     if (tree == NULL) {
         goto ON_FAIL;
     }
+    // `cur` is the document's root element unless `element` — or an ancestor
+    // of it — was removed from the tree: lxml leaves such a subtree pointing
+    // at the document it left, which the dump below therefore does not hold.
+    live_root = PyObject_CallMethod(tree, "getroot", NULL);
+    if (live_root == NULL) {
+        goto ON_FAIL;
+    }
     // The whole tree is dumped either way here, so only the external subset
     // matters: it has to be loaded for the copy to know the IDs it declares.
     bytes = PyObject_CallFunctionObjArgs(PyXmlSec_LxmlEtreeToString, tree, NULL);
@@ -1632,6 +1727,9 @@ int PyXmlSec_LxmlShadowBeginDoc(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElemen
         goto ON_FAIL;
     }
     shadow->root = xmlDocGetRootElement(shadow->doc);
+    if (live_root != cur && PyXmlSec_LxmlShadowUnlinkedCopy(shadow, cur) < 0) {
+        goto ON_FAIL;
+    }
     if (PyXmlSec_LxmlShadowMark(shadow) < 0) {
         goto ON_FAIL;
     }
@@ -1649,14 +1747,23 @@ int PyXmlSec_LxmlShadowBeginDoc(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElemen
     cur = NULL;
 
     // The registered IDs live in lxml's document, which the copy knows
-    // nothing about; replay them so that #id references resolve.
-    if (PyXmlSec_LxmlShadowReplayIds(shadow) < 0) {
+    // nothing about; replay them so that #id references resolve. An unlinked
+    // subtree carries its own registrations, and the document it left keeps
+    // the rest — a reference from the subtree into that document resolves on
+    // the raw path too.
+    if (PyXmlSec_LxmlShadowReplayIds(shadow, (PyObject*)shadow->element, shadow->root) < 0) {
         goto ON_FAIL;
     }
+    if (shadow->unlinked
+        && PyXmlSec_LxmlShadowReplayIds(shadow, live_root, xmlDocGetRootElement(shadow->doc)) < 0) {
+        goto ON_FAIL;
+    }
+    Py_CLEAR(live_root);
     return 0;
 
 ON_FAIL:
     Py_XDECREF(cur);
+    Py_XDECREF(live_root);
     Py_XDECREF(tree);
     Py_XDECREF(bytes);
     Py_XDECREF(url_holder);
@@ -1993,20 +2100,27 @@ static int PyXmlSec_LxmlShadowReflectSites(PyXmlSec_LxmlShadow* shadow) {
     int rv = -1;
 
     // Re-fetch the root: replacement operations may swap nodes at the top.
-    list.top = xmlDocGetRootElement(shadow->doc);
+    // An unlinked subtree cannot be replaced at all (libxml2 needs a parent
+    // to put the replacement in), so its top is the one the shadow made.
+    list.top = shadow->unlinked ? shadow->root : xmlDocGetRootElement(shadow->doc);
     if (list.top == NULL) {
         PyErr_SetString(PyXmlSec_InternalError, "unexpected mutation site.");
         goto DONE;
     }
     if (!PYXMLSEC_SHADOW_TAGGED(shadow, list.top)) {
+        xmlNodePtr n;
+        int elements = 0;
+        int others = 0;
+
+        if (shadow->unlinked) {
+            PyErr_SetString(PyXmlSec_InternalError, "unexpected mutation site.");
+            goto DONE;
+        }
         // A fresh root: the call replaced the root itself, so there are no
         // sites to graft — the live root is morphed into it wholesale. lxml
         // holds one element (plus comments and PIs) at document level, so a
         // root replaced by anything else (a Type=Content decryption of the
         // root) cannot be reflected.
-        xmlNodePtr n;
-        int elements = 0;
-        int others = 0;
         for (n = shadow->doc->children; n != NULL; n = n->next) {
             if (n->type == XML_ELEMENT_NODE) {
                 ++elements;

src/lxml.h

@@ -90,6 +90,8 @@ typedef struct {
     xmlNodePtr root;                  // doc's root element, the copy of `element`; NULL for BeginNewDoc
     PyXmlSec_LxmlShadowTag* tags;     // one per pre-existing node of `doc`, owned by the shadow
     int ntags;
+    int unlinked;                     // `root` hangs in `doc` outside its tree (BeginDoc of an element
+                                      // removed from its document); the shadow frees it itself
 } PyXmlSec_LxmlShadow;
 
 // Subtree copy: `shadow.root` is the copy of `element`. When the document
@@ -105,6 +107,11 @@ int PyXmlSec_LxmlShadowBegin(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElementPt
 // `*target` receives the copy's counterpart of `element` (the live node
 // itself on the fast path); `shadow.root` / `shadow.element` become the copy
 // root / the live root, which is what the End functions map paths between.
+// An element removed from its document keeps pointing at it, and so does the
+// raw path — xmlsec works on the unlinked node while the document it left
+// still answers its `#id` references. The copy holds both: the document, and
+// the unlinked subtree copied into it beside the tree (`shadow.unlinked`,
+// with `shadow.root` / `shadow.element` the subtree's top on either side).
 int PyXmlSec_LxmlShadowBeginDoc(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElementPtr element, xmlNodePtr* target);
 
 // Create shape (template.create, encrypted_data_create): the call only needs