Certain strings get erroneously deserialised to ObjectID
H3xept opened this issue · 2 comments
H3xept commented
Describe the bug
In RedisCacheEngine
there's a check with ObjectId.isValid
to determine if a given string is an objectId. If the function returns true, the string is used to build an ObjectId.
This is wrong as isValid
will return true for strings that are not ObjectId(s) (but can potentially be converted to objectIds). A Clear reproducible example of this is with a 12 character string. See full bson implementation of isValid
here.
To Reproduce
Steps to reproduce the behavior:
- Create a cache entry containing a string field with 12 characters.
- Retrieve such cache entry.
- Validate that the returned object has been altered.
Here's a suitable unit test:
it('should not misclassify certain fields as objectIds', async () => {
// ObjectId.isValid will return true for multiple scenarios.
// A string being a potentially valid objectId should not be the
// determining factor on wether or not deserialize it as objectId.
await User.create({ name: '12CharString', role: 'admin' })
const miss = await User.find({ name: '12CharString' }).lean().cache('30 seconds')
const hit = await User.find({ name: '12CharString' }).lean().cache('30 seconds')
expect(miss).not.toBeNull()
expect(hit).not.toBeNull()
expect(hit).toEqual(miss)
})
Expected behavior
Fields do not get altered.
ilovepixelart commented
Good catch @H3xept!
Since we support mongo 6+ @mindrunner we can leverage new method in mongoose isObjectIdOrHexString
PR: #121
ilovepixelart commented