Skip the shadow copy when lxml links the same libxml2

The import guard only lets matched libxml2 versions run, and for them the
old direct behavior — xmlsec mutating lxml's nodes — is safe; that is
what shipped for years. Yet every converted function paid the full shadow
round-trip (serialize with lxml, re-parse, dump, re-parse with lxml) even
in that case, four serializations per call for zero safety benefit. That
is noise for the small xmlSecTmpl* trees, but would become a real
regression when the pattern reaches sign/encrypt on whole documents.

Make Begin/End dual-path, decided once at import: on matched versions
Begin aliases the live _c_node (no copy) and End just wraps the node
xmlsec returned, machine-identical to the pre-shadow code; the shadow
round-trip activates only under a mismatch — or when PYXMLSEC_FORCE_SHADOW
is set, which CI now uses to run the suite a second time so the shadow
path stays exercised on matched builds. Call sites cannot tell the
difference, and every function converted later inherits both paths.

While in there, resolve lxml.etree's tostring/fromstring once at module
init instead of importing lxml.etree on every shadow crossing.

Benchmark (matched static build, create + add_reference + add_transform +
ensure_key_info per iteration): fast path 8.6us vs shadow 72.3us, ~8x.
Validated three ways, with byte-identical template output across the two
paths: full suite on the fast path and on PYXMLSEC_FORCE_SHADOW=1 (matched
static build, 300 passed / 6 skipped each), and full suite under a real
2.14 vs 2.15 mismatch (288 passed, 6 skipped).

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

Author: mxamin

.github/scripts/manylinux_build_and_test.sh

@@ -56,6 +56,11 @@ echo "== [container] Step: Install test dependencies =="
 echo "== [container] Step: Run tests =="
 /opt/python/${PY_ABI}/bin/pytest -v --color=yes
 
+# Step: Run tests again on the shadow-copy path (issue #356), which the
+# matched libxml2 build would otherwise never exercise.
+echo "== [container] Step: Run tests (forced shadow path) =="
+PYXMLSEC_FORCE_SHADOW=1 /opt/python/${PY_ABI}/bin/pytest -v --color=yes
+
 # Step: Fix mounted workspace file ownership on host
 echo "== [container] Step: Fix mounted workspace file ownership on host =="
 chown -R "${HOST_UID}:${HOST_GID}" dist wheelhouse build libs || true

.github/workflows/linuxbrew.yml

@@ -48,3 +48,6 @@ jobs:
           pip3 install --upgrade --no-binary=lxml -r requirements-test.txt
           pip3 install xmlsec --only-binary=xmlsec --no-index --find-links=dist/
           pytest -v --color=yes
+          # Same suite on the shadow-copy path (issue #356), which the
+          # matched libxml2 build would otherwise never exercise.
+          PYXMLSEC_FORCE_SHADOW=1 pytest -v --color=yes

.github/workflows/macosx.yml

@@ -87,6 +87,14 @@ jobs:
         run: |
           coverage run -m pytest -v --color=yes
 
+      # Same suite on the shadow-copy path (issue #356), which the matched
+      # libxml2 build would otherwise never exercise.
+      - name: Run tests (forced shadow path)
+        env:
+          PYXMLSEC_FORCE_SHADOW: "1"
+        run: |
+          pytest -v --color=yes
+
       - name: Report coverage to codecov
         if: matrix.static_deps != 'static'
         run: |

356-summary.md

@@ -53,6 +53,12 @@ byte-identical — namespaces and xmlsec's `"\n"` formatting included.
     synced) instead of duplicating it.
   A conversion is four lines: `Begin` / the unchanged xmlsec call on
   `shadow.root` / `End` — no per-function callback or context struct.
+- **A fast path** decided once at import: when lxml links the same libxml2 as
+  the extension (the only configuration the import guard lets run today),
+  `Begin`/`End` skip the copy entirely and behave exactly like the old direct
+  code — zero overhead. The shadow round-trip activates under a mismatch, or
+  with `PYXMLSEC_FORCE_SHADOW=1`, which CI uses to keep the shadow path
+  exercised on matched libraries.
 - **An escape hatch** for development: `PYXMLSEC_SKIP_VERSION_CHECK` bypasses
   the import-time mismatch guard so the converted paths can be exercised under
   a real mismatch. The guard itself stays on by default.

developer.md

@@ -59,6 +59,26 @@ call mutates at most one place in the tree — true for all `xmlSecTmpl*`
 functions. Whole-document operations (`sign`, `encrypt`) mutate many places
 and will need a different reflect step on top of the same Begin machinery.
 
