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

bad problem with ObjectId.isValid() #3823

Closed
ORESoftware opened this issue Jan 30, 2016 · 5 comments
Closed

bad problem with ObjectId.isValid() #3823

ORESoftware opened this issue Jan 30, 2016 · 5 comments
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature
Milestone

Comments

@ORESoftware
Copy link

I am having a helluva a time trying to determining if a value is a valid ObjectId, there seems to be a big problem with this -

I tried

var ObjectId = mongoose.Types.ObjectId;
var ObjectId = mongoose.Schema.Types.ObjectId;

using ObjectId.isValid() and new ObjectId(val);

take a look here:

you can see that the value "95" or 95 slips through:

filter
screenshot 2016-01-29 20 04 30

map
screenshot 2016-01-29 20 04 48

what the heck is going on?
please help, thanks!

@ORESoftware
Copy link
Author

oh shit, wtf

/**
* Checks if a value is a valid bson ObjectId
*
* @method
* @return {boolean} return true if the value is a valid bson ObjectId, return false otherwise.
*/
ObjectID.isValid = function isValid(id) {
  if(id == null) return false;

  if(typeof id == 'number')
    return true;
  if(typeof id == 'string') {
    return id.length == 12 || (id.length == 24 && checkForHexRegExp.test(id));
  }
  return false;
};

what is it ok if it's a number? This makes no sense because 5 moments later Mongo will barf saying 95 can't be cast to ObjectId by the database.

@TrejGun
Copy link
Contributor

TrejGun commented Feb 1, 2016

js-bson looks outdated :(

it also throws error
Buffer.set is deprecated. Use array indexes instead.

can someone update it?

@TrejGun
Copy link
Contributor

TrejGun commented Feb 1, 2016

oh thats nice mongodb/js-bson#160

@vkarpov15
Copy link
Collaborator

new ObjectId(95); is completely valid because a valid ObjectId is any 12 bytes, so the number 95 works just fine. If you want to check for the usual hex string, just use /[a-fA-F0-9]{24}/.test(val);

@fires3as0n
Copy link

It works with strings, but if I try to check if id is valid by using ObjectId.isValid(6), passing 6 as a number, and it says that it is fine.
Then I pass it to .findById(6) and get an exception.
UnhandledPromiseRejectionWarning: CastError: Cast to ObjectId failed for value "6" at path "_id" for model "SomeModel" at new CastError (node_modules\mongoose\lib\cast.js:315:32)

So seems like ObjectId.isValid() is really quite a useless stuff unless you implicitly cast data passed to it to a string.

@vkarpov15 vkarpov15 reopened this Nov 8, 2019
@vkarpov15 vkarpov15 added this to the 5.7.10 milestone Nov 8, 2019
@vkarpov15 vkarpov15 added the needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue label Nov 8, 2019
@vkarpov15 vkarpov15 modified the milestones: 5.7.10, 5.7.11 Nov 11, 2019
@vkarpov15 vkarpov15 added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature and removed needs repro script Maybe a bug, but no repro script. The issue reporter should create a script that demos the issue labels Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature
Projects
None yet
Development

No branches or pull requests

4 participants