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 handling of invalid bytes in stream decoders #528

Merged
merged 1 commit into from
Jul 10, 2023
Merged

Conversation

Lysxia
Copy link
Contributor

@Lysxia Lysxia commented Jul 10, 2023

This is a different bug that I found while troubleshooting #525. When we have 4 bytes buffered and they are invalid (whether in UTF8, 16, or 32), we skip the first byte, but the following (fifth) byte was being discarded instead of added to the end of the buffer. Interestingly, this bug and #525 have been around since 0.3.

This includes the commit from #527 because the changes are next to each other so there would be a conflict to resolve anyway. Rebasing should remove it.

@Lysxia
Copy link
Contributor Author

Lysxia commented Jul 10, 2023

The broken test suggests that the strict decodeUtf8With is also buggy when the flag simdutf is disabled.

         t_decode_utf8_lenient:                                FAIL (0.01s)
          *** Failed! Falsified (after 99 tests and 55 shrinks):
          ["!\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL","\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\128\NUL"]
          "!\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\65533\NUL" /= "!\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\128\NUL"

Update: I can't reproduce the bug on my linux machine.

Copy link
Contributor

@Bodigrim Bodigrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please rebase.

Update: I can't reproduce the bug on my linux machine.

I raised a separate ticket #529 to investigate.

@Bodigrim Bodigrim merged commit bc6f403 into master Jul 10, 2023
49 checks passed
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

2 participants