Skip to content

Commit

Permalink
fix: revert breaking change in results creation (#2591)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
wellwelwel authored Apr 17, 2024
1 parent 7f5b395 commit f7c60d0
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 167 deletions.
16 changes: 13 additions & 3 deletions lib/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
function srcEscape(str) {
return JSON.stringify({
[str]: 1
[str]: 1,
}).slice(1, -3);
}

Expand All @@ -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');
Expand All @@ -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;
19 changes: 9 additions & 10 deletions lib/parsers/binary_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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') {
Expand All @@ -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)}]`;
Expand Down
19 changes: 9 additions & 10 deletions lib/parsers/text_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand All @@ -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(
Expand Down
55 changes: 55 additions & 0 deletions test/esm/integration/parsers/execute-results-creation.test.mjs
Original file line number Diff line number Diff line change
@@ -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();
});
55 changes: 55 additions & 0 deletions test/esm/integration/parsers/query-results-creation.test.mjs
Original file line number Diff line number Diff line change
@@ -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();
});
24 changes: 24 additions & 0 deletions test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs
Original file line number Diff line number Diff line change
@@ -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}`,
);
}
});
24 changes: 24 additions & 0 deletions test/esm/unit/parsers/ensure-safe-text-fields.test.mjs
Original file line number Diff line number Diff line change
@@ -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}`,
);
}
});
72 changes: 0 additions & 72 deletions test/esm/unit/parsers/prototype-binary-results.test.mjs

This file was deleted.

Loading

0 comments on commit f7c60d0

Please sign in to comment.