fix: infinite loop in flatten

juanjoDiaz · Nov 11, 2024 · e81cd60 · e81cd60
1 parent 8c7895c
commit e81cd60
Show file tree

Hide file tree

Showing 14 changed files with 297 additions and 46 deletions.
diff --git a/packages/cli/test/CLI.ts b/packages/cli/test/CLI.ts
@@ -833,6 +833,20 @@ export default function (
     },
   );
 
+  testRunner.add(
+    'should support custom flatten separator using the flatten transform',
+    async (t) => {
+      const opts =
+        '--delimiter , --flatten-objects --flatten-arrays --flatten-separator .';
+
+      const { stdout: csv } = await execAsync(
+        `${cli} -i "${getFixturePath('/json/objectWithEmptyFields.json')}" ${opts}`,
+      );
+
+      t.equal(csv, csvFixtures.objectWithEmptyFieldsStream);
+    },
+  );
+
   testRunner.add(
     'should support multiple transforms and honor the order in which they are declared',
     async (t) => {

diff --git a/packages/node/test/AsyncParser.ts b/packages/node/test/AsyncParser.ts
@@ -914,6 +914,24 @@ export default function (
     },
   );
 
+  testRunner.add(
+    'should support custom flatten separator using the flatten transform',
+    async (t) => {
+      const opts: ParserOptions = {
+        delimiter: ';',
+        transforms: [flatten({ separator: '.', arrays: true, objects: true })],
+      };
+
+      const parser = new Parser(opts);
+      const csv = await parseInput(
+        parser,
+        jsonFixtures.objectWithEmptyFields(),
+      );
+
+      t.equal(csv, csvFixtures.objectWithEmptyFieldsStream);
+    },
+  );
+
   testRunner.add(
     'should support multiple transforms and honor the order in which they are declared',
     async (t) => {

diff --git a/packages/node/test/AsyncParserInMemory.ts b/packages/node/test/AsyncParserInMemory.ts
@@ -914,6 +914,24 @@ export default function (
     },
   );
 
+  testRunner.add(
+    'should support custom flatten separator using the flatten transform',
+    async (t) => {
+      const opts: ParserOptions = {
+        delimiter: ';',
+        transforms: [flatten({ separator: '.', arrays: true, objects: true })],
+      };
+
+      const parser = new Parser(opts);
+      const csv = await parseInput(
+        parser,
+        jsonFixtures.objectWithEmptyFields(),
+      );
+
+      t.equal(csv, csvFixtures.objectWithEmptyFieldsStream);
+    },
+  );
+
   testRunner.add(
     'should support multiple transforms and honor the order in which they are declared',
     async (t) => {

diff --git a/packages/node/test/Transform.ts b/packages/node/test/Transform.ts
@@ -900,6 +900,24 @@ export default function (
     },
   );
 
+  testRunner.add(
+    'should support custom flatten separator using the flatten transform',
+    async (t) => {
+      const opts: ParserOptions = {
+        delimiter: ';',
+        transforms: [flatten({ separator: '.', arrays: true, objects: true })],
+      };
+
+      const parser = new Parser(opts);
+      const csv = await parseInput(
+        parser,
+        jsonFixtures.objectWithEmptyFields(),
+      );
+
+      t.equal(csv, csvFixtures.objectWithEmptyFieldsStream);
+    },
+  );
+
   testRunner.add(
     'should support multiple transforms and honor the order in which they are declared',
     async (t) => {

diff --git a/packages/plainjs/src/utils.ts b/packages/plainjs/src/utils.ts
@@ -41,23 +41,60 @@ type GetFieldType<T, P> = P extends `${infer Left}.${infer Right}`
 
 type PropertyName = string | number | symbol;
 
-const rePropName = RegExp(
-  // Match anything that isn't a dot or bracket.
-  '[^.[\\]]+' +
-    '|' +
-    // Or match property names within brackets.
-    '\\[(?:' +
-    // Match a non-string expression.
-    '([^"\'][^[]*)' +
-    '|' +
-    // Or match strings (supports escaping characters).
-    '(["\'])((?:(?!\\2)[^\\\\]|\\\\.)*?)\\2' +
-    ')\\]' +
-    '|' +
-    // Or match "" as the space between consecutive dots or empty brackets.
-    '(?=(?:\\.|\\[\\])(?:\\.|\\[\\]|$))',
-  'g',
-);
+const reIsDeepProp = /\.|\[(?:[^[\]]*|(["'])(?:(?!\1)[^\\]|\\.)*?\1)\]/;
+const reIsPlainProp = /^\w*$/;
+const rePropName =
+  /[^.[\]]+|\[(?:(-?\d+(?:\.\d+)?)|(["'])((?:(?!\2)[^\\]|\\.)*?)\2)\]|(?=(?:\.|\[\])(?:\.|\[\]|$))/g;
+const reEscapeChar = /\\(\\)?/g;
+
+/**
+ * Checks if `value` is a property name and not a property path.
+ *
+ * @private
+ * @param {*} value The value to check.
+ * @param {Object} [object] The object to query keys on.
+ * @returns {boolean} Returns `true` if `value` is a property name, else `false`.
+ */
+function isKey<TObject extends object>(value: any, object: TObject): boolean {
+  if (Array.isArray(value)) {
+    return false;
+  }
+  const type = typeof value;
+  if (
+    type == 'number' ||
+    type == 'symbol' ||
+    type == 'boolean' ||
+    value == null
+  ) {
+    return true;
+  }
+  return (
+    reIsPlainProp.test(value) ||
+    !reIsDeepProp.test(value) ||
+    (object != null && value in Object(object))
+  );
+}
+
+/**
+ * Converts `string` to a property path array.
+ *
+ * @private
+ * @param {string} string The string to convert.
+ * @returns {Array} Returns the property path array.
+ */
+function stringToPath(string: string): string[] {
+  const result = [];
+  if (string.charCodeAt(0) === 46 /* . */) {
+    result.push('');
+  }
+  string.replace(rePropName, (match, number, quote, subString) => {
+    result.push(
+      quote ? subString.replace(reEscapeChar, '$1') : number || match,
+    );
+    return match;
+  });
+  return result;
+}
 
 /**
  * Casts `value` to a path array if it's not one.
@@ -75,13 +112,14 @@ function castPath<TPath extends string, TObject>(
   path: TPath,
   obj: TObject,
 ): Exclude<GetFieldType<TObject, TPath>, null | undefined>;
-function castPath(value: string): string[] {
-  const result: string[] = [];
-  let match: RegExpExecArray | null;
-  while ((match = rePropName.exec(value))) {
-    result.push(match[3] ?? match[1]?.trim() ?? match[0]);
+function castPath<TObject extends object>(
+  value: string,
+  object: TObject,
+): string[] {
+  if (Array.isArray(value)) {
+    return value;
   }
-  return result;
+  return isKey(value, object) ? [value] : stringToPath(String(value));
 }
 
 export function getProp<TObject extends object, TKey extends keyof TObject>(

diff --git a/packages/plainjs/test/Parser.ts b/packages/plainjs/test/Parser.ts
@@ -791,6 +791,24 @@ export default function (
     },
   );
 
+  testRunner.add(
+    'should support custom flatten separator using the flatten transform',
+    async (t) => {
+      const opts: ParserOptions = {
+        delimiter: ';',
+        transforms: [flatten({ separator: '.', arrays: true, objects: true })],
+      };
+
+      const parser = new Parser(opts);
+      const csv = await parseInput(
+        parser,
+        jsonFixtures.objectWithEmptyFields(),
+      );
+
+      t.equal(csv, csvFixtures.objectWithEmptyFields);
+    },
+  );
+
   testRunner.add(
     'should support multiple transforms and honor the order in which they are declared',
     async (t) => {

diff --git a/packages/plainjs/test/StreamParser.ts b/packages/plainjs/test/StreamParser.ts
@@ -872,6 +872,24 @@ export default function (
     },
   );
 
+  testRunner.add(
+    'should support custom flatten separator using the flatten transform',
+    async (t) => {
+      const opts: ParserOptions = {
+        delimiter: ';',
+        transforms: [flatten({ separator: '.', arrays: true, objects: true })],
+      };
+
+      const parser = new Parser(opts);
+      const csv = await parseInput(
+        parser,
+        jsonFixtures['objectWithEmptyFields'](),
+      );
+
+      t.equal(csv, csvFixtures.objectWithEmptyFieldsStream);
+    },
+  );
+
   testRunner.add(
     'should support multiple transforms and honor the order in which they are declared',
     async (t) => {

diff --git a/packages/test-helpers/fixtures/csv/objectWithEmptyFields.csv b/packages/test-helpers/fixtures/csv/objectWithEmptyFields.csv
@@ -0,0 +1,3 @@
+"a..b";"anything"
+1;
+;2
diff --git a/packages/test-helpers/fixtures/csv/objectWithEmptyFieldsStream.csv b/packages/test-helpers/fixtures/csv/objectWithEmptyFieldsStream.csv
@@ -0,0 +1,2 @@
+"a..b"
+1
diff --git a/packages/test-helpers/fixtures/json/objectWithEmptyFields.json b/packages/test-helpers/fixtures/json/objectWithEmptyFields.json
@@ -0,0 +1,12 @@
+[
+  {
+    "a": {
+      "": {
+        "b": 1
+      }
+    }
+  },
+  {
+    "anything": 2
+  }
+]
diff --git a/packages/transforms/src/utils.ts b/packages/transforms/src/utils.ts
@@ -41,23 +41,60 @@ type GetFieldType<T, P> = P extends `${infer Left}.${infer Right}`
 
 type PropertyName = string | number | symbol;
 
-const rePropName = RegExp(
-  // Match anything that isn't a dot or bracket.
-  '[^.[\\]]+' +
-    '|' +
-    // Or match property names within brackets.
-    '\\[(?:' +
-    // Match a non-string expression.
-    '([^"\'][^[]*)' +
-    '|' +
-    // Or match strings (supports escaping characters).
-    '(["\'])((?:(?!\\2)[^\\\\]|\\\\.)*?)\\2' +
-    ')\\]' +
-    '|' +
-    // Or match "" as the space between consecutive dots or empty brackets.
-    '(?=(?:\\.|\\[\\])(?:\\.|\\[\\]|$))',
-  'g',
-);
+const reIsDeepProp = /\.|\[(?:[^[\]]*|(["'])(?:(?!\1)[^\\]|\\.)*?\1)\]/;
+const reIsPlainProp = /^\w*$/;
+const rePropName =
+  /[^.[\]]+|\[(?:(-?\d+(?:\.\d+)?)|(["'])((?:(?!\2)[^\\]|\\.)*?)\2)\]|(?=(?:\.|\[\])(?:\.|\[\]|$))/g;
+const reEscapeChar = /\\(\\)?/g;
+
+/**
+ * Checks if `value` is a property name and not a property path.
+ *
+ * @private
+ * @param {*} value The value to check.
+ * @param {Object} [object] The object to query keys on.
+ * @returns {boolean} Returns `true` if `value` is a property name, else `false`.
+ */
+function isKey<TObject extends object>(value: any, object: TObject): boolean {
+  if (Array.isArray(value)) {
+    return false;
+  }
+  const type = typeof value;
+  if (
+    type == 'number' ||
+    type == 'symbol' ||
+    type == 'boolean' ||
+    value == null
+  ) {
+    return true;
+  }
+  return (
+    reIsPlainProp.test(value) ||
+    !reIsDeepProp.test(value) ||
+    (object != null && value in Object(object))
+  );
+}
+
+/**
+ * Converts `string` to a property path array.
+ *
+ * @private
+ * @param {string} string The string to convert.
+ * @returns {Array} Returns the property path array.
+ */
+function stringToPath(string: string): string[] {
+  const result = [];
+  if (string.charCodeAt(0) === 46 /* . */) {
+    result.push('');
+  }
+  string.replace(rePropName, (match, number, quote, subString) => {
+    result.push(
+      quote ? subString.replace(reEscapeChar, '$1') : number || match,
+    );
+    return match;
+  });
+  return result;
+}
 
 /**
  * Casts `value` to a path array if it's not one.
@@ -75,13 +112,14 @@ function castPath<TPath extends string, TObject>(
   path: TPath,
   obj: TObject,
 ): Exclude<GetFieldType<TObject, TPath>, null | undefined>;
-function castPath(value: string): string[] {
-  const result: string[] = [];
-  let match: RegExpExecArray | null;
-  while ((match = rePropName.exec(value))) {
-    result.push(match[3] ?? match[1]?.trim() ?? match[0]);
+function castPath<TObject extends object>(
+  value: string,
+  object: TObject,
+): string[] {
+  if (Array.isArray(value)) {
+    return value;
   }
-  return result;
+  return isKey(value, object) ? [value] : stringToPath(String(value));
 }
 
 export function getProp<TObject extends object, TKey extends keyof TObject>(

diff --git a/packages/whatwg/test/AsyncParser.ts b/packages/whatwg/test/AsyncParser.ts
@@ -904,6 +904,24 @@ export default function (
     },
   );
 
+  testRunner.add(
+    'should support custom flatten separator using the flatten transform',
+    async (t) => {
+      const opts: ParserOptions = {
+        delimiter: ';',
+        transforms: [flatten({ separator: '.', arrays: true, objects: true })],
+      };
+
+      const parser = new Parser(opts);
+      const csv = await parseInput(
+        parser,
+        jsonFixtures.objectWithEmptyFields(),
+      );
+
+      t.equal(csv, csvFixtures.objectWithEmptyFieldsStream);
+    },
+  );
+
   testRunner.add(
     'should support multiple transforms and honor the order in which they are declared',
     async (t) => {