From f7c60d01a49666130f51d3847ccfdd3d6e3d33e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Weslley=20Ara=C3=BAjo?= <46850407+wellwelwel@users.noreply.github.com> Date: Wed, 17 Apr 2024 07:56:28 -0300 Subject: [PATCH] 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 --- lib/helpers.js | 16 ++++- lib/parsers/binary_parser.js | 19 +++-- lib/parsers/text_parser.js | 19 +++-- .../parsers/execute-results-creation.test.mjs | 55 ++++++++++++++ .../parsers/query-results-creation.test.mjs | 55 ++++++++++++++ .../ensure-safe-binary-fields.test.mjs | 24 +++++++ .../parsers/ensure-safe-text-fields.test.mjs | 24 +++++++ .../parsers/prototype-binary-results.test.mjs | 72 ------------------- .../parsers/prototype-text-results.test.mjs | 72 ------------------- 9 files changed, 189 insertions(+), 167 deletions(-) create mode 100644 test/esm/integration/parsers/execute-results-creation.test.mjs create mode 100644 test/esm/integration/parsers/query-results-creation.test.mjs create mode 100644 test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs create mode 100644 test/esm/unit/parsers/ensure-safe-text-fields.test.mjs delete mode 100644 test/esm/unit/parsers/prototype-binary-results.test.mjs delete mode 100644 test/esm/unit/parsers/prototype-text-results.test.mjs diff --git a/lib/helpers.js b/lib/helpers.js index 2a8f80367f..fdba2ddb76 100644 --- 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 index 26ac7893e8..5a08df7b7c 100644 --- 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 index e757b45623..7e46514463 100644 --- 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,9 +143,7 @@ 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])}] = {};`); } } @@ -159,6 +151,13 @@ function compile(fields, options, config) { 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 new file mode 100644 index 0000000000..b6296af2ea --- /dev/null +++ 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 new file mode 100644 index 0000000000..030c33dad1 --- /dev/null +++ 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 new file mode 100644 index 0000000000..a16cc43014 --- /dev/null +++ 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 new file mode 100644 index 0000000000..36ef7f4516 --- /dev/null +++ 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 deleted file mode 100644 index a33eec077b..0000000000 --- a/test/esm/unit/parsers/prototype-binary-results.test.mjs +++ /dev/null @@ -1,72 +0,0 @@ -import { test, describe, assert } from 'poku'; -import { createConnection, describeOptions } from '../../../common.test.cjs'; - -const connection = createConnection().promise(); - -describe('Binary Parser: Prototype Sanitization', describeOptions); - -Promise.all([ - test(async () => { - const expected = [{}]; - expected[0].test = 2; - - const [results] = await connection.query('SELECT 1+1 AS `test`'); - - assert.notDeepStrictEqual( - results, - expected, - `Ensure "results" doesn't contain a standard object ({})`, - ); - }), - test(async () => { - const expected = [Object.create(null)]; - expected[0].test = 2; - - const [results] = await connection.execute('SELECT 1+1 AS `test`'); - - assert.deepStrictEqual(results, expected, 'Ensure clean object "results"'); - assert.strictEqual( - Object.getPrototypeOf(results[0]), - null, - 'Ensure clean properties in results items', - ); - assert.strictEqual( - typeof results[0].toString, - 'undefined', - 'Re-check prototypes (manually) in results columns', - ); - assert.strictEqual( - typeof results[0].test.toString, - 'function', - 'Ensure that the end-user is able to use prototypes', - ); - assert.strictEqual( - results[0].test.toString(), - '2', - 'Ensure that the end-user is able to use prototypes (manually): toString', - ); - assert.strictEqual( - results[0].test.toFixed(2), - '2.00', - 'Ensure that the end-user is able to use prototypes (manually): toFixed', - ); - - 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/unit/parsers/prototype-text-results.test.mjs b/test/esm/unit/parsers/prototype-text-results.test.mjs deleted file mode 100644 index 99a6a69312..0000000000 --- a/test/esm/unit/parsers/prototype-text-results.test.mjs +++ /dev/null @@ -1,72 +0,0 @@ -import { test, describe, assert } from 'poku'; -import { createConnection, describeOptions } from '../../../common.test.cjs'; - -const connection = createConnection().promise(); - -describe('Text Parser: Prototype Sanitization', describeOptions); - -Promise.all([ - test(async () => { - const expected = [{}]; - expected[0].test = 2; - - const [results] = await connection.query('SELECT 1+1 AS `test`'); - - assert.notDeepStrictEqual( - results, - expected, - `Ensure "results" doesn't contain a standard object ({})`, - ); - }), - test(async () => { - const expected = [Object.create(null)]; - expected[0].test = 2; - - const [results] = await connection.query('SELECT 1+1 AS `test`'); - - assert.deepStrictEqual(results, expected, 'Ensure clean object "results"'); - assert.strictEqual( - Object.getPrototypeOf(results[0]), - null, - 'Ensure clean properties in results items', - ); - assert.strictEqual( - typeof results[0].toString, - 'undefined', - 'Re-check prototypes (manually) in results columns', - ); - assert.strictEqual( - typeof results[0].test.toString, - 'function', - 'Ensure that the end-user is able to use prototypes', - ); - assert.strictEqual( - results[0].test.toString(), - '2', - 'Ensure that the end-user is able to use prototypes (manually): toString', - ); - assert.strictEqual( - results[0].test.toFixed(2), - '2.00', - 'Ensure that the end-user is able to use prototypes (manually): toFixed', - ); - - 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(); -});