Merge pull request #320 from DotJoshJohnson/bug/293-cdata-spaces

Prevent CDATA Formatting
DotJoshJohnson · Jul 7, 2020 · fafa053 · fafa053
2 parents 6689fd3 + 10619ee
commit fafa053
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 17 deletions.
diff --git a/src/formatting/formatters/v2-xml-formatter.ts b/src/formatting/formatters/v2-xml-formatter.ts
@@ -7,9 +7,9 @@ const MagicalStringOfWonders = "~::~MAAAGIC~::~";
 /* tslint:disable no-use-before-declare */
 export class V2XmlFormatter implements XmlFormatter {
     formatXml(xml: string, options: XmlFormattingOptions): string {
-        // this replaces all "<" brackets inside of comments to a magical string
-        // so the following minification steps don't mess with comment formatting
-        xml = this._sanitizeComments(xml);
+        // this replaces all "<" brackets inside of comments and CDATA to a magical string
+        // so the following minification steps don't mess with comment and CDATA formatting
+        xml = this._sanitizeCommentsAndCDATA(xml);
 
         // remove whitespace from between tags, except for line breaks
         xml = xml.replace(/>\s{0,}</g, (match: string) => {
@@ -25,7 +25,7 @@ export class V2XmlFormatter implements XmlFormatter {
         });
 
         // the coast is clear - we can drop those "<" brackets back in
-        xml = this._unsanitizeComments(xml);
+        xml = this._unsanitizeCommentsAndCDATA(xml);
 
         let output = "";
 
@@ -298,30 +298,30 @@ export class V2XmlFormatter implements XmlFormatter {
         return text.replace(/[^\r\n\S]+$/, "");
     }
 
-    private _sanitizeComments(xml: string): string {
+    private _sanitizeCommentsAndCDATA(xml: string): string {
         let output = "";
-        let inComment = false;
+        let inCommentOrCDATA = false;
 
         for (let i = 0; i < xml.length; i++) {
             const cc = xml[i];
             const nc = xml.charAt(i + 1);
             const nnc = xml.charAt(i + 2);
             const pc = xml.charAt(i - 1);
 
-            if (!inComment && cc === "<" && nc === "!" && nnc === "-") {
-                inComment = true;
-                output += "<!--";
+            if (!inCommentOrCDATA && cc === "<" && nc === "!" && (nnc === "-" || nnc === "[")) {
+                inCommentOrCDATA = true;
+                output += (nnc === "-") ? "<!--" : "<![CDATA[";
 
-                i += 3;
+                i += (nnc === "-") ? 3 : 8;
             }
 
-            else if (inComment && cc === "<") {
+            else if (inCommentOrCDATA && cc === "<") {
                 output += MagicalStringOfWonders;
             }
 
-            else if (inComment && cc === "-" && nc === "-" && nnc === ">") {
-                inComment = false;
-                output += "-->";
+            else if (inCommentOrCDATA && (cc === "-" && nc === "-" && nnc === ">") || (cc === "]" && nc === "]" && nnc === ">")) {
+                inCommentOrCDATA = false;
+                output += (cc === "-") ? "-->" : "]]>";
 
                 i += 2;
             }
@@ -334,7 +334,7 @@ export class V2XmlFormatter implements XmlFormatter {
         return output;
     }
 
-    private _unsanitizeComments(xml: string): string {
+    private _unsanitizeCommentsAndCDATA(xml: string): string {
         return xml.replace(new RegExp(MagicalStringOfWonders, "g"), "<");
     }
 }

diff --git a/src/test/extension.test.ts b/src/test/extension.test.ts
@@ -99,6 +99,10 @@ describe("V2XmlFormatter", () => {
         it("should handle mixed content as a child of another element", () => {
             testFormatter(xmlFormatter, options, "issue-257");
         });
+
+        it("should not touch CDATA content", () => {
+            testFormatter(xmlFormatter, options, "issue-293");
+        });
     });
 
     describe("#minifyXml(xml, options)", () => {
@@ -130,7 +134,7 @@ function testFormatter(xmlFormatter: XmlFormatter, options: XmlFormattingOptions
     const actualFormattedXml = xmlFormatter.formatXml(unformattedXml, options).replace(/\r/g, "");
 
     // tslint:disable-next-line
-    assert.ok((actualFormattedXml === expectedFormattedXml), `Actual formatted XML does not match expected formatted XML.\n\nACTUAL\n${actualFormattedXml.replace(/\s/, "~ws~")}\n\nEXPECTED\n${expectedFormattedXml.replace(/\s/, "~ws~")}`);
+    assert.ok((actualFormattedXml === expectedFormattedXml), `Actual formatted XML does not match expected formatted XML.\n\nACTUAL\n${actualFormattedXml.replace(/\s/g, "~ws~")}\n\nEXPECTED\n${expectedFormattedXml.replace(/\s/g, "~ws~")}`);
 }
 
 function testMinifier(xmlFormatter: XmlFormatter, options: XmlFormattingOptions, fileLabel: string): void {
@@ -140,5 +144,5 @@ function testMinifier(xmlFormatter: XmlFormatter, options: XmlFormattingOptions,
     const actualMinifiedXml = xmlFormatter.minifyXml(unminifiedXml, options).replace(/\r/g, "");
 
     // tslint:disable-next-line
-    assert.ok((actualMinifiedXml === expectedMinifiedXml), `Actual minified XML does not match expected minified XML.\n\nACTUAL\n${actualMinifiedXml.replace(/\s/, "~ws~")}\n\nEXPECTED\n${expectedMinifiedXml.replace(/\s/, "~ws~")}`);
+    assert.ok((actualMinifiedXml === expectedMinifiedXml), `Actual minified XML does not match expected minified XML.\n\nACTUAL\n${actualMinifiedXml.replace(/\s/g, "~ws~")}\n\nEXPECTED\n${expectedMinifiedXml.replace(/\s/g, "~ws~")}`);
 }
diff --git a/src/test/test-data/issue-293.formatted.xml b/src/test/test-data/issue-293.formatted.xml
@@ -0,0 +1,6 @@
+<xml>
+    <element><![CDATA[asdf]]></element>
+    <element><![CDATA[<secondXml>
+    <formattedNode>val</formattedNode>
+</secondXml>]]></element>
+</xml>
diff --git a/src/test/test-data/issue-293.unformatted.xml b/src/test/test-data/issue-293.unformatted.xml
@@ -0,0 +1,6 @@
+<xml>
+    <element><![CDATA[asdf]]></element>
+    <element><![CDATA[<secondXml>
+    <formattedNode>val</formattedNode>
+</secondXml>]]></element>
+</xml>