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

The number accessors are too permissive #143

Closed
jerome-fox opened this issue Apr 22, 2021 · 3 comments
Closed

The number accessors are too permissive #143

jerome-fox opened this issue Apr 22, 2021 · 3 comments

Comments

@jerome-fox
Copy link
Contributor

Describe the bug πŸ›
The number accessors are too permissive.
The parseInt and parseFloat functions allow strings beginning with numbers.

To Reproduce πŸ“

parseInt('123hello') // 123
parseFloat('192.168.1.1') // 192.168

Expected behaviour πŸ€·β€β™‚οΈπŸ€·
Sending '123hello' or '192.168.1.1' should throw an exception

Additional context
The solution could be to check if the parsed value is stringify, it should be the same as the input value.
I've a PR ready if you're interested.

@evanshortiss
Copy link
Owner

evanshortiss commented May 1, 2021

@jerome-fox good find. I am happy to take a look at the PR!

A regex might do the trick too? Something like:

const numbers = 'a123'.match(/^[0-9]*$/)

if (numbers === null) {
  // Error, string contained non numeric characters
}

@jerome-fox
Copy link
Contributor Author

@evanshortiss I made the PR #144
A regex do the trick too.

My solution has one problem : if a number starts or ends with some 0 characters

00234 will be parsed has 234 but as string it will be 234 and not 00234
we should trim the string to remove unnecessary 0 characters (but keep one 0 if the number is 0 or 0.xxx)
I put aside this problem because I think that we do not really give a number with unnecessary 0
But the solution with 2 regex (one for integer /^[0-9]+$/ and one for float /^[0-9]+(\.[0-9]+)?$/) fixes this problem

@evanshortiss
Copy link
Owner

Thanks @jerome-fox will create a patch release shortly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants