encode characters illegal in XML in junitlauncher's report

also avoid double-encoding of sysout/err

Bugzilla Reports 63436 and 65030

WHATSNEW

@@ -11,6 +11,14 @@ Fixed bugs:
    the # character. This has now been fixed.
    Bugzilla Reports 64912, 64790
 
+ * made sure LegacyXmlResultFormatter encodes characters illegal in
+   XML the same way JUnit5's built-in formatter would.
+   Bugzilla Report 65030
+
+ * LegacyXmlResultFormatter no longer double-encodes <>& in system-err
+   and system-out
+   Bugzilla Report 63436
+
 Other changes:
 --------------
 

src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatter.java

@@ -199,16 +199,16 @@ class LegacyXmlResultFormatter extends AbstractJUnitResultFormatter implements T
             // write the testsuite element
             writer.writeStartElement(ELEM_TESTSUITE);
             final String testsuiteName = determineTestSuiteName();
-            writer.writeAttribute(ATTR_NAME, testsuiteName);
+            writeAttribute(writer, ATTR_NAME, testsuiteName);
             // time taken for the tests execution
-            writer.writeAttribute(ATTR_TIME, String.valueOf((testPlanEndedAt - testPlanStartedAt) / ONE_SECOND));
+            writeAttribute(writer, ATTR_TIME, String.valueOf((testPlanEndedAt - testPlanStartedAt) / ONE_SECOND));
             // add the timestamp of report generation
             final String timestamp = DateUtils.format(new Date(), DateUtils.ISO8601_DATETIME_PATTERN);
-            writer.writeAttribute(ATTR_TIMESTAMP, timestamp);
-            writer.writeAttribute(ATTR_NUM_TESTS, String.valueOf(numTestsRun.longValue()));
-            writer.writeAttribute(ATTR_NUM_FAILURES, String.valueOf(numTestsFailed.longValue()));
-            writer.writeAttribute(ATTR_NUM_SKIPPED, String.valueOf(numTestsSkipped.longValue()));
-            writer.writeAttribute(ATTR_NUM_ABORTED, String.valueOf(numTestsAborted.longValue()));
+            writeAttribute(writer, ATTR_TIMESTAMP, timestamp);
+            writeAttribute(writer, ATTR_NUM_TESTS, String.valueOf(numTestsRun.longValue()));
+            writeAttribute(writer, ATTR_NUM_FAILURES, String.valueOf(numTestsFailed.longValue()));
+            writeAttribute(writer, ATTR_NUM_SKIPPED, String.valueOf(numTestsSkipped.longValue()));
+            writeAttribute(writer, ATTR_NUM_ABORTED, String.valueOf(numTestsAborted.longValue()));
 
             // write the properties
             writeProperties(writer);
@@ -228,8 +228,8 @@ class LegacyXmlResultFormatter extends AbstractJUnitResultFormatter implements T
             writer.writeStartElement(ELEM_PROPERTIES);
             for (final String prop : properties.stringPropertyNames()) {
                 writer.writeStartElement(ELEM_PROPERTY);
-                writer.writeAttribute(ATTR_NAME, prop);
-                writer.writeAttribute(ATTR_VALUE, properties.getProperty(prop));
+                writeAttribute(writer, ATTR_NAME, prop);
+                writeAttribute(writer, ATTR_VALUE, properties.getProperty(prop));
                 writer.writeEndElement();
             }
             writer.writeEndElement();
@@ -257,11 +257,11 @@ class LegacyXmlResultFormatter extends AbstractJUnitResultFormatter implements T
                 }
                 final String classname = (parentClassSource.get()).getClassName();
                 writer.writeStartElement(ELEM_TESTCASE);
-                writer.writeAttribute(ATTR_CLASSNAME, classname);
-                writer.writeAttribute(ATTR_NAME, useLegacyReportingName ? testId.getLegacyReportingName()
+                writeAttribute(writer, ATTR_CLASSNAME, classname);
+                writeAttribute(writer, ATTR_NAME, useLegacyReportingName ? testId.getLegacyReportingName()
                         : testId.getDisplayName());
                 final Stats stats = entry.getValue();
-                writer.writeAttribute(ATTR_TIME, String.valueOf((stats.endedAt - stats.startedAt) / ONE_SECOND));
+                writeAttribute(writer, ATTR_TIME, String.valueOf((stats.endedAt - stats.startedAt) / ONE_SECOND));
                 // skipped element if the test was skipped
                 writeSkipped(writer, testId);
                 // failed element if the test failed
@@ -280,7 +280,7 @@ class LegacyXmlResultFormatter extends AbstractJUnitResultFormatter implements T
             writer.writeStartElement(ELEM_SKIPPED);
             final Optional<String> reason = skipped.get(testIdentifier);
             if (reason.isPresent()) {
-                writer.writeAttribute(ATTR_MESSAGE, reason.get());
+                writeAttribute(writer, ATTR_MESSAGE, reason.get());
             }
             writer.writeEndElement();
         }
@@ -295,9 +295,9 @@ class LegacyXmlResultFormatter extends AbstractJUnitResultFormatter implements T
                 final Throwable t = cause.get();
                 final String message = t.getMessage();
                 if (message != null && !message.trim().isEmpty()) {
-                    writer.writeAttribute(ATTR_MESSAGE, message);
+                    writeAttribute(writer, ATTR_MESSAGE, message);
                 }
