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

audit-argument-checks throws an error for NaN #2829

Closed
mitar opened this issue Oct 16, 2014 · 4 comments
Closed

audit-argument-checks throws an error for NaN #2829

mitar opened this issue Oct 16, 2014 · 4 comments

Comments

@mitar
Copy link
Contributor

mitar commented Oct 16, 2014

If I have such method:

  Meteor.methods({
    'test': function (test) {
      check(test, Number);
    }
  });

And I call the method with NaN as test value, an error is thrown:

I20141016-01:42:50.170(-7)? Exception while invoking method 'test' Error: Did not check() all arguments during call to 'test'
I20141016-01:42:50.173(-7)?     at _.extend.throwUnlessAllArgumentsHaveBeenChecked (packages/check/match.js:352)
I20141016-01:42:50.174(-7)?     at Object.Match._failIfArgumentsAreNotAllChecked (packages/check/match.js:108)
I20141016-01:42:50.174(-7)?     at maybeAuditArgumentChecks (packages/ddp/livedata_server.js:1591)
I20141016-01:42:50.174(-7)?     at packages/ddp/livedata_server.js:648
I20141016-01:42:50.175(-7)?     at _.extend.withValue (packages/meteor/dynamics_nodejs.js:56)
I20141016-01:42:50.175(-7)?     at packages/ddp/livedata_server.js:647
I20141016-01:42:50.175(-7)?     at _.extend.withValue (packages/meteor/dynamics_nodejs.js:56)
I20141016-01:42:50.175(-7)?     at _.extend.protocol_handlers.method (packages/ddp/livedata_server.js:646)
I20141016-01:42:50.176(-7)?     at packages/ddp/livedata_server.js:546

This is not correct. I am checking the value. For normal numbers everything works as expected.

@mitar
Copy link
Contributor Author

mitar commented Oct 16, 2014

Example repository. Click on the button which calls the method and error happens. Which should not.

@glasser
Copy link
Contributor

glasser commented Oct 21, 2014

That makes sense, since we do the audit-argument-checks logic using ===. I guess we need special-case code for NaN here. Pull requests encouraged.

@matteodem
Copy link
Contributor

see #2914 for a fix

@glasser
Copy link
Contributor

glasser commented Dec 3, 2014

Fixed, thanks @matteodem !

@glasser glasser closed this as completed Dec 3, 2014
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

3 participants