Skip to content

fix: return EOF when buffer is empty - #472

Closed
secDre4mer wants to merge 2 commits into
bodgit:mainfrom
secDre4mer:main
Closed

fix: return EOF when buffer is empty#472
secDre4mer wants to merge 2 commits into
bodgit:mainfrom
secDre4mer:main

Conversation

@secDre4mer

Copy link
Copy Markdown
Contributor

The bra reader could return invalid results when a read ended exactly so no data was left:
In this case, the rc.rc read returned (0, EOF), rc.n was 0.
Therefore the rc.buf.Read() was called with an empty slice, which caused it to return (0, nil).
This was then returned to the caller, and left the reader in the same state as before.
In combination with e.g. io.ReadAll, this could cause an endless loop.

The bra reader could return invalid results when a read ended
exactly so no data was left: In this case, the rc.rc read returned
(0, EOF), rc.n was 0. Therefore the rc.buf.Read() was called with
an empty slice, which caused it to return (0, nil), which was
then returned to the caller, and left the reader in the same
state as before.
In combination with e.g. io.ReadAll, this could cause an endless
loop.
@bodgit

bodgit commented Jun 25, 2026

Copy link
Copy Markdown
Owner

Is there an archive available that demonstrates the problem so it can be added as a test case?

@secDre4mer

Copy link
Copy Markdown
Contributor Author

I'll upload a test. Please note that I don't own (or trust) that test data file; and I've noticed that reads on some other files in it fail, so it might even be partially invalid...

@secDre4mer

Copy link
Copy Markdown
Contributor Author

@bodgit Have you had the time to look at the uploaded file yet?

@bodgit

bodgit commented Jul 9, 2026

Copy link
Copy Markdown
Owner

@bodgit Have you had the time to look at the uploaded file yet?

Trying to extract the whole archive I get:

sevenzip: read error: lzma2: error reading: lzma: unexpected chunk type

Either the archive is corrupt or it's triggering a bug in the LZMA library that I use. That means I can't add that file to the existing test that just tries to extract the whole archive, but your new test case that just extracts the one file seems to be enough to trigger the bug and it's reproducible on my side; it just gets stuck in a loop.

Your fix seems to work and doesn't impact any of the other existing tests so I will merge this shortly.

@bodgit

bodgit commented Jul 10, 2026

Copy link
Copy Markdown
Owner

I cherry-picked your commits and tweaked the test in #476 so it is wrapped in a timeout and quickly times out rather than wait for the default go test timeout when the fix is backed out.

This will be in the next release. Thanks for the contribution!

@bodgit bodgit closed this Jul 10, 2026
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