Skip to content

stream: fix NewDecryptReaderAt EOF handling#708

Open
arpitguptagithub wants to merge 1 commit intoFiloSottile:mainfrom
arpitguptagithub:fix-stream-eof
Open

stream: fix NewDecryptReaderAt EOF handling#708
arpitguptagithub wants to merge 1 commit intoFiloSottile:mainfrom
arpitguptagithub:fix-stream-eof

Conversation

@arpitguptagithub
Copy link
Copy Markdown

The bug

I noticed that the NewDecryptReaderAt fails when the io.ReaderAt it's given returns io.EOF along with a successful read of the final chunk.

The current code is like :

if _, err := src.ReadAt(finalChunk, finalChunkOff); err != nil {
    return nil, fmt.Errorf("failed to read final chunk: %w", err)
}

The io.ReaderAt docs say implementations are allowed to return err == io.EOF even when n == len(p), if the read happens to land right at EOF. The current code treats that as a fatal error.... so whether decryption works or not depends on which ReaderAt you pass in. bytes.Reader happens to return nil here so it works, but other valid implementations hit the EOF path and break.

The fix

If ReadAt returns io.EOF but we got all the bytes we asked for, treat it as success and move on. If we got fewer bytes than expected, surface it as io.ErrUnexpectedEOF instead ... which is the more accurate error anyway.

This is the same EOF handling that DecryptReaderAt.ReadAt already does a few lines down in the same file. The constructor was just inconsistent with the method on the same type.

Test

Added TestDecryptReaderAtEOF in internal/stream/stream_test.go. It wraps the source in a small eofReaderAt helper that forces an eager io.EOF at the payload boundary, so this edge case stays covered going forward.

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