Merge pull request #273 from cdloh/fix/esi-args-parse-empty-string

fix: handle empty strings correctly (dont force them to be ints)
cdloh · Jun 18, 2024 · 0491048 · 0491048
2 parents d946414 + eb61116
commit 0491048
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/src/processConditionals.ts b/src/processConditionals.ts
@@ -50,7 +50,7 @@ export async function process(
             // eslint-disable-next-line no-inner-declarations, jsdoc/require-jsdoc
             async function processWhen(match: RegExpMatchArray) {
               const condition = match[1];
-              const conditionValidated = await _esi_evaluate_condition(
+              const conditionValidated = _esi_evaluate_condition(
                 esiData,
                 condition,
               );
@@ -127,7 +127,7 @@ function esi_eval_var_in_when_tag(
   const varInTag = esi_eval_var(eventData, match);
   // we have to check varInTag is *actually* a number and doesn't just have leading numbers in it
   if (strIsNumber(varInTag)) {
-    return parseInt(varInTag, 10).toString();
+    return varInTag;
   } else {
     // Change to replaceAll once upgraded node
     return "'" + varInTag.replace(/'/g, "\\'") + "'";
@@ -141,7 +141,14 @@ function esi_eval_var_in_when_tag(
  * @returns {boolean} check result
  */
 function strIsNumber(str: string): boolean {
-  return !isNaN(Number(str));
+  const int = parseInt(str, 10);
+  // definitely not an int
+  if (!int) {
+    return false;
+  }
+
+  // double check we have correctly converted it to an int and not been tricked by parseInt
+  return int.toString() === str;
 }
 
 /**

diff --git a/test/esi.spec.ts b/test/esi.spec.ts
@@ -1919,6 +1919,7 @@ describe("TEST 51: ESI Args that lead with ints shouldn't convert to ints", () =
     { arg: "1719,1918", result: "third-lineage" },
     { arg: "1719,1922", result: "forth-lineage" },
     { arg: "1719,1926", result: "fith-lineage" },
+    { arg: "1719,2000", result: "sixth-lineage" },
   ];
 
   const url = `/esi/test-51`;
@@ -1932,6 +1933,7 @@ describe("TEST 51: ESI Args that lead with ints shouldn't convert to ints", () =
     <esi:when test="$(ESI_ARGS{lineage}) =~ '/(?:^|,)(1918)(?:,|$)/'">third-lineage</esi:when>
     <esi:when test="$(ESI_ARGS{lineage}) =~ '/(?:^|,)(1922)(?:,|$)/'">forth-lineage</esi:when>
     <esi:when test="$(ESI_ARGS{lineage}) =~ '/(?:^|,)(1926)(?:,|$)/'">fith-lineage</esi:when>
+    <esi:when test="$(ESI_ARGS{lineage}) == '1719,2000'">sixth-lineage</esi:when>
 </esi:choose>`);
       });
       const res = await makeRequest(`${url}?esi_lineage=${check.arg}`);
@@ -1941,3 +1943,23 @@ describe("TEST 51: ESI Args that lead with ints shouldn't convert to ints", () =
     });
   });
 });
+
+test("TEST 52: Empty variables can be compared", async () => {
+  const url = `/esi/test-52?x=`;
+  routeHandler.add(url, function (req, res) {
+    res.writeHead(200, esiHead);
+    res.end(`<esi:choose>
+    <esi:when test="$(QUERY_STRING{x}) == ''">x empty;</esi:when>
+    <esi:when test="$(QUERY_STRING{x}) != ''">x not empty;</esi:when>
+    <esi:otherwise>x neither empty nor not empty;</esi:otherwise>
+</esi:choose><esi:choose>
+    <esi:when test="$(QUERY_STRING{y}) == ''">y empty;</esi:when>
+    <esi:when test="$(QUERY_STRING{y}) != ''">y not empty;</esi:when>
+    <esi:otherwise>y neither empty nor not empty;</esi:otherwise>
+</esi:choose>`);
+  });
+  const res = await makeRequest(url);
+  expect(res.ok).toBeTruthy();
+  expect(checkSurrogate(res)).toBeTruthy();
+  expect(await res.text()).toEqual(`x empty;y empty;`);
+});