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

compress/gzip: Reader returns EOF even if requested read successfully completed. #24713

Closed
Dirbaio opened this issue Apr 5, 2018 · 3 comments

Comments

@Dirbaio
Copy link

Dirbaio commented Apr 5, 2018

go version go1.10.1 linux/amd64

  • Call gzip.Reader.Read() with an n-byte slice
  • If there is exactly n remaining bytes, it successfully reads them, returning n, but it returns EOF as error even though it read all the n requested bytes successfully.

Playground: https://play.golang.org/p/ohlYXG931mh

I think it should return n, nil because it has read all the requested bytes successfully. IMO Read() should only return EOF if the passed slice is bigger than the remaining bytes (ie, you requested n bytes, but only fewer could be read).

This is an issue because there's lots of code out there that assume this behavior, and break when passed a gzip.Reader.

os.File doesn't return EOF in this case: https://play.golang.org/p/S1JiyxrrBpu
bytes.Reader neither: https://play.golang.org/p/Xx2yEQIMXjp

@fraenkel
Copy link
Contributor

fraenkel commented Apr 5, 2018

Not sure why you think this is a bug. The gzip.Read says it follows io.Reader which clearly documents this behavior.

@Dirbaio
Copy link
Author

Dirbaio commented Apr 5, 2018

The docs say:

When Read encounters an error or end-of-file condition after successfully reading n > 0 bytes, it returns the number of bytes read. It may return the (non-nil) error from the same call or return the error (and n == 0) from a subsequent call. An instance of this general case is that a Reader returning a non-zero number of bytes at the end of the input stream may return either err == EOF or err == nil. The next Read should return 0, EOF.

That paragraph doesn't say that the error or EOF can happen after reading all of the requested bytes. (You're right it doesn't explicitly forbid it either).

Applying common sense here tells you "I'm requesting len(p) bytes, if for whatever reason you can't read them, give me whatever you could read, and the error". It's not obvious at all that the read can be fully successful AND return an error at the same time.

Why isn't Read() specified to return an error if and only if n != len(p)?

If this can't be done for backwards compat or some reason, maybe the docs could be updated to be more explicit about this.

@dsnet
Copy link
Member

dsnet commented Apr 6, 2018

This is not specific to gzip, but actually very common across many io.Reader in Go.

The paragraph you cited explicitly calls this out:

Reader returning a non-zero number of bytes at the end of the input stream may return either err == EOF or err == nil

We certainly don't want to document that this always returns io.EOF early since that's actually not the behavior either. See #14867 for some in-depth discussion about detected the end of a stream in DEFLATE.

@dsnet dsnet closed this as completed Apr 6, 2018
@golang golang locked and limited conversation to collaborators Apr 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants