Repository navigation
Conversation
…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.
Author
|
Closing to keep the queue current; the fix stands if a maintainer wants to pick it up. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All three entry points throw
TypeErroron objects that lack a callabletoString- and that includes a shape modern JavaScript hands out routinely:Object.groupByreturns null-prototype objects.The custom-toString check in each module reads
arg.toStringand calls it (via thetoString.toString()inclusion test) without verifying the method exists and is callable. The fix adds onetypeof 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
toStringstill stringifies through it (the existingclassFromMethodand 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
typeofon 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.groupByoutput, and non-callabletoString, per module and for both the bound and unbound bind forms. They fail onmain(TypeError) and pass with the fix;npm testandnpm run check-typesare 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.