Details
-
Bug
-
Status: Open
-
Minor
-
Resolution: Unresolved
-
3.0.5
-
None
-
Encounted on MacOS Catalina, which I don't believe is relevant. Using AdoptOpenJDK 14 throughout, which should also not be relevant.
-
Patch
Description
The bug is in StreamingSAXBuilder.groovy line 81 (in 3.0.5 and master as of now), in tagClosure, where the line:
pendingStack.add pendingNamespaces.clone()
should read:
pendingStack.push pendingNamespaces.clone()
... to match the pendingStack.pop() happening a few lines later, after processing the tag body. So the current pendingNamespaces map is being added to the end of the stack, and then popped off the beginning. Which means the wrong map is being popped off, and can result in the next element being given those namespaces as if it was declaring them anew.
I modified the testDefaultSerializationNamespaces in StreamingSAXBuilderTest.groovy to add two inner elements to demonstrate this, so the test now had:
def doc = new StreamingSAXBuilder().bind {
mkp.declareNamespace("" : "uri:urn1")
mkp.declareNamespace("x" : "uri:urn2")
outer {
'x:inner'('hello')
'x:inner'('hello again')
}
}
def expected = '<outer xmlns="uri:urn1" xmlns:x="uri:urn2"><x:inner>hello</x:inner><x:inner>hello again</x:inner></outer>'
The test still passes at this point because the XmlUnit/Diff implementations elide the extra namespace declarations away. If you actually print the resulting XML you can see the problem:
<outer xmlns="uri:urn1" xmlns:x="uri:urn2"><x:inner>hello</x:inner><x:inner xmlns="uri:urn1" xmlns:x="uri:urn2">hello again</x:inner></outer>
I think this will result in larger problems with more complicated XMLs being built.
So I added an assertion to compare the output string with the expected value, to reveal the problem. This necessitated (for now) an annoying change in StreamingSAXBuilder to sort the pending namespaces map before processing them, so they would appear in a predictable order. (BaseMarkupBuilder uses a HashMap - not sure what permanent solution you'd prefer there, seeing as it's just for the string-comparison test to run. It might be to write a separate test that only tests with one added namespace, for instance.)
Attachment is a diff based on current-master to include this test in StreamingSAXBuilderTest.groovy and the changes in StreamingSAXBuilder.groovy: the sort() which is only there for the test, and changing the pendingStack.add to a pendingStack.push, which fixes the problem, so the test passes. Revert it to pendingStack.add to see it fail.