+## The fast path: shadows only when needed
+
+Copying is pointless when lxml links the same libxml2 as the extension — the
+raw-node behavior that shipped for years is safe then, and it is the only
+configuration the import guard currently lets run. So `Begin`/`End` are
+dual-path, decided once at import:
+
+- **matched versions** (the guard passed): `Begin` aliases the live
+  `_c_node` into `shadow.root` with no serialization, and `End` just wraps
+  the node xmlsec returned — machine-identical to the pre-shadow code, zero
+  overhead;
+- **mismatch** (import allowed via `PYXMLSEC_SKIP_VERSION_CHECK` today,
+  automatic once everything is converted), **or `PYXMLSEC_FORCE_SHADOW` set**:
+  the full shadow round-trip described above.
+
+Call sites cannot tell the difference; every converted function inherits both
+paths. `PYXMLSEC_FORCE_SHADOW` exists so CI keeps the shadow path exercised on
+matched libraries (see the test matrix), where it must also pass the full
+suite.
+
 > The shadow decouples *lxml* from xmlsec. The extension and `libxmlsec1`
 > must still share one libxml2 (wheels/static builds guarantee this).
 
@@ -88,6 +108,10 @@ exists to exercise the shadow paths; it is **unsafe** for every operation
 still on the raw-node path, so keep it off in normal use. The guard can only
 be relaxed once all node-passing paths are converted.
 
+On a *matched* build (no mismatch available), run the suite twice instead:
+once plain (fast path) and once with `PYXMLSEC_FORCE_SHADOW=1` (shadow path
+on matched libraries — safe everywhere, so the whole suite must pass).
+
 ## Status
 
 - ✅ `template.add_reference`, `template.add_transform`,

src/lxml.c

@@ -101,18 +101,46 @@ static int PyXmlSec_CheckLxmlLibraryVersion(void) {
     return result;
 }
 
