Skip to content

Remove originalXmlWithIds #512

Description

@shunkica

This is a proposal to remove the originalXmlWithIds property and the getOriginalXmlWithIds() function from the library.

In the documentation it says it is for "validation" but that seems to be a misinterpretation of the original intent.

It seems to be a relic from the first commit, when there was no way to specify where you wanted to insert the signature.
But since we now have the ComputeSignatureOptionsLocation where the location of the signature can be specified, there seems to no longer be a need for this function.

It also creates a copy of the entire XML string inside the library, which is an unnecessary memory cost, especially when working with large XML files.

Within the library this is only used for some tests regarding Id generation.
These tests could be rewritten to be DOM aware and work on the signed document.

For example this:

    const originalXmlWithIds = sig.getOriginalXmlWithIds();
    const expectedOriginalXmlWithIds =
      '<root><x xmlns="ns" Id="_0"/><y attr="value" Id="_1"/><z><w Id="_2"/></z></root>';
    expect(originalXmlWithIds, "wrong OriginalXmlWithIds").to.equal(expectedOriginalXmlWithIds);

could be refactored to this:

    const signedDoc = new xmldom.DOMParser().parseFromString(signedXml);
    const xId = xpath.select1("//*[local-name(.)='x']/@*[local-name(.)='Id']", signedDoc);
    isDomNode.assertIsAttributeNode(xId);
    expect(xId.textContent).to.equal("_0");
    const yId = xpath.select1("//*[local-name(.)='y']/@*[local-name(.)='Id']", signedDoc);
    isDomNode.assertIsAttributeNode(yId);
    expect(yId.textContent).to.equal("_1");
    const wId = xpath.select1("//*[local-name(.)='w']/@*[local-name(.)='Id']", signedDoc);
    isDomNode.assertIsAttributeNode(wId);
    expect(wId.textContent).to.equal("_2");

I am willing to do this as part of my PR #506 if this is acceptable.

Activity

  1. cjbarth commented on Oct 17, 2025

    @cjbarth
    Contributor

    If the tests can be re-written to not use this property, we can do that now (ideally in its own PR). Then, we can have another PR to actually remove the property in an upcoming breaking change. That would be very welcome.

  2. added a commit that references this issue on Sep 8, 2026
  3. added this to the v7.0 milestone on Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions