Skip to content

Preserve groups named __proto__ - #2094

Open
hao0x1 wants to merge 1 commit into
caolan:masterfrom
hao0x1:codex/fix-groupby-proto-key
Open

hao0x1 wants to merge 1 commit into
caolan:masterfrom
hao0x1:codex/fix-groupby-proto-key

Conversation

@hao0x1

@hao0x1 hao0x1 commented Oct 1, 2026 •

Copy link
Copy Markdown

groupBy loses the group when the iteratee returns __proto__:

const groups = await async.groupBy(['first', 'second'], async () => '__proto__');
JSON.stringify(groups); // before: {}; after: {"__proto__":["first","second"]}

The assignment calls the inherited __proto__ setter instead of creating a group. Each matching item then replaces the previous one. Define an own property when starting a group, while keeping the result a normal object.

The tests cover groupBy, groupByLimit, and groupBySeries, with callbacks and promises. They also check constructor, toString, and hasOwnProperty groups. All six new cases fail before the fix.

Checks on macOS:

  • npm test: lint and all 709 tests pass on Node 20.20.2 and 25.2.1.
  • npm run coverage: 706 tests pass on Node 20.20.2; groupByLimit.js has full coverage.
  • The existing Karma suite passes all 697 tests in Firefox 153. The temporary test profile connects directly to localhost (network.proxy.type=0); repository test files and configuration are unchanged.
  • make build-modules fails on both unchanged master and this branch because yargs.argv is undefined in support/build/compile-module.js. No build files were changed.

There is a cost to defining each new group this way. In a local synchronous-iteratee benchmark over 1,000 items, the median time went from 0.053 ms to 0.060 ms with 10 groups, and from 0.162 ms to 0.322 ms with 1,000 distinct groups. These are microbenchmark results, not measurements of an I/O workload. Using defineProperty keeps normal object behavior and also handles keys that coerce to __proto__.

Prepared with OpenAI Codex, including the investigation, code, and tests.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant