Bug 63436

correct code for JUnit Task starts here:

https://github.com/apache/ant/blob/16d0f4d811220bf60b27d6402a7732bbaec81186/src/main/org/apache/tools/ant/taskdefs/optional/junit/XMLJUnitResultFormatter.java#L327

---------------

incorrect code for JUnitLauncher Task starts here:

https://github.com/apache/ant/blob/440ef41bb99cdbbb66d10b59349a1efbf8ab7111/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatter.java#L311

Created attachment 36591 [details]
PATCH with bugfix

Thank you for reporting this and the patch. However, the reason it's been coded that way was to avoid loading the entire system.out/system.err messages into memory and potentially running into OOM. This was the motivation to have (although a bit involved logic) it stream instead of holding the whole content in memory and then writing it into that report. This isn't a theoretical case either since issues like these OOMs have been reported in our JUnit task of Ant.

Having said that, the XML is expected to be well formed as well as usable by the junitreport. Can you please attach a sample xml which got generated and which is causing issues with the junitreport task?

(In reply to Jaikiran Pai from comment #3)
> Thank you for reporting this and the patch. However, the reason it's been
> coded that way was to avoid loading the entire system.out/system.err
> messages into memory and potentially running into OOM. This was the
> motivation to have (although a bit involved logic) it stream instead of
> holding the whole content in memory and then writing it into that report.
> This isn't a theoretical case either since issues like these OOMs have been
> reported in our JUnit task of Ant.
> 
> Having said that, the XML is expected to be well formed as well as usable by
> the junitreport. Can you please attach a sample xml which got generated and
> which is causing issues with the junitreport task?

You're right, XML "Files" inside sysout/err are escaped correct in junitreport task.

Maybe my problems came all from https://bz.apache.org/bugzilla/show_bug.cgi?id=63446

Anyway, even JUnit 5's org.junit.platform.reporting.legacy.xml.LegacyXmlReportGeneratingListener is using CDATA for sysout/err. 
And they do it in a way without having sysout/err in memory.

Please see:
https://github.com/junit-team/junit5/blob/master/junit-platform-reporting/src/main/java/org/junit/platform/reporting/legacy/xml/XmlReportWriter.java

We are using DOMElementWriter.encode and add XMLStreamWriter.writCharacters on top of it, which may lead to double-escaped & characters as the code currently stands. If I read the code correctly encode will turn a < into &lt; and writeCharacters will turn that into &amp;lt; - will have to check.

Simply using CDATA instead of writeCharacters will not help as you need to make sure the characters you write do not contain the sequence ]]>. 

Michael can you give us an example of something that went wring with the existing code? I believe we should not invoke encode but leave that job to writeCharacters but we do need to strip out characters illegal in XML - see bug 65030.

I performed some experiments and writeCharacters by itself takes care of replacing ampersands and friends, so encoding ourselves is hurting rather than helping. IMHO we should only deal with the characters illegal in XML (see bug #65030 ) and leave the rest of the input alone.

And of course, if there are additional problems, we should solve them.

If I'm guessing correctly that double-encoding of sysout/err has been a problem, which I've fixed in master - so I'm closing this now.