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

Enforce stricter integer parsing on encode function #24

Merged

Conversation

ebramanti
Copy link
Contributor

@ebramanti ebramanti commented Aug 29, 2016

Currently, the encode function in hashids is using parseInt in order to validate that a stringified number is indeed a number before encoding. However, there is a caveat to parseInt, detailed in MDN's documentation:

If parseInt encounters a character that is not a numeral in the specified radix, it ignores it and all succeeding characters and returns the integer value parsed up to that point.

If an API consumer were to pass hashids.encode('B6'), this would result in a proper result of '' since the API specifies returning an empty string if an integer is not received.

However, if the consumer were to pass hashids.encode('6B'), the parseInt function would parse this as 6 and ignore the succeeding characters. This would allow the encode function to continue with the encode, even though the function was passed a bad value.

I've updated the tests to demonstrate the issue. To address the bug, I added a stricter filterInt function. This function verifies that the number being passed is indeed an integer before executing parseInt. I pulled filterInt from MDN's documentation on parseInt and modified it to be a bit cleaner.

Thanks to @juliancoleman for pairing with me to find/implement a fix for this bug.

filterInt enforces that the value to be encoded is indeed an integer.

From parseInt documentation on MDN:

> If parseInt encounters a character that is not a numeral in the
specified radix, it ignores it and all succeeding characters and
returns the integer value parsed up to that point.

See more here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global
_Objects/parseInt
@coveralls
Copy link

coveralls commented Aug 29, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 0036859 on jadengore:feature/encode-validation-integer-parsing into b8b0a6e on ivanakimov:master.

@ebramanti ebramanti changed the title Enforce srtricter integer parsing on encode function Enforce stricter integer parsing on encode function Aug 29, 2016
@jd327 jd327 merged commit 9b64989 into niieani:master Aug 29, 2016
@jd327
Copy link
Collaborator

jd327 commented Aug 29, 2016

Awesome description & good catch. Thank you @jadengore.

@ebramanti ebramanti deleted the feature/encode-validation-integer-parsing branch August 29, 2016 17:58
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

Successfully merging this pull request may close these issues.

None yet

3 participants