Skip to content

Fix handling of invalid bytes in stream decoders - #528

Merged
Bodigrim merged 1 commit into
masterfrom
fix-528
Jul 10, 2023
Merged

Fix handling of invalid bytes in stream decoders#528
Bodigrim merged 1 commit into
masterfrom
fix-528

Conversation

@Lysxia

@Lysxia Lysxia commented Jul 10, 2023

Copy link
Copy Markdown
Contributor

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

Lysxia commented Jul 10, 2023

Copy link
Copy Markdown
Contributor Author

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.

@Bodigrim Bodigrim left a comment

Copy link
Copy Markdown
Contributor

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
@Lysxia
Lysxia deleted the fix-528 branch January 27, 2026 11:12
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