Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: xmlsec/python-xmlsec
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: master
Choose a base ref
...
head repository: xmlsec/python-xmlsec
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: fix/356-shadow-copy
Choose a head ref
Checking mergeability… Don’t worry, you can still create the pull request.
  • 5 commits
  • 13 files changed
  • 2 contributors

Commits on Jul 2, 2026

  1. 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>
    mxamin and claude committed Jul 2, 2026
    Configuration menu
    Copy the full SHA
    b620369 View commit details
    Browse the repository at this point in the history
  2. 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>
    mxamin and claude committed Jul 2, 2026
    Configuration menu
    Copy the full SHA
    9a7b1a6 View commit details
    Browse the repository at this point in the history
  3. 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>
    mxamin and claude committed Jul 2, 2026
    Configuration menu
    Copy the full SHA
    e939f1e View commit details
    Browse the repository at this point in the history
  4. 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>
    mxamin and claude committed Jul 2, 2026
    Configuration menu
    Copy the full SHA
    72a78cb View commit details
    Browse the repository at this point in the history

Commits on Jul 7, 2026

  1. 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>
    mxamin and claude committed Jul 7, 2026
    Configuration menu
    Copy the full SHA
    92d68ad View commit details
    Browse the repository at this point in the history
Loading