ilovepixelart/ts-cache-mongoose

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:

  1. Create a cache entry containing a string field with 12 characters.
  2. Retrieve such cache entry.
  3. 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