-                writer.writeAttribute(ATTR_TYPE, t.getClass().getName());
+                writeAttribute(writer, ATTR_TYPE, t.getClass().getName());
                 // write out the stacktrace
                 writer.writeCData(StringUtils.getStackTrace(t));
             }
@@ -314,9 +314,9 @@ class LegacyXmlResultFormatter extends AbstractJUnitResultFormatter implements T
                 final Throwable t = cause.get();
                 final String message = t.getMessage();
                 if (message != null && !message.trim().isEmpty()) {
-                    writer.writeAttribute(ATTR_MESSAGE, message);
+                    writeAttribute(writer, ATTR_MESSAGE, message);
                 }
-                writer.writeAttribute(ATTR_TYPE, t.getClass().getName());
+                writeAttribute(writer, ATTR_TYPE, t.getClass().getName());
                 // write out the stacktrace
                 writer.writeCData(StringUtils.getStackTrace(t));
             }
@@ -349,15 +349,29 @@ class LegacyXmlResultFormatter extends AbstractJUnitResultFormatter implements T
             final char[] chars = new char[1024];
             int numRead = -1;
             while ((numRead = reader.read(chars)) != -1) {
-                // although it's called a DOMElementWriter, the encode method is just a
-                // straight forward XML util method which doesn't concern about whether
-                // DOM, SAX, StAX semantics.
-                // TODO: Perhaps make it a static method
-                final String encoded = new DOMElementWriter().encode(new String(chars, 0, numRead));
-                writer.writeCharacters(encoded);
+                writer.writeCharacters(encode(new String(chars, 0, numRead)));
             }
         }
 
+        private void writeAttribute(final XMLStreamWriter writer, final String name, final String value)
+            throws XMLStreamException {
+            writer.writeAttribute(name, encode(value));
+        }
+
+        private String encode(final String s) {
+            boolean changed = false;
+            final StringBuilder sb = new StringBuilder();
+            for (char c : s.toCharArray()) {
+                if (!DOMElementWriter.isLegalXmlCharacter(c)) {
+                    changed = true;
+                    sb.append("&#").append((int) c).append(';');
+                } else {
+                    sb.append(c);
+                }
+            }
+            return changed ? sb.toString() : s;
+        }
+
         private String determineTestSuiteName() {
             // this is really a hack to try and match the expectations of the XML report in JUnit4.x
             // world. In JUnit5, the TestPlan doesn't have a name and a TestPlan (for which this is a

src/main/org/apache/tools/ant/util/DOMElementWriter.java

@@ -588,6 +588,20 @@ public class DOMElementWriter {
      * @since 1.10, Ant 1.5
      */
     public boolean isLegalCharacter(final char c) {
+        return isLegalXmlCharacter(c);
+    }
+
+    /**
+     * Is the given character allowed inside an XML document?
+     *
+     * <p>See XML 1.0 2.2 <a
+     * href="https://www.w3.org/TR/1998/REC-xml-19980210#charsets">
+     * https://www.w3.org/TR/1998/REC-xml-19980210#charsets</a>.</p>
+     * @param c the character to test.
+     * @return true if the character is allowed.
+     * @since 1.10.10
+     */
+    public static boolean isLegalXmlCharacter(final char c) {
         // CheckStyle:MagicNumber OFF
         if (c == 0x9 || c == 0xA || c == 0xD) {
             return true;

src/tests/junit/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatterTest.java

@@ -0,0 +1,86 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.junit.Test;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.Optional;
+import java.util.Properties;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.Assert.assertThat;
+
+public class LegacyXmlResultFormatterTest {
+
+    private static final String KEY = "key";
+    private static final String ORIG = "<\u0000&>foo";
+    private static final String ENCODED = "&lt;&amp;#0;&amp;&gt;foo";
+
+    private final LegacyXmlResultFormatter f = new LegacyXmlResultFormatter();
+
+    @Test
+    public void encodesAttributesProperly() throws Exception {
+        final TestPlan plan = startTest(true);
+        final String result = finishTest(plan);
+        assertThat(result, containsString("=\"" + ENCODED + "\""));
+    }
+
+    @Test
+    public void encodesSysOutProperly() throws Exception {
+        final TestPlan plan = startTest(false);
+        f.sysOutAvailable(ORIG.getBytes(StandardCharsets.UTF_8));
+        final String result = finishTest(plan);
+        assertThat(result, containsString(ENCODED));
+    }
+
+    private TestPlan startTest(final boolean withProperties) {
+        f.setContext(new TestExecutionContext() {
+            @Override
+            public Properties getProperties() {
+                final Properties p = new Properties();
+                if (withProperties) {
+                    p.setProperty(KEY, ORIG);
+                }
+                return p;
+            }
+
+            @Override
+            public Optional<Project> getProject() {
+                return Optional.empty();
+            }
+        });
+        final TestPlan testPlan = TestPlan.from(Collections.emptySet());
+        f.testPlanExecutionStarted(testPlan);
+        return testPlan;
+    }
+
+    private String finishTest(final TestPlan testPlan) throws IOException {
+        try (final ByteArrayOutputStream bos = new ByteArrayOutputStream()) {
+            f.setDestination(bos);
+            f.testPlanExecutionFinished(testPlan);
+            return new String(bos.toByteArray(), StandardCharsets.UTF_8);
+        }
+    }
+}