Skip to content

fix: round-trip __proto__ keys as own properties (#159)#192

Open
spokodev wants to merge 1 commit into
kriszyp:masterfrom
spokodev:fix/proto-key-roundtrip
Open

fix: round-trip __proto__ keys as own properties (#159)#192
spokodev wants to merge 1 commit into
kriszyp:masterfrom
spokodev:fix/proto-key-roundtrip

Conversation

@spokodev

Copy link
Copy Markdown

Implements the __proto__ part of #159.

Problem

A __proto__ object key does not survive a round-trip — it is silently renamed to __proto_ and its value is misplaced:

import { pack, unpack } from 'msgpackr';

const value = JSON.parse('{"__proto__": 1, "a": 2}');
Object.keys(unpack(pack(value))); // ["__proto_", "a"]  ← key corrupted
Object.getOwnPropertyDescriptor(unpack(pack(value)), '__proto__'); // undefined

unpack rewrites the key to __proto_ (in four places) to avoid prototype pollution from object['__proto__'] = …. It avoids pollution, but at the cost of corrupting the data.

Fix

Per the suggestion in #159, set the key with Object.defineProperty, which creates a genuine own __proto__ property without invoking the prototype setter — the same result JSON.parse and structuredClone produce. A small setObjectKey helper handles the three manual readers (the two mapsAsObjects paths and the record structure reader); the optimized compiled reader emits a computed ["__proto__"] key (which is an own property, not the setter), so the fast path is preserved rather than disabled.

Verification

Round-tripping __proto__ now preserves it as an own property across every reader — the record path, the optimized record path (after the read threshold), the mapsAsObjects/map readers, and the default top-level unpack — and the prototype is never mutated (({}).polluted stays undefined even when the value is an object). A test covering these paths is added to tests/test.js.

Note: I couldn't run mocha locally (it fails to start under Node 24+ via its yargs dependency), so the new test is verified against the source through a standalone harness exercising each path; CI runs it on a supported Node.

unpack renamed a `__proto__` object key to `__proto_` to avoid prototype
pollution, which silently corrupted the key and dropped its value
(`unpack(pack({['__proto__']: 1}))` came back with a `__proto_` key).

As suggested in kriszyp#159, `Object.defineProperty` sets it as a real own
property without touching the prototype — matching how `JSON.parse` and
`structuredClone` handle the same key.

Applied to every object-reading path: the two map-as-object readers, the
record structure reader, and its optimized (compiled) variant, which now
emits a computed `["__proto__"]` key instead of the prototype setter.
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