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

Merge generates invalid xml when failure message contains "]]>" text #59

Closed
sarod opened this issue Dec 11, 2020 · 3 comments
Closed
Assignees

Comments

@sarod
Copy link

sarod commented Dec 11, 2020

Given a test file TEST-repo.MyTest.xml like this

<?xml version="1.0" encoding="UTF-8"?>
<testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd" 
name="repro.MyTest" time="2.6" tests="1" errors="0" skipped="0" failures="1">
  <properties/>
  <testcase name="myMethod" classname="repro.MyTest" time="1.573">
    <failure message="failure message with ]]&gt;" type="java.lang.AssertionError">
      java.lang.AssertionError: failure message with ]]&gt;
    </failure>
  </testcase>
</testsuite>
```

junit-report-merger ./combined.xml "TEST-repro.MyTest.xml"
Generates an invalid xml file:
```
<?xml version="1.0"?>
<testsuites failures="1" errors="0" tests="1"><testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd" name="repro.MyTest" time="2.6" tests="1" errors="0" skipped="0" failures="1">
  <properties/>
  <testcase name="myMethod" classname="repro.MyTest" time="1.573">
    <failure message="failure message with ]]>" type="java.lang.AssertionError">
      java.lang.AssertionError: failure message with ]]>
    </failure>
  </testcase>
</testsuite></testsuites>
```

This file is invalid because "]]>" should not appear in xml file outside of xml CDATA. To solve this the > should be escaped as &gt; similarly to what is done in original file
@bhovhannes
Copy link
Owner

Thanks for logging this!
Working on that. I guess this was introduced with the recent update of xmldom dependency.

@bhovhannes
Copy link
Owner

This seems similar: xmldom/xmldom#58
Perhaps I'll stop using xmldom here and replace it with some sax parser. That will increase performance.

@bhovhannes bhovhannes self-assigned this Dec 11, 2020
bhovhannes added a commit that referenced this issue Dec 11, 2020
BREAKING CHANGE: xmlbuilder2 does not support xml version 1.1, so junit-report-merger won't be able to process xml files with version 1.1 anymore. To my knowledge no tools use xml version 1.1 in their test reports, so this should not affect people. However, technically, this is a breaking change.

xmlbuilder2 is faster than xmldom, is maintained better and is successor of extremely popular xmlbuilder package. Also, it is more standard compliant.
@bhovhannes
Copy link
Owner

Released fix version 2.0.0.
Release notes: https://github.com/bhovhannes/junit-report-merger/releases/tag/v2.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants