Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: revert breaking change in results creation #2591

Merged
merged 7 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading