fix: junit testresult file generation (#285)

Resolved an issue where the junit file generation created invalid XML files, because the failure message contains characters that should be XML-escaped. These are: <, >, &, " and '. Made sure to xml-encode these in the message, updated a testcase to make sure that this is now covered in a test as well. Additional commit to also escape the method name, it can be '<compile>' when testcases fail because of compilation errors. The '<' and '>' characters also need to be escaped in that case. Co-authored-by: Johannes Verelst <[email protected]>
forcedotcom · Oct 6, 2022 · ddb3480 · ddb3480
1 parent 3f9d8cb
commit ddb3480
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 8 deletions.
diff --git a/packages/apex-node/src/reporters/junitReporter.ts b/packages/apex-node/src/reporters/junitReporter.ts
@@ -72,17 +72,18 @@ export class JUnitReporter {
     let junitTests = '';
 
     for (const testCase of tests) {
-      junitTests += `${tab}${tab}<testcase name="${
-        testCase.methodName
-      }" classname="${testCase.apexClass.fullName}" time="${msToSecond(
-        testCase.runTime
-      )}">\n`;
+      const methodName = this.xmlEscape(testCase.methodName);
+      junitTests += `${tab}${tab}<testcase name="${methodName}" classname="${
+        testCase.apexClass.fullName
+      }" time="${msToSecond(testCase.runTime)}">\n`;
 
       if (
         testCase.outcome === ApexTestResultOutcome.Fail ||
         testCase.outcome === ApexTestResultOutcome.CompileFail
       ) {
-        junitTests += `${tab}${tab}${tab}<failure message="${testCase.message}">`;
+        let message = this.isEmpty(testCase.message) ? '' : testCase.message;
+        message = this.xmlEscape(message);
+        junitTests += `${tab}${tab}${tab}<failure message="${message}">`;
         if (testCase.stackTrace) {
           junitTests += `<![CDATA[${testCase.stackTrace}]]>`;
         }
@@ -94,6 +95,15 @@ export class JUnitReporter {
     return junitTests;
   }
 
+  private xmlEscape(value: string): string {
+    return value
+      .replace(/&/g, '&amp;')
+      .replace(/</g, '&lt;')
+      .replace(/>/g, '&gt;')
+      .replace(/"/g, '&quot;')
+      .replace(/'/g, '&apos;');
+  }
+
   private isEmpty(value: string | number): boolean {
     if (
       value === null ||

diff --git a/packages/apex-node/test/reporters/tapReporter.test.ts b/packages/apex-node/test/reporters/tapReporter.test.ts
@@ -70,6 +70,7 @@ describe('TAP Reporter Tests', () => {
       'Class.AccountProcessorTest.testCountContacts: line 47, column 1'
     ]);
     expect(result[12].diagnostics).to.eql([
+      'Weird characters <>&"\'',
       'Surrounded by newlines.',
       'and whitespace.'
     ]);

diff --git a/packages/apex-node/test/reporters/testResults.ts b/packages/apex-node/test/reporters/testResults.ts
@@ -445,7 +445,8 @@ export const testResults: TestResult = {
       id: '07M3t000003bQwREAU',
       queueItemId: '7093t000000c0eiAAA',
       stackTrace: null,
-      message: '\r\n\r\nSurrounded by newlines.\r\n  and whitespace.\r\n\r\n',
+      message:
+        'Weird characters <>&"\'\r\n\r\nSurrounded by newlines.\r\n  and whitespace.\r\n\r\n',
       asyncApexJobId: '7073t000061uwZIAAY',
       methodName: 'testGetCallout',
       outcome: ApexTestResultOutcome.Fail,
@@ -568,7 +569,7 @@ export const junitResult = `<?xml version="1.0" encoding="UTF-8"?>
         </testcase>
         <testcase name="testCallout" classname="ParkLocatorTest" time="0.01">
         </testcase>
-        <testcase name="testGetCallout" classname="AnimalsCalloutsTest" time="0.03">\n            <failure message="\r\n\r\nSurrounded by newlines.\r\n  and whitespace.\r\n\r
+        <testcase name="testGetCallout" classname="AnimalsCalloutsTest" time="0.03">\n            <failure message="Weird characters &lt;&gt;&amp;&quot;&apos;\r\n\r\nSurrounded by newlines.\r\n  and whitespace.\r\n\r
 "></failure>
         </testcase>
         <testcase name="testPostCallout" classname="AnimalsCalloutsTest" time="0.01">