fix: revert breaking change in results creation

[wellwelwel]

The previous solution (#2574) aimed to create a clean object, but it caused a major change.

Instead, I just grouped the object's private properties:

const privateObjectProps = new Set([
  '__defineGetter__',
  '__defineSetter__',
  '__lookupGetter__',
  '__lookupSetter__',
  '__proto__',
]);

Then, it will perform a simple validation to ensure that the fields[i].name is safe:

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.`,
  );
}

By this way, it's possible to:

• use the native methods available from an object
• deep compare results with an expected variable
• test the vulnerability without use a query for that
• If an user, for some reason, wants to create or customize a prototype after the query has been finished, they can

Basically, as it should be for a patch/minor bump 🙋🏻‍♂️

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.32%. Comparing base (6dccf55) to head (a866100). Report is 2 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2591 +/- ## ======================================= Coverage 90.32% 90.32% ======================================= Files 71 71 Lines 15717 15725 +8 Branches 1334 1339 +5 ======================================= + Hits 14196 14204 +8 Misses 1521 1521 ``` | [Flag](https://app.codecov.io/gh/sidorares/node-mysql2/pull/2591/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Sidorov) | Coverage Δ | | |---|---|---| | [compression-0](https://app.codecov.io/gh/sidorares/node-mysql2/pull/2591/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Sidorov) | `90.32% <100.00%> (+<0.01%)` | :arrow_up: | | [compression-1](https://app.codecov.io/gh/sidorares/node-mysql2/pull/2591/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Sidorov) | `90.32% <100.00%> (+<0.01%)` | :arrow_up: | | [tls-0](https://app.codecov.io/gh/sidorares/node-mysql2/pull/2591/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Sidorov) | `89.85% <100.00%> (+<0.01%)` | :arrow_up: | | [tls-1](https://app.codecov.io/gh/sidorares/node-mysql2/pull/2591/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Sidorov) | `90.14% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Sidorov#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[sidorares]

Let's run some benchmarks before merging

[wellwelwel]

@sidorares, I rewrote the logic and description of this PR.

Could you check again? 🙋🏻‍♂️

[wellwelwel]

Using Set and has instead of Array and includes, due to:

Basic benchmark:

const arrayIncludes = (field) => {
  console.time('Array.includes');

  const privateObjectProps = [
    '__defineGetter__',
    '__defineSetter__',
    '__lookupGetter__',
    '__lookupSetter__',
    '__proto__',
  ];

  for (let i = 0; i < 1000000; i++) privateObjectProps.includes(field);

  console.timeEnd('Array.includes');
};

const setHas = (field) => {
  console.time('Set.has');

  const privateObjectProps = new Set([
    '__defineGetter__',
    '__defineSetter__',
    '__lookupGetter__',
    '__lookupSetter__',
    '__proto__',
  ]);

  for (let i = 0; i < 1000000; i++) privateObjectProps.has(field);

  console.timeEnd('Set.has');
};

for (let i = 0; i < 5; i++) arrayIncludes('field');
for (let i = 0; i < 5; i++) setHas('field');

---

Results:

Array.includes: 1.869ms
Array.includes: 0.621ms
Array.includes: 0.644ms
Array.includes: 0.658ms
Array.includes: 0.677ms
Set.has: 1.275ms
Set.has: 0.572ms
Set.has: 0.599ms
Set.has: 0.619ms
Set.has: 0.553ms

---

Compatibility:

Set: Node.js 0.12.18.

[wellwelwel]

Bringing a comment here: https://github.com/sidorares/node-mysql2/commit/74abf9ef94d76114d9a09415e28b496522a94805#r140992502 🤝

[wellwelwel]

As suggested by @sidorares:

Instead of checking the field name manually, we have some alternatives:

• Refactor the results object to a mapped object
• Keep it as Object.create(null) by adding a config option to use a standard object for results (not sure about bump a major version for this)

---

Although the issue #2585 doesn't have that many interactions, I think it's important to consider the users who don't participate directly 🙋🏻‍♂️

Also, if we consider the current solution, it's possible to return an empty object instead of throwing an error, as in mysqljs/mysql.

[wellwelwel]

Merging this.

For a major release, I recommend reverting this PR and include a safer approach in docs, such as:

Object.hasOwn(row, 'column');

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwnProperty#objects_created_with_object.createnull

[ert78gb]

thank you so much for the solution