Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ObjectID.isValid function return true for number #39

Open
mjancarik opened this issue May 18, 2021 · 7 comments
Open

ObjectID.isValid function return true for number #39

mjancarik opened this issue May 18, 2021 · 7 comments

Comments

@mjancarik
Copy link

Hi,

I have problem with ObjectID.isValid function which returns true for number. I think that number is not valid ObjectID or I miss something important. I found that behaviour when I update to [email protected].

@mjancarik
Copy link
Author

@mjancarik mjancarik reopened this May 27, 2021
@niftylettuce
Copy link
Collaborator

Can you submit a PR to fix this?

@niftylettuce
Copy link
Collaborator

The PR #46 broke all tests. I am going to revert it in the interim.

I also noticed we need to merge this 418sec#2.

@supersime
Copy link

Hey there @niftylettuce I have a suggested resolution to this based on how the mongoose library has tackled this. They created a new method to address the issue, so they now have:

  • isValidObjectId: Returns true if Mongoose can cast the given value to an ObjectId, or false otherwise.
  • isObjectIdOrHexString Returns true if the given value is a Mongoose ObjectId (using instanceof) or if the given value is a 24 character hex string, which is the most commonly used string representation of an ObjectId.

Here is a great write-up on the issues they encountered: Automattic/mongoose#11419

Thanks!

@titanism
Copy link
Contributor

@supersime it also seems that Mongoose will return true when it's an IP address, such as 192.168.1.20

@titanism
Copy link
Contributor

> require('bson-objectid').isValid('192.168.1.20')
true
> require('mongoose').Types.ObjectId.isValid('192.168.1.20')
true

@titanism
Copy link
Contributor

We should probably update this repo to use the approach with hex string and 24 char length as per isObjectIdOrHexString

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

No branches or pull requests

4 participants