[ENHANCEMENT]: Make signature-emission template methods `protected` so subclasses can customize output without type suppressions

[msheby]

Is your feature request related to a problem? Please describe...

Summary

SignedXml has a natural template-method shape — computeSignature calls createSignedInfo, which calls createReferences, which calls getCanonReferenceXml, which calls the (public) getCanonXml; separately computeSignature calls getKeyInfo and createSignature. In v6 every one of those intermediate methods is declared private, which makes the class impossible to subclass cleanly in strict TypeScript: any override triggers TS4114 (must have 'override' modifier) and adding override triggers TS2415 (private in base class). The only way to keep the subclass compiling is @ts-expect-error or an intersection-type cast — both of which sidestep type safety rather than express the design.

The proposed change is a one-line-per-method visibility change from private to protected on the methods that participate in the template. No behavior change; consumers who don't subclass see no difference. Consumers who do subclass can now express legitimate customizations in typed code.

Motivating cases

• Pretty-printing / spec-mandated formatting of SignedInfo and its children. Post-processing the emitted signature to reformat <SignedInfo> isn't safe — non-exclusive C14N preserves inter-element whitespace as text nodes, so any post-hoc reformat changes the canonical form and invalidates the already-computed SignatureValue. The only correct place to intervene is inside the same chain that produces the pre-canonicalization string, which today means overriding createSignedInfo (private) or createReferences (private).

• Reference-emission customization. Some SMPTE standards require that Reference elements not contain a <Transforms> child at all. The digest still has to be computed under C14N; only the emission changes. Doing this cleanly needs a createReferences override, which is currently blocked.

• Custom getKeyInfo output (e.g. emitting a full X.509 certificate chain rather than a single cert) — the getKeyInfoContent callback covers the content, but a subclass that wants to alter attribute ordering, whitespace, or attach Id attributes to the outer <KeyInfo> element needs the wrapping method.

• Repro-friendly algorithm lookups in tests. The findSignatureAlgorithm / findCanonicalizationAlgorithm / findHashAlgorithm helpers are the natural stub points for deterministic tests of subclass logic; keeping them private forces test subclasses into the same visibility gymnastics.

Describe teh solution you'd like...

Concrete ask

Change visibility from private to protected on the following methods in src/signed-xml.ts (line numbers from v6.1.2):

Method	Line	Role
getCanonSignedInfoXml	383	Called by calculateSignatureValue; entry point for SignedInfo C14N
getCanonReferenceXml	425	Per-reference C14N; entry point for reference digest computation
calculateSignatureValue	441	Signature computation; useful for algorithm-injection subclasses
findSignatureAlgorithm	454	Algorithm lookup
findCanonicalizationAlgorithm	466	Algorithm lookup
findHashAlgorithm	477	Algorithm lookup
loadReference	699	Per-reference load during verify; parallel to createReferences
getKeyInfo	1055	Emits the <KeyInfo> wrapper
createReferences	1077	Emits <Reference> elements
ensureHasId	1163	Id-attribute placement
createSignedInfo	1208	Emits <SignedInfo>
createSignature	1243	Emits the wrapping <Signature> element

The state fields those methods touch (this.signatureNode, this.references, this.signatureValue, etc.) can stay private — subclasses that need them can be added incrementally.

Compatibility

private → protected is a source-compatible widening. Existing consumers that don't subclass are unaffected. Existing subclasses (if any) that were already relying on TypeScript workarounds can drop those workarounds. The compiled JavaScript is byte-identical.

Related

• Separate defect: findNSPrefix returns only the first xmlns declaration → duplicate default xmlns in non-exclusive C14N when subset declares any xmlns:* #538 (findNSPrefix returns only the first xmlns[:*] attribute).

Describe the alternatives you've considered...

Forking the type declarations -- ugh.

[cjbarth]

That is a lot of changes and I'm not sure they would all be required. First, #540 should address part of this concern. For the others, can you cite which SMPTE standards require a transform to be applied, but not specified? That seems broken on their end.

For your getKeyInfo concern, that is why we have an interface for getKeyInfoContent. Does that work for you or am I missing something?

Are there specific tests you have in mind that are made more complex by our current structure?

[msheby]

Thanks for the quick response!

SMPTE ST 430-3:2012 §8.2 is the relevant normative text — it states verbatim: "The Transforms field of the Reference elements shall be omitted, meaning the bytes of the referenced node are hashed without any transformations." So the spec doesn't require a transform applied implicitly; it requires <Transforms> to be absent entirely. The problem is the output side: xml-crypto currently emits <Transforms></Transforms> when addReference is called with an empty array, which violates this. You're right that #540 is the fix for that — we agree it covers this concern and we've submitted a PR for #538 as well.

On getKeyInfoContent: That interface controls the inner content but not the indentation of the outer <KeyInfo> wrapper element itself. Our override is purely cosmetic — D-Cinema XML files are frequently read manually by operators and engineers, so consistent indentation throughout the signature block is useful in practice. That said, if this is the only concern with getKeyInfo, we'd be happy to drop it from the request.

On scope: You're right that not all 11 methods are strictly necessary. The minimum set that would let us eliminate the as unknown as type casts in our subclass is:

• getCanonXml — fixes a C14N serialization bug (self-closing elements after enveloped-signature); notably this method isn't declared in the type definitions at all today
• getCanonReferenceXml — fixes the duplicate default namespace issue from findNSPrefix returns only the first xmlns declaration → duplicate default xmlns in non-exclusive C14N when subset declares any xmlns:* #538
• createSignedInfo, createSignature, createReferences — pretty-printing for human-readable ETM output
• findCanonicalizationAlgorithm, findSignatureAlgorithm — called from the above overrides

We're happy to narrow the PR to just these if that's preferable.

On tests: The main complexity today is that algorithm lookups (findCanonicalizationAlgorithm, findSignatureAlgorithm) are private, so unit tests that want to inject a deterministic stub algorithm can't do so cleanly without the same as unknown as workarounds. Making them protected would allow straightforward test subclasses.

[cjbarth]

What tests are you trying to write? Would they be better suited to being directly in this project? That might make several things easier.

As for pretty-printing, I feel like that can easily be done on the reviewer's copy of the XML while leaving the signed copy alone.

As for the first two issues, is this something we can include in this project?