From 2e8752183f81a74570784451fa336cede7c1be41 Mon Sep 17 00:00:00 2001
From: Erik Marks <25517051+rekmarks@users.noreply.github.com>
Date: Fri, 29 Oct 2021 16:45:27 -0700
Subject: [PATCH] Fix off-by-one error in code fence transform (#12540)

The code fence transform was including contents after the final END directive twice. That was not covered by the tests, because none of the examples contained any content after the final END directive, and concatenating the empty string twice does not produce an observable difference in the test results.

This bug was due to an off-by-one error in the loop of the multiSplice function. The error has been fixed, and more test cases have been added.
---
 .../build/transforms/remove-fenced-code.js    |   4 +-
 .../transforms/remove-fenced-code.test.js     | 115 ++++++++++++++++++
 2 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/development/build/transforms/remove-fenced-code.js b/development/build/transforms/remove-fenced-code.js
index 7f26ad6cf..200233fe8 100644
--- a/development/build/transforms/remove-fenced-code.js
+++ b/development/build/transforms/remove-fenced-code.js
@@ -408,7 +408,9 @@ function multiSplice(toSplice, splicingIndices) {
   // pushes the substring between each "end" index and the next "begin" index
   // to the array of retained substrings.
   if (splicingIndices.length > 2) {
-    for (let i = 1; i < splicingIndices.length; i += 2) {
+    // Note the boundary index of "splicingIndices.length - 1". This loop must
+    // not iterate over the last element of the array.
+    for (let i = 1; i < splicingIndices.length - 1; i += 2) {
       retainedSubstrings.push(
         toSplice.substring(splicingIndices[i], splicingIndices[i + 1]),
       );
diff --git a/development/build/transforms/remove-fenced-code.test.js b/development/build/transforms/remove-fenced-code.test.js
index 33461b89d..5eebc1135 100644
--- a/development/build/transforms/remove-fenced-code.test.js
+++ b/development/build/transforms/remove-fenced-code.test.js
@@ -201,6 +201,14 @@ describe('build/transforms/remove-fenced-code', () => {
           ),
         ).toStrictEqual(testData.validOutputs[buildType]);
 
+        expect(
+          removeFencedCode(
+            mockFileName,
+            buildType,
+            testData.validInputs.extraContentWithFences,
+          ),
+        ).toStrictEqual(testData.validOutputsWithExtraContent[buildType]);
+
         // Ensure that the minimal input template is in fact valid
         const minimalInput = getMinimalFencedCode(buildType);
         expect(
@@ -216,6 +224,17 @@ describe('build/transforms/remove-fenced-code', () => {
             testData.validInputs.withoutFences,
           ),
         ).toStrictEqual([testData.validInputs.withoutFences, false]);
+
+        expect(
+          removeFencedCode(
+            mockFileName,
+            buildType,
+            testData.validInputs.extraContentWithoutFences,
+          ),
+        ).toStrictEqual([
+          testData.validInputs.extraContentWithoutFences,
+          false,
+        ]);
       });
     });
 
@@ -611,6 +630,43 @@ Conditionally_Included
        ///: END:ONLY_INCLUDE_IN
 `,
 
+      extraContentWithFences: `
+///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
+Conditionally_Included
+///: END:ONLY_INCLUDE_IN
+  Always_Included
+Always_Included
+   Always_Included
+Always_Included
+  ///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
+  Conditionally_Included
+
+  Conditionally_Included
+  Conditionally_Included
+  ///: END:ONLY_INCLUDE_IN
+Always_Included
+
+Always_Included
+   Always_Included
+          ///: BEGIN:ONLY_INCLUDE_IN(flask)
+
+  Conditionally_Included
+    Conditionally_Included
+       ///: END:ONLY_INCLUDE_IN
+Always_Included
+   Always_Included
+Always_Included
+
+///: BEGIN:ONLY_INCLUDE_IN(flask)
+  Conditionally_Included
+Conditionally_Included
+
+       ///: END:ONLY_INCLUDE_IN
+    Always_Included
+      Always_Included
+Always_Included
+`,
+
       withoutFences: `
   Always_Included
 Always_Included
@@ -624,6 +680,24 @@ Always_Included
    Always_Included
 Always_Included
 
+`,
+
+      extraContentWithoutFences: `
+  Always_Included
+Always_Included
+   Always_Included
+Always_Included
+Always_Included
+
+Always_Included
+   Always_Included
+Always_Included
+   Always_Included
+Always_Included
+
+    Always_Included
+      Always_Included
+Always_Included
 `,
     },
 
@@ -651,6 +725,38 @@ Always_Included
    Always_Included
 Always_Included
 
+`,
+        true,
+      ],
+    },
+
+    validOutputsWithExtraContent: {
+      beta: [
+        `
+///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
+Conditionally_Included
+///: END:ONLY_INCLUDE_IN
+  Always_Included
+Always_Included
+   Always_Included
+Always_Included
+  ///: BEGIN:ONLY_INCLUDE_IN(flask,beta)
+  Conditionally_Included
+
+  Conditionally_Included
+  Conditionally_Included
+  ///: END:ONLY_INCLUDE_IN
+Always_Included
+
+Always_Included
+   Always_Included
+Always_Included
+   Always_Included
+Always_Included
+
+    Always_Included
+      Always_Included
+Always_Included
 `,
         true,
       ],
@@ -659,5 +765,14 @@ Always_Included
 
   data.validOutputs.flask = [data.validInputs.withFences, false];
   data.validOutputs.main = [data.validInputs.withoutFences, true];
+
+  data.validOutputsWithExtraContent.flask = [
+    data.validInputs.extraContentWithFences,
+    false,
+  ];
+  data.validOutputsWithExtraContent.main = [
+    data.validInputs.extraContentWithoutFences,
+    true,
+  ];
   return deepFreeze(data);
 }