Skip to content

Guard the custom-toString check against objects without a callable toString - #579

Closed
lenamonj wants to merge 1 commit into
JedWatson:mainfrom
lenamonj:fix-custom-tostring-guard
Closed

lenamonj wants to merge 1 commit into
JedWatson:mainfrom
lenamonj:fix-custom-tostring-guard

Conversation

@lenamonj

@lenamonj lenamonj commented Sep 1, 2026

Copy link
Copy Markdown

All three entry points throw TypeError on objects that lack a callable toString - and that includes a shape modern JavaScript hands out routinely: Object.groupBy returns null-prototype objects.

classNames(Object.create(null));            // TypeError: Cannot read properties of undefined (reading 'toString')
classNames(Object.groupBy([1, 2], String)); // same
classNames({ toString: null, a: true });    // TypeError: toString is not a function

The custom-toString check in each module reads arg.toString and calls it (via the toString.toString() inclusion test) without verifying the method exists and is callable. The fix adds one typeof arg.toString === 'function' guard ahead of the existing check, at all three sites (index.js, bind.js, dedupe.js).

Behavior preserved: the guard can only stop the custom-toString branch from being taken, never start it. An object with a callable custom toString still stringifies through it (the existing classFromMethod and cross-realm VM tests still pass), a plain object still falls through to own-key iteration, and the only inputs whose result changes are those that previously threw - verified with a differential sweep between the unfixed and fixed trees.

Performance: per CONTRIBUTING's concern - the cost is one typeof on a property already being read on that path, and only on the object branch; the string/number fast paths are untouched. Happy to run the benchmark suite comparison if you'd like it in the record.

Tests: eight regression tests - null-prototype object, Object.groupBy output, and non-callable toString, per module and for both the bound and unbound bind forms. They fail on main (TypeError) and pass with the fix; npm test and npm run check-types are green.


Provenance, for transparency: this defect was found by an automated audit loop I run against open-source projects; the patch and its verification were reviewed by me before filing, and I'm happy to answer any questions during review.

…String

All three entry points threw TypeError on an object with no
Object.prototype in its chain or whose toString is not callable -
including the null-prototype objects Object.groupBy returns. The
custom-toString check now verifies the method is callable before
invoking it, at all three sites (index.js, bind.js, dedupe.js).

An object with a callable custom toString still stringifies through it;
a plain object still falls through to own-key iteration. The only
inputs whose result changes are those that previously threw. Regression
tests cover null-prototype objects, Object.groupBy output and
non-callable toString per module, for both bound and unbound forms.
@lenamonj

Copy link
Copy Markdown
Author

Closing to keep the queue current; the fix stands if a maintainer wants to pick it up.

@lenamonj lenamonj closed this Oct 24, 2026
@lenamonj
lenamonj deleted the fix-custom-tostring-guard branch October 24, 2026 00:06
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