+// Non-zero when converted functions must run their xmlsec call on a shadow
+// copy instead of directly on lxml's nodes; decided once at import, below.
+static int PyXmlSec_LxmlShadowActive = 1;
+
+// The two lxml.etree callables the shadow crossings use, resolved once at
+// import and kept for the lifetime of the process.
+static PyObject* PyXmlSec_LxmlEtreeToString;
+static PyObject* PyXmlSec_LxmlEtreeFromString;
+
 int PyXmlSec_InitLxmlModule(void) {
     // By default refuse to import when lxml and xmlsec link different libxml2
     // versions: passing raw nodes between the two libraries then corrupts
     // memory (https://github.com/xmlsec/python-xmlsec/issues/283). Setting
     // PYXMLSEC_SKIP_VERSION_CHECK bypasses the guard — needed to exercise the
     // shadow-copy paths (issue #356) under a mismatch, but unsafe for every
     // operation that still hands an lxml node to xmlsec.
-    if (PyXmlSec_CheckLxmlLibraryVersion() < 0 && getenv("PYXMLSEC_SKIP_VERSION_CHECK") == NULL) {
+    int mismatch = PyXmlSec_CheckLxmlLibraryVersion() < 0;
+    if (mismatch && getenv("PYXMLSEC_SKIP_VERSION_CHECK") == NULL) {
         PyXmlSec_SetLastError("lxml & xmlsec libxml2 library version mismatch");
         return -1;
     }
 
+    // Matched versions are the long-standing status quo: xmlsec may mutate
+    // lxml's nodes directly, so converted functions skip the copy (the
+    // Begin/End fast path). Shadows turn on under a mismatch — or always with
+    // PYXMLSEC_FORCE_SHADOW, the knob CI uses to exercise the shadow path on
+    // matched libraries.
+    PyXmlSec_LxmlShadowActive = mismatch || getenv("PYXMLSEC_FORCE_SHADOW") != NULL;
+
+    PyObject* etree = PyImport_ImportModule("lxml.etree");
+    if (etree == NULL) {
+        return -1;
+    }
+    PyXmlSec_LxmlEtreeToString = PyObject_GetAttrString(etree, "tostring");
+    PyXmlSec_LxmlEtreeFromString = PyObject_GetAttrString(etree, "fromstring");
+    Py_DECREF(etree);
+    if (PyXmlSec_LxmlEtreeToString == NULL || PyXmlSec_LxmlEtreeFromString == NULL) {
+        return -1;
+    }
+
     return import_lxml__etree();
 }
 
@@ -151,39 +179,19 @@ int PyXmlSec_LxmlElementConverter(PyObject* o, PyXmlSec_LxmlElementPtr* p) {
 // tree; with_tail keeps the serialization to the element itself.
 static PyObject* PyXmlSec_LxmlElementToBytes(PyObject* element) {
     PyObject* result = NULL;
-    PyObject* args = NULL;
-    PyObject* kwargs = NULL;
-    PyObject* tostring = NULL;
-    PyObject* etree = PyImport_ImportModule("lxml.etree");
-    if (etree == NULL) {
-        return NULL;
-    }
-    tostring = PyObject_GetAttrString(etree, "tostring");
-    Py_DECREF(etree);
-    if (tostring == NULL) {
-        return NULL;
-    }
-    args = PyTuple_Pack(1, element);
-    kwargs = Py_BuildValue("{s:O}", "with_tail", Py_False);
+    PyObject* args = PyTuple_Pack(1, element);
+    PyObject* kwargs = Py_BuildValue("{s:O}", "with_tail", Py_False);
     if (args != NULL && kwargs != NULL) {
-        result = PyObject_Call(tostring, args, kwargs);
+        result = PyObject_Call(PyXmlSec_LxmlEtreeToString, args, kwargs);
     }
     Py_XDECREF(args);
     Py_XDECREF(kwargs);
-    Py_DECREF(tostring);
     return result;
 }
 
 // etree.fromstring(data) — the parsed nodes are owned and managed by lxml.
 static PyObject* PyXmlSec_LxmlElementFromBytes(PyObject* data) {
-    PyObject* result;
-    PyObject* etree = PyImport_ImportModule("lxml.etree");
-    if (etree == NULL) {
-        return NULL;
-    }
-    result = PyObject_CallMethod(etree, "fromstring", "O", data);
-    Py_DECREF(etree);
-    return result;
+    return PyObject_CallFunctionObjArgs(PyXmlSec_LxmlEtreeFromString, data, NULL);
 }
 
 // Nodes that exist before the xmlsec call are tagged through the libxml2
@@ -295,6 +303,14 @@ int PyXmlSec_LxmlShadowBegin(PyXmlSec_LxmlShadow* shadow, PyXmlSec_LxmlElementPt
     shadow->doc = NULL;
     shadow->root = NULL;
 
+    // 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
+    // doc == NULL and just wraps the result node.
+    if (!PyXmlSec_LxmlShadowActive) {
+        shadow->root = element->_c_node;
+        return 0;
+    }
+
     bytes = PyXmlSec_LxmlElementToBytes((PyObject*)element);
     if (bytes == NULL || PyBytes_AsStringAndSize(bytes, &data, &size) < 0) {
         Py_XDECREF(bytes);
@@ -332,6 +348,15 @@ PyObject* PyXmlSec_LxmlShadowEnd(PyXmlSec_LxmlShadow* shadow, xmlNodePtr res, co
     int rel[PYXMLSEC_SHADOW_MAX_DEPTH];
     int depth, rel_depth, insert_idx;
 
+    // Fast path (no copy was made): res is a node in the live lxml tree.
+    if (shadow->doc == NULL) {
+        if (res == NULL) {
+            PyXmlSec_SetLastError(error);
+            return NULL;
+        }
+        return (PyObject*)PyXmlSec_elementFactory(shadow->element->_doc, res);
+    }
+
     if (res == NULL) {
         PyXmlSec_SetLastError(error);
         goto DONE;

src/lxml.h

@@ -52,6 +52,13 @@ int PyXmlSec_LxmlElementConverter(PyObject* o, PyXmlSec_LxmlElementPtr* p);
 // when the node is NULL). End must be called exactly once after a successful
 // Begin; it always releases the copy.
 //
+// Fast path: when lxml links the same libxml2 as this extension (the
+// import-time version check passed), no copy is needed — Begin aliases the
+// live node into `root` (leaving `doc` NULL) and End just wraps the result,
+// which is the long-standing direct behavior with zero overhead. Setting
+// PYXMLSEC_FORCE_SHADOW in the environment forces the shadow path even on
+// matched libraries; CI uses it to keep that path exercised.
+//
 // The reflection covers the whole xmlSecTmpl* family: a new subtree grafted at
 // the position xmlsec chose (including intermediate nodes like <Transforms>
 // and the "\n" formatting text around it), or — for find-or-create calls that