fix: revert breaking change in results creation (#2591)

* fix: revert breaking change in results creation * refactor: simplify prototype validation * ci: fix typo * chore: improve performance * chore: fix lint * chore: change from `nativeObjectProps` to `privateObjectProps` * chore: improve error message
sidorares · Apr 17, 2024 · f7c60d0 · f7c60d0
1 parent 7f5b395
commit f7c60d0
Show file tree

Hide file tree

Showing 9 changed files with 189 additions and 167 deletions.
diff --git a/lib/helpers.js b/lib/helpers.js
@@ -16,7 +16,7 @@
   */
 function srcEscape(str) {
   return JSON.stringify({
-    [str]: 1
+    [str]: 1,
   }).slice(1, -3);
 }
 
@@ -29,7 +29,7 @@ try {
   const REQUIRE_TERMINATOR = '';
   highlightFn = require(`cardinal${REQUIRE_TERMINATOR}`).highlight;
 } catch (err) {
-  highlightFn = text => {
+  highlightFn = (text) => {
     if (!cardinalRecommended) {
       // eslint-disable-next-line no-console
       console.log('For nicer debug output consider install cardinal@^2.0.0');
@@ -56,10 +56,20 @@ exports.printDebugWithCode = printDebugWithCode;
  */
 function typeMatch(type, list, Types) {
   if (Array.isArray(list)) {
-    return list.some(t => type === Types[t]);
+    return list.some((t) => type === Types[t]);
   }
 
   return !!list;
 }
 
 exports.typeMatch = typeMatch;
+
+const privateObjectProps = new Set([
+  '__defineGetter__',
+  '__defineSetter__',
+  '__lookupGetter__',
+  '__lookupSetter__',
+  '__proto__',
+]);
+
+exports.privateObjectProps = privateObjectProps;
diff --git a/lib/parsers/binary_parser.js b/lib/parsers/binary_parser.js
@@ -122,13 +122,7 @@ function compile(fields, options, config) {
   if (options.rowsAsArray) {
     parserFn(`const result = new Array(${fields.length});`);
   } else {
-    parserFn('const result = Object.create(null);');
-    parserFn(`Object.defineProperty(result, "constructor", {
-      value: Object.create(null),
-      writable: false,
-      configurable: false,
-      enumerable: false
-    });`);
+    parserFn('const result = {};');
   }
 
   // Global typeCast
@@ -152,6 +146,13 @@ function compile(fields, options, config) {
 
   for (let i = 0; i < fields.length; i++) {
     fieldName = helpers.srcEscape(fields[i].name);
+
+    if (helpers.privateObjectProps.has(fields[i].name)) {
+      throw new Error(
+        `The field name (${fieldName}) can't be the same as an object's private property.`,
+      );
+    }
+
     parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
 
     if (typeof options.nestTables === 'string') {
@@ -160,9 +161,7 @@ function compile(fields, options, config) {
       )}]`;
     } else if (options.nestTables === true) {
       tableName = helpers.srcEscape(fields[i].table);
-      parserFn(
-        `if (!result[${tableName}]) result[${tableName}] = Object.create(null);`,
-      );
+      parserFn(`if (!result[${tableName}]) result[${tableName}] = {};`);
       lvalue = `result[${tableName}][${fieldName}]`;
     } else if (options.rowsAsArray) {
       lvalue = `result[${i.toString(10)}]`;

diff --git a/lib/parsers/text_parser.js b/lib/parsers/text_parser.js
@@ -131,13 +131,7 @@ function compile(fields, options, config) {
   if (options.rowsAsArray) {
     parserFn(`const result = new Array(${fields.length});`);
   } else {
-    parserFn('const result = Object.create(null);');
-    parserFn(`Object.defineProperty(result, "constructor", {
-      value: Object.create(null),
-      writable: false,
-      configurable: false,
-      enumerable: false
-    });`);
+    parserFn('const result = {};');
   }
 
   const resultTables = {};
@@ -149,16 +143,21 @@ function compile(fields, options, config) {
     }
     resultTablesArray = Object.keys(resultTables);
     for (let i = 0; i < resultTablesArray.length; i++) {
-      parserFn(
-        `result[${helpers.srcEscape(resultTablesArray[i])}] = Object.create(null);`,
-      );
+      parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
     }
   }
 
   let lvalue = '';
   let fieldName = '';
   for (let i = 0; i < fields.length; i++) {
     fieldName = helpers.srcEscape(fields[i].name);
+
+    if (helpers.privateObjectProps.has(fields[i].name)) {
+      throw new Error(
+        `The field name (${fieldName}) can't be the same as an object's private property.`,
+      );
+    }
+
     parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
     if (typeof options.nestTables === 'string') {
       lvalue = `result[${helpers.srcEscape(

diff --git a/test/esm/integration/parsers/execute-results-creation.test.mjs b/test/esm/integration/parsers/execute-results-creation.test.mjs
@@ -0,0 +1,55 @@
+import { test, describe, assert } from 'poku';
+import { createConnection, describeOptions } from '../../../common.test.cjs';
+
+const connection = createConnection().promise();
+
+describe('Execute: Results Creation', describeOptions);
+
+Promise.all([
+  test(async () => {
+    const expected = [
+      {
+        test: 2,
+      },
+    ];
+    const emptyObject = {};
+    const proto = Object.getPrototypeOf(emptyObject);
+    const privateObjectProps = Object.getOwnPropertyNames(proto);
+
+    const [results] = await connection.execute('SELECT 1+1 AS `test`');
+
+    assert.deepStrictEqual(results, expected, 'Ensure exact object "results"');
+    assert.deepStrictEqual(
+      Object.getOwnPropertyNames(results[0]),
+      Object.getOwnPropertyNames(expected[0]),
+      'Deep ensure exact object "results"',
+    );
+    assert.deepStrictEqual(
+      Object.getPrototypeOf(results[0]),
+      Object.getPrototypeOf({}),
+      'Ensure clean properties in results items',
+    );
+
+    privateObjectProps.forEach((prop) => {
+      assert(prop in results[0], `Ensure ${prop} exists`);
+    });
+
+    results[0].customProp = true;
+    assert.strictEqual(
+      results[0].customProp,
+      true,
+      'Ensure that the end-user is able to use custom props',
+    );
+  }),
+  test(async () => {
+    const [result] = await connection.execute('SET @1 = 1;');
+
+    assert.strictEqual(
+      result.constructor.name,
+      'ResultSetHeader',
+      'Ensure constructor name in result object',
+    );
+  }),
+]).then(async () => {
+  await connection.end();
+});
diff --git a/test/esm/integration/parsers/query-results-creation.test.mjs b/test/esm/integration/parsers/query-results-creation.test.mjs
@@ -0,0 +1,55 @@
+import { test, describe, assert } from 'poku';
+import { createConnection, describeOptions } from '../../../common.test.cjs';
+
+const connection = createConnection().promise();
+
+describe('Query: Results Creation', describeOptions);
+
+Promise.all([
+  test(async () => {
+    const expected = [
+      {
+        test: 2,
+      },
+    ];
+    const emptyObject = {};
+    const proto = Object.getPrototypeOf(emptyObject);
+    const privateObjectProps = Object.getOwnPropertyNames(proto);
+
+    const [results] = await connection.query('SELECT 1+1 AS `test`');
+
+    assert.deepStrictEqual(results, expected, 'Ensure exact object "results"');
+    assert.deepStrictEqual(
+      Object.getOwnPropertyNames(results[0]),
+      Object.getOwnPropertyNames(expected[0]),
+      'Deep ensure exact object "results"',
+    );
+    assert.deepStrictEqual(
+      Object.getPrototypeOf(results[0]),
+      Object.getPrototypeOf({}),
+      'Ensure clean properties in results items',
+    );
+
+    privateObjectProps.forEach((prop) => {
+      assert(prop in results[0], `Ensure ${prop} exists`);
+    });
+
+    results[0].customProp = true;
+    assert.strictEqual(
+      results[0].customProp,
+      true,
+      'Ensure that the end-user is able to use custom props',
+    );
+  }),
+  test(async () => {
+    const [result] = await connection.query('SET @1 = 1;');
+
+    assert.strictEqual(
+      result.constructor.name,
+      'ResultSetHeader',
+      'Ensure constructor name in result object',
+    );
+  }),
+]).then(async () => {
+  await connection.end();
+});
diff --git a/test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs b/test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs
@@ -0,0 +1,24 @@
+import { describe, assert } from 'poku';
+import { describeOptions } from '../../../common.test.cjs';
+import getBinaryParser from '../../../../lib/parsers/binary_parser.js';
+import { srcEscape } from '../../../../lib/helpers.js';
+import { privateObjectProps } from '../../../../lib/helpers.js';
+
+describe('Binary Parser: Block Native Object Props', describeOptions);
+
+const blockedFields = Array.from(privateObjectProps).map((prop) => [
+  { name: prop },
+]);
+
+blockedFields.forEach((fields) => {
+  try {
+    getBinaryParser(fields, {}, {});
+    assert.fail('An error was expected');
+  } catch (error) {
+    assert.strictEqual(
+      error.message,
+      `The field name (${srcEscape(fields[0].name)}) can't be the same as an object's private property.`,
+      `Ensure safe ${fields[0].name}`,
+    );
+  }
+});
diff --git a/test/esm/unit/parsers/ensure-safe-text-fields.test.mjs b/test/esm/unit/parsers/ensure-safe-text-fields.test.mjs
@@ -0,0 +1,24 @@
+import { describe, assert } from 'poku';
+import { describeOptions } from '../../../common.test.cjs';
+import TextRowParser from '../../../../lib/parsers/text_parser.js';
+import { srcEscape } from '../../../../lib/helpers.js';
+import { privateObjectProps } from '../../../../lib/helpers.js';
+
+describe('Text Parser: Block Native Object Props', describeOptions);
+
+const blockedFields = Array.from(privateObjectProps).map((prop) => [
+  { name: prop },
+]);
+
+blockedFields.forEach((fields) => {
+  try {
+    TextRowParser(fields, {}, {});
+    assert.fail('An error was expected');
+  } catch (error) {
+    assert.strictEqual(
+      error.message,
+      `The field name (${srcEscape(fields[0].name)}) can't be the same as an object's private property.`,
+      `Ensure safe ${fields[0].name}`,
+    );
+  }
+});
diff --git a/test/esm/unit/parsers/prototype-binary-results.test.mjs b/test/esm/unit/parsers/prototype-binary-results.test.mjs