Skip to content

fix(helpers): resolveObjectKey returns wrong value for Object.prototype keys - #12286

Open
contactjawad wants to merge 1 commit into
chartjs:masterfrom
contactjawad:fix-resolve-object-key-proto
Open

fix(helpers): resolveObjectKey returns wrong value for Object.prototype keys#12286
contactjawad wants to merge 1 commit into
chartjs:masterfrom
contactjawad:fix-resolve-object-key-proto

Conversation

@contactjawad

Copy link
Copy Markdown

What

resolveObjectKey returns an incorrect value (or throws) when the key name collides with an Object.prototype member such as toString, hasOwnProperty, valueOf, isPrototypeOf, __proto__, etc.

Why

The module-level resolver cache keyResolvers was created as a plain object literal, so it inherits from Object.prototype. The cache-miss check

const resolver = keyResolvers[key] || (keyResolvers[key] = _getKeyResolver(key));

sees the inherited prototype method as a truthy value for any such key, and never builds the correct resolver. That inherited function is then invoked as resolver(obj), producing garbage (e.g. toString called with the wrong this returns '[object Undefined]') or throwing (__proto__ is not callable).

How

Make the cache prototype-free by creating it with Object.create(null). Keys that match Object.prototype members now miss the cache and fall through to _getKeyResolver, which builds the correct literal path resolver. Existing '', x, and y fast-paths are unchanged.

Test

Added a case under the resolveObjectKey describe block asserting own properties named after prototype members resolve to their real values:

expect(helpers.resolveObjectKey({toString: 5}, 'toString')).toEqual(5);
expect(helpers.resolveObjectKey({hasOwnProperty: 42}, 'hasOwnProperty')).toEqual(42);
expect(helpers.resolveObjectKey({valueOf: 7}, 'valueOf')).toEqual(7);

Before the fix the first assertion fails with '[object Undefined]' to equal 5; after the fix all pass.

…pe keys

The resolver cache was a plain object literal, so it inherited from
Object.prototype. Keys matching prototype members (e.g. toString,
hasOwnProperty, valueOf) hit the inherited method instead of falling
through to build the correct resolver, yielding wrong output. Use a
null-prototype object for the cache.
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