Rachel created GROOVY-9698:
------------------------------
Summary: StreamingSAXBuilder repeats xmlns declarations on inner
elements
Key: GROOVY-9698
URL: https://issues.apache.org/jira/browse/GROOVY-9698
Project: Groovy
Issue Type: Bug
Components: XML Processing
Affects Versions: 3.0.5
Environment: Encounted on MacOS Catalina, which I don't believe is
relevant. Using AdoptOpenJDK 14 throughout, which should also not be relevant.
Reporter: Rachel
Attachments: saxbuilder.diff
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.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
