Skip to content

Support parent namespaces#17

Merged
microshine merged 5 commits into
PeculiarVentures:masterfrom
brainbeanapps:support_parent_namespaces
Nov 23, 2017
Merged

Support parent namespaces#17
microshine merged 5 commits into
PeculiarVentures:masterfrom
brainbeanapps:support_parent_namespaces

Conversation

@alexey-pelykh

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/signed_xml.ts Outdated
if (found) {
let el = found.cloneNode(true) as Element;
this.FixupNamespaceNodes(doc as Element, el, true);
this.CopyNamespaces(el, el, true);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexey-pelykh I updated this line to

this.CopyNamespaces(found, el, true);

I've got asic namespace on verifying

<xades:SignedProperties xmlns:asic="http://uri.etsi.org/02918/v1.2.1#" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:xades="http://uri.etsi.org/01903/v1.3.2#" Id="xades-id-a46287b499d8">

Could you try to update and test signing again?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would work, done

Comment thread src/signed_xml.ts
return namespaces
}

protected CopyNamespaces(src: Element, dst: Element, ignoreDefault: boolean): void {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexey-pelykh Could you add JSDOC?

@alexey-pelykh alexey-pelykh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@microshine added comments and adjusted code

Comment thread src/signed_xml.ts Outdated
if (found) {
let el = found.cloneNode(true) as Element;
this.FixupNamespaceNodes(doc as Element, el, true);
this.CopyNamespaces(el, el, true);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would work, done

@microshine microshine left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks ok. There are some TS issues, I'll fix them by myself
@alexey-pelykh thank you for xmldsigjs improvement

@microshine microshine merged commit cc67752 into PeculiarVentures:master Nov 23, 2017
@alexey-pelykh

Copy link
Copy Markdown
Contributor Author

Thanks for prompt approval! Sorry for TS-related issues

@alexey-pelykh alexey-pelykh deleted the support_parent_namespaces branch November 23, 2017 09:03
@microshine

Copy link
Copy Markdown
Collaborator

I published new version v2.0.15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants