Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOM Node serialization omits the default namespace declaration #3174

Merged
merged 1 commit into from Jun 11, 2021

Conversation

magott
Copy link
Contributor

@magott magott commented Jun 10, 2021

When serializing xml with a default namespace (eg: <root xmlns='http://foo'/>) the default namespace is omitted when using the DOMImplementationLS for serialization. This seem to be a change that has happened in the JDK around JDK11. By switching the serialization method to using the Transformer api, the default namespace is included in the xml string.

The PR has a test that verifies the issue on Java15.

I didn't have java14 so I had to do some modification to build the project on my machine using Java15, so this has to be verified on your build setup before you consider merging it.

@cowtowncoder
Copy link
Member

Sounds reasonable, thank you for contributing this!

Aside from one minor comment I made, one question: can you think of any security issues wrt using default Transformer? With JDK XML functionality there have been a few reports about default setting of expanding external general entities and such -- I don't think those should apply here (since this is not about parsing but generation) but thought I'd ask just in case.

Another thing I'd need (unless already asked) would be CLA: it only needs to be sent once, and can found here:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and is often done by printing the doc, filling & signing, scan/photo, email to info at fasterxml dot com.
Once that is done CLA is good for all contributions (so if you have done it before just let me know and I can verify, no need to send again).

Thank you again!

@magott
Copy link
Contributor Author

magott commented Jun 10, 2021

I don't see how those issues with XML Transformer would come to play when generating a string, but I added the secure processing flag to the transformer anyway, just to be sure 😄

CLA signed and e-mailed

@cowtowncoder
Copy link
Member

Agreed: thank you for adding that setting: shouldn't hurt and might help for something non-obvious.
I can see the CLA too, thanks!

@cowtowncoder cowtowncoder merged commit 5e82b9e into FasterXML:2.13 Jun 11, 2021
@cowtowncoder cowtowncoder added the JDK11 Features that need JDK11 (and/or support JDK11) label Jun 11, 2021
@cowtowncoder cowtowncoder changed the title Xml to json serializing omits default namespace DOM Node serialization omits the default namespace declaration Jun 11, 2021
cowtowncoder added a commit that referenced this pull request Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JDK11 Features that need JDK11 (and/or support JDK11)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants