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

Fix bug for when the first char is non-ascii char #5

Merged
merged 1 commit into from
May 18, 2024

Conversation

aleris
Copy link
Contributor

@aleris aleris commented May 17, 2024

Crashes with:
Index 258 out of bounds for length 256
java.lang.ArrayIndexOutOfBoundsException:
Index 258 out of bounds for length 256

This adds a check for bigger SUFFIX_LOOKUP.length indexes for the first char.

@fxlae
Copy link
Owner

fxlae commented May 17, 2024

Thanks, good catch!

Now that I'm looking at that line, I'm wondering why I didn't just check the first character directly against the range of valid characters (if (firstChar < '0' || firstChar > '7') // -> error) and avoid the lookup altogether. The second part of the check ((SUFFIX_LOOKUP[firstChar] >>> 3) & 0x3) > 0) isn't that great either and could then be removed.

What do you think?

Crashes with:
Index 258 out of bounds for length 256
java.lang.ArrayIndexOutOfBoundsException:
Index 258 out of bounds for length 256

This changes the check for first char with a direct range check.
@aleris aleris force-pushed the fix_lookup_non_ascii branch from 85d486e to 043b0aa Compare May 18, 2024 04:43
@aleris
Copy link
Contributor Author

aleris commented May 18, 2024

you are right :) updated

@fxlae fxlae self-requested a review May 18, 2024 08:31
@fxlae
Copy link
Owner

fxlae commented May 18, 2024

Thanks again :) I will push a new release later today.

@fxlae fxlae merged commit c1a9db3 into fxlae:main May 18, 2024
@fxlae fxlae mentioned this pull request May 18, 2024
@aleris aleris deleted the fix_lookup_non_ascii branch May 18, 2024 10:18
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