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

Bugfix - Handle invalid characters when decoding #6

Merged

Conversation

Rican7
Copy link
Contributor

@Rican7 Rican7 commented Mar 27, 2018

When using this library in a project, I ran into an issue when I wrote a test that passed intentionally "bad data" to the decode method.

Specifically, when passing the invalid data to the decode method of the GmpEncoder, which uses gmp_init(), the GMP extension was throwing an unchecked warning about the data not being a valid number within the specified base (62).

I've recreated this error in the provided tests, along with a fix:
screen shot 2018-03-26 at 6 53 48 pm

... Unfortunately, there's not a great way to check for this error at a higher level, due to GMP's usage of warnings and a false return value. Interestingly enough, I've actually run into this problem before when building my own library years ago, so I've actually reported this as a PHP bug, but it was closed with a "Won't Fix" status: https://bugs.php.net/bug.php?id=68002

In any case, I've created this fix to handle the issue for both the GMP encoder's case and for the base encoder, as otherwise not handling this case could lead to an invalid decode result or "data-loss".

@tuupola tuupola merged commit 2f2553a into tuupola:master Mar 27, 2018
@tuupola
Copy link
Owner

tuupola commented Mar 27, 2018

Thanks! Today I also learned about strspn() :)

tuupola added a commit that referenced this pull request Mar 27, 2018
@Rican7 Rican7 deleted the bugfix/handle-invalid-characters-when-decoding branch March 27, 2018 06:42
@Rican7
Copy link
Contributor Author

Rican7 commented Mar 27, 2018

Ha, thank YOU for the merge!

I hadn't known about strspn myself either, but I went and dug through the various PHP string methods, because I just figured that there was something in there that could do it... I just didn't expect it to work so well with a string character set, haha. But hell, if there's anything I've learned about PHP in the 13+ years that I've been writing it, its that the standard library has amazing string utilities... as long as you don't care about UTF-8. 😆

In any case, thanks for the merge and for the open source project! I'll be looking forward to a release. 😃

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.

2 participants