Repository: stoplightio/spectral (ts)

Issue:
JUnit formatter incorrectly escapes failure details
**Describe the bug**

The JUnit formatter runs failure details through `xmlEscape` although these are contained within a `CDATA` section:

https://github.com/stoplightio/spectral/blob/develop/packages/cli/src/formatters/junit.ts#L51-L55

This [is unnecessary](https://stackoverflow.com/a/223782) and can cause visualization problems when rendering the results in a post-processing tool.

**To Reproduce**

Run spectral with JUnit formatter against a spec with errors:

```xml
<testcase time="0" name="org.spectral.json:api-ContentType-Error" classname="path/to/spec">
    <failure message="SHOULD have a content type 'application/vnd.api+json'.">
        <![CDATA[
line 126, col 19, SHOULD have a content type &apos;application/vnd.api+json&apos;. (json:api-ContentType-Error) at path             #/paths/~1info/get/responses/200/content
        ]]>
    </failure>
</testcase>
```

**Expected behavior**

```xml
<testcase time="0" name="org.spectral.json:api-ContentType-Error" classname="path/to/spec">
    <failure message="SHOULD have a content type 'application/vnd.api+json'.">
        <![CDATA[
line 126, col 19, SHOULD have a content type 'application/vnd.api+json'. (json:api-ContentType-Error) at path             #/paths/~1info/get/responses/200/content
        ]]>
    </failure>
</testcase>
```

**Prososal**

Do what's suggested in the [SO thread](https://stackoverflow.com/a/36331725) and drop `xmlEscape` in favor of something like `replace("]]>", "]]]]><![CDATA[>")`.



Pull request:
fix(cli): correctly handle special characters in JUnit failure details
Those details are contained in XML CDATA section and should therefore not be XML-escaped.

Fixes #2027

**Checklist**

- [x] Tests added / updated
- [ ] Docs added / updated

**Does this PR introduce a breaking change?**

- [ ] Yes
- [x] No

🛠 with ❤ at [Siemens](https://opensource.siemens.io)

Patch:
diff --git a/packages/cli/src/formatters/junit.ts b/packages/cli/src/formatters/junit.ts
index 7af7f84a..646341af 100644
--- a/packages/cli/src/formatters/junit.ts
+++ b/packages/cli/src/formatters/junit.ts
@@ -29,6 +29,13 @@ import { printPath, PrintStyle } from '@stoplight/spectral-runtime';
 import { Formatter } from './types';
 import { groupBySource, xmlEscape } from './utils';
 
+/**
+ * Prepares the given text for inclusion in an XML CDATA section.
+ */
+function prepareForCdata(text: string): string {
+  return text.replace(']]>', ']]]]><![CDATA[>');
+}
+
 export const junit: Formatter = (results, { failSeverity }) => {
   let output = '';
 
@@ -53,8 +60,8 @@ export const junit: Formatter = (results, { failSeverity }) => {
         output += `<failure message="${xmlEscape(result.message)}">`;
         output += '<![CDATA[';
         output += `line ${result.range.start.line + 1}, col ${result.range.start.character + 1}, `;
-        output += `${xmlEscape(result.message)} (${result.code}) `;
-        output += `at path ${xmlEscape(path)}`;
+        output += `${prepareForCdata(result.message)} (${result.code}) `;
+        output += `at path ${prepareForCdata(path)}`;
         output += ']]>';
         output += `</failure>`;
         output += '</testcase>\n';


Test patch:
diff --git a/packages/cli/src/formatters/__tests__/__fixtures__/errors-with-special-xml-strings.json b/packages/cli/src/formatters/__tests__/__fixtures__/errors-with-special-xml-strings.json
new file mode 100644
index 00000000..93aeda09
--- /dev/null
+++ b/packages/cli/src/formatters/__tests__/__fixtures__/errors-with-special-xml-strings.json
@@ -0,0 +1,46 @@
+[
+  {
+    "code": "special-xml-strings",
+    "message": "start ' \" < > end",
+    "path": [
+      "root",
+      "'",
+      "\"",
+      "leaf"
+    ],
+    "severity": 0,
+    "source": "",
+    "range": {
+      "start": {
+        "line": 0,
+        "character": 0
+      },
+      "end": {
+        "line": 0,
+        "character": 0
+      }
+    }
+  },
+  {
+    "code": "special-cdata-strings",
+    "message": "start <![CDATA[ ]]> <![CDATA[ end",
+    "path": [
+      "root",
+      "]]>",
+      "<![CDATA[",
+      "leaf"
+    ],
+    "severity": 0,
+    "source": "",
+    "range": {
+      "start": {
+        "line": 0,
+        "character": 0
+      },
+      "end": {
+        "line": 0,
+        "character": 0
+      }
+    }
+  }
+]
\ No newline at end of file
diff --git a/packages/cli/src/formatters/__tests__/junit.test.ts b/packages/cli/src/formatters/__tests__/junit.test.ts
index b6527a48..b38b1a42 100644
--- a/packages/cli/src/formatters/__tests__/junit.test.ts
+++ b/packages/cli/src/formatters/__tests__/junit.test.ts
@@ -4,6 +4,7 @@ import { DiagnosticSeverity } from '@stoplight/types';
 
 const oas3SchemaErrors = require('./__fixtures__/oas3-schema-errors.json');
 const mixedErrors = require('./__fixtures__/mixed-errors.json');
+const specialXmlStrings = require('./__fixtures__/errors-with-special-xml-strings.json');
 
 describe('JUnit formatter', () => {
   let parse: Parser['parseStringPromise'];
@@ -69,7 +70,7 @@ describe('JUnit formatter', () => {
                     $: {
                       message: "should have required property '$ref'",
                     },
-                    _: 'line 36, col 22, should have required property &apos;$ref&apos; (oas3-schema) at path #/paths/~1pets/get/responses/200/headers/header-1',
+                    _: "line 36, col 22, should have required property '$ref' (oas3-schema) at path #/paths/~1pets/get/responses/200/headers/header-1",
                   },
                 ],
               },
@@ -168,4 +169,56 @@ describe('JUnit formatter', () => {
       },
     });
   });
+
+  test('handles special XML strings properly', async () => {
+    const result = await parse(junit(specialXmlStrings, { failSeverity: DiagnosticSeverity.Error }));
+    expect(result).toEqual({
+      testsuites: {
+        testsuite: [
+          {
+            $: {
+              errors: '0',
+              failures: '2',
+              name: '',
+              package: 'org.spectral',
+              tests: '2',
+              time: '0',
+            },
+            testcase: [
+              {
+                $: {
+                  classname: '',
+                  name: 'org.spectral.special-xml-strings(#/root/\'/"/leaf)',
+                  time: '0',
+                },
+                failure: [
+                  {
+                    $: {
+                      message: 'start \' " < > end',
+                    },
+                    _: 'line 1, col 1, start \' " < > end (special-xml-strings) at path #/root/\'/"/leaf',
+                  },
+                ],
+              },
+              {
+                $: {
+                  classname: '',
+                  name: 'org.spectral.special-cdata-strings(#/root/]]>/<![CDATA[/leaf)',
+                  time: '0',
+                },
+                failure: [
+                  {
+                    $: {
+                      message: 'start <![CDATA[ ]]> <![CDATA[ end',
+                    },
+                    _: 'line 1, col 1, start <![CDATA[ ]]> <![CDATA[ end (special-cdata-strings) at path #/root/]]>/<![CDATA[/leaf',
+                  },
+                ],
+              },
+            ],
+          },
+        ],
+      },
+    });
+  });
 });
diff --git a/test-harness/scenarios/formats/results-format-junit.scenario b/test-harness/scenarios/formats/results-format-junit.scenario
index 9d218b7d..4f34b4c2 100644
--- a/test-harness/scenarios/formats/results-format-junit.scenario
+++ b/test-harness/scenarios/formats/results-format-junit.scenario
@@ -56,6 +56,6 @@ info:
 <?xml version="1.0" encoding="utf-8"?>
 <testsuites>
 <testsuite package="org.spectral" time="0" tests="1" errors="0" failures="1" name="{document}">
-<testcase time="0" name="org.spectral.api-servers(#)" classname="{document}"><failure message="&quot;servers&quot; must be present and non-empty array."><![CDATA[line 1, col 1, &quot;servers&quot; must be present and non-empty array. (api-servers) at path #]]></failure></testcase>
+<testcase time="0" name="org.spectral.api-servers(#)" classname="{document}"><failure message="&quot;servers&quot; must be present and non-empty array."><![CDATA[line 1, col 1, "servers" must be present and non-empty array. (api-servers) at path #]]></failure></testcase>
 </testsuite>
 </testsuites>