Repository navigation
Keep internal state when cloning a value for an object schema - #3158
Open
yu2971512385-ui wants to merge 1 commit into
Open
yu2971512385-ui wants to merge 1 commit into
yu2971512385-ui wants to merge 1 commit into
Conversation
An object schema with keys clones the value before applying them, with
Object.create() and Object.assign(). That copies own enumerable properties,
which is everything a plain object or an array has, but nothing of what a
Date, Map, Set, RegExp, URL or Buffer keeps in internal slots. The clone
came back with the right prototype and no state, so every method on it
threw, and validation reported no error:
const schema = Joi.object({ unused: Joi.any() });
schema.validate(new Date()).value.getTime();
// TypeError: this is not a Date object.
Hand those values to the shallow clone from hoek, which this function
already uses for the nonEnumerables case. Plain objects and arrays keep
taking the existing path.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
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.
Problem
An object schema with keys clones the value before applying them (
lib/types/keys.js):Object.assign()copies own enumerable properties. That is everything a plain object or an array holds, but nothing of what aDate,Map,Set,RegExp,URLorBufferkeeps in internal slots, so the clone comes back with the right prototype and no state — and validation reports success:schema.validate(value).valueon masternew Date(0).getTime()throwsnew Map([['a', 1]]).get('a')throwsnew Set([1]).has(1)throws/ab/g.test('ab')throwsBuffer.from('hi').toString()throwsnew URL('https://x.y/').toString()throwsAny object schema with at least one key, pattern or rename takes this path —
Joi.object()andJoi.object().unknown()do not, which is why this hides so well. The failure is silent: joi accepts the value, and the corruption only surfaces later, when the caller uses the returned object.Fix
Hand values that are not plain objects or arrays to the shallow clone from hoek, which the same function already uses for the
nonEnumerablescase, and which knows these types. Plain objects, arrays and class instances keep taking the existingObject.create()path, so the common case is unchanged.Tests
Two cases in
test/types/object.js: one asserting the types above survive validation (plus plain objects, arrays and a class instance with a getter), one asserting the input is still not mutated when a property is stripped. Both fail on master and pass with the change;npx labis green (1839 tests, no leaks).One case this does not fix: a plain typed array (
new Uint8Array([1, 2])) is still broken, because hoek's clone handlesBufferbut not other typed arrays. That looked like a separate fix to make in hoek rather than something to work around here, but say the word if you would rather have it covered.AI assistance (Claude) was used to find this — by round-tripping schemas through
describe()/build()and comparing validation results, which is how the broken clone first showed up — and to draft this description. I verified and tested every line.🤖 Generated with Claude Code