-
Notifications
You must be signed in to change notification settings - Fork 104
Comparing changes
Open a pull request
base repository: xmlsec/python-xmlsec
base: master
head repository: xmlsec/python-xmlsec
compare: fix/356-shadow-copy
- 5 commits
- 13 files changed
- 2 contributors
Commits on Jul 2, 2026
-
Decouple lxml from xmlsec via shadow copies (#356)
python-xmlsec hands lxml's raw libxml2 node pointers straight to xmlsec1. That only works when lxml and xmlsec link the same libxml2 at runtime; when they differ (e.g. lxml's bundled libxml2 vs a system/homebrew one), mixing the two libraries' nodes corrupts memory and segfaults. Rework the template functions to run each xmlsec call on a private "shadow" copy of the element: PyXmlSec_LxmlShadowBegin serializes the element with lxml's own libxml2 and re-parses the bytes with ours, the xmlsec call mutates that copy, and PyXmlSec_LxmlShadowEnd reflects the change back into the live lxml tree. Only bytes ever cross the boundary, never node pointers. Converting a function is four lines (Begin / the unchanged xmlsec call on shadow.root / End) with no per-function callback or context struct. End detects what the call did generically, by tagging pre-existing nodes through the libxml2 _private field, and covers the whole xmlSecTmpl* family: - plain adds graft the new subtree at the position xmlsec chose, - calls that create intermediate ancestors (add_transform's <Transforms>) graft the topmost new node and return the inner one, - find-or-create calls (ensure_key_info) return the existing live node and mirror any attributes set on it, instead of duplicating it. Reflection dumps the whole mutated copy, not just the new node, so ancestor-declared namespaces and xmlsec's "\n" formatting siblings survive the round-trip and signatures stay byte-identical; the one text slot xmlsec may touch before the new node (parent text / previous sibling tail) is mirrored explicitly. Child indices count exactly the node types lxml exposes as children, so comments/PIs in templates don't skew paths. add_reference, add_transform and ensure_key_info - one per reflect shape - are converted; the rest of template.c is mechanical follow-up. ds.c, enc.c and tree.c still pass raw nodes, so the import-time version guard stays, now with a PYXMLSEC_SKIP_VERSION_CHECK opt-out used to exercise the shadow paths under a mismatch. Validated under a real 2.14<->2.15 libxml2 mismatch: full suite green (288 passed) including the per-test leak detector, plus a 10k-iteration loop over the three converted functions with no crash, no RSS growth and byte-identical output. See developer.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Configuration menu - View commit details
-
Copy full SHA for b620369 - Browse repository at this point
Copy the full SHA b620369View commit details -
Add step-by-step guide for converting functions to shadow copies
developer.md explains why the shadow copy exists and how the reflection works; converting-functions.md is the operational companion: pick a function, classify the xmlSecTmpl* call against the shapes the reflect covers (including the int-returning and detached-create shapes that need extra care), apply the mechanical binding edit, and the test / mismatch-validation checklist — including the tests/base.py leak detector gotcha. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Configuration menu - View commit details
-
Copy full SHA for 9a7b1a6 - Browse repository at this point
Copy the full SHA 9a7b1a6View commit details -
Add high-level summary of the shadow-copy solution for #356
The problem, the shadow-copy idea, what the change consists of, why this design replaced the first (op/ctx) attempt, validation results, and what remains. Entry point to developer.md (design detail) and converting-functions.md (rollout how-to). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Configuration menu - View commit details
-
Copy full SHA for e939f1e - Browse repository at this point
Copy the full SHA e939f1eView commit details -
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>
Configuration menu - View commit details
-
Copy full SHA for 72a78cb - Browse repository at this point
Copy the full SHA 72a78cbView commit details
Commits on Jul 7, 2026
-
Convert every remaining raw-node crossing to shadow copies (#356)
Roll the shadow pattern out beyond templates so no binding hands an lxml node to xmlsec anymore (fast path unchanged on matched libxml2): - template.c: all remaining functions, incl. the create shape (BeginNewDoc/EndNewDoc builds the detached template in a private doc) and the status-int C14N helper (FindFresh locates the created node). - tree.c: finders map results back by path (EndFind, None on not-found; find_parent shadows the whole tree); add_ids records id-attribute specs instead of writing lxml's ID hash with our libxml2. - ds.c: sign/verify run on a whole-document copy (BeginDoc) with the recorded IDs replayed so #id references resolve; sign reflects all mutation sites (DigestValue/SignatureValue/KeyInfo) via ReflectAll, verify just discards the copy. - enc.c: encrypt_binary/encrypt_uri reflect the mutated template; encrypt_xml/decrypt re-parse everything into one copy and reflect the replacement through lxml (element, content, or returned bytes). Replacing the document root cannot be expressed through lxml's API and raises a clear error on the shadow path. ReflectAll is two-phase (prefetch payloads from the re-parsed copy in its final state, then apply to the live tree in document order) because each graft moves a node out of the copy and would invalidate the indices later sites resolve through. Two template tests now attach the created template before asserting liveness: a shadow-created template lives in its own document until grafted, as lxml cannot express the raw path's "detached node inside an existing document". Validated under a real 2.14<->2.15 mismatch (full suite, 10k-iteration sign/verify/encrypt/decrypt loop, flat RSS, byte-identical output) and on a matched static wheel with and without PYXMLSEC_FORCE_SHADOW. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Configuration menu - View commit details
-
Copy full SHA for 92d68ad - Browse repository at this point
Copy the full SHA 92d68adView commit details
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff master...fix/356-shadow-copy