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/zlib: Reader returns unexpected EOF when calling Read after EOF #14675

Closed
AsamK opened this issue Mar 6, 2016 · 11 comments
Closed

compress/zlib: Reader returns unexpected EOF when calling Read after EOF #14675

AsamK opened this issue Mar 6, 2016 · 11 comments
Milestone

Comments

@AsamK
Copy link

AsamK commented Mar 6, 2016

  1. What version of Go are you using (go version)?
    go1.6
  2. What operating system and processor architecture are you using (go env)?
    linux/amd64
  3. What did you do?
    Test returning of EOF and unexpected EOF
    https://play.golang.org/p/RJfp3HgcQ5
  4. What did you expect to see?
    The last read should return EOF
  5. What did you see instead?
    The last read returns UnexpectedEOF
    The issue was triggered by b179739 , which introduced unexpected EOF for truncated streams.

I'm not certain, if the io.Read() interface guarantees that calls after the first one that returned EOF should also return EOF, but returning "unexpected EOF" is definitely unexpected.

@AsamK
Copy link
Author

AsamK commented Mar 6, 2016

A simple fix would be to always store the error returned by z.decompressor.read:

diff --git a/src/compress/zlib/reader.go b/src/compress/zlib/reader.go
index 78ea704..640089f 100644
--- a/src/compress/zlib/reader.go
+++ b/src/compress/zlib/reader.go
@@ -94,8 +94,10 @@ func (z *reader) Read(p []byte) (n int, err error) {

    n, err = z.decompressor.Read(p)
    z.digest.Write(p[0:n])
-   if n != 0 || err != io.EOF {
+   if (err != nil) {
        z.err = err
+   }
+   if n != 0 || err != io.EOF {
        return
    }

@bradfitz
Copy link
Contributor

bradfitz commented Mar 6, 2016

I'm not certain, if the io.Read() interface guarantees that calls after the first one that returned EOF should also return EOF, but returning "unexpected EOF" is definitely unexpected.

The io.Reader interfaces guarantees nothing about any calls past the first error, but I suppose this is fixable, especially if behavior changes from Go 1.5 to Go 1.6.

@bradfitz
Copy link
Contributor

bradfitz commented Mar 6, 2016

bradfitz@laptop ~$ GOROOT=$HOME/go1.4 go run zread.go
go1.4.2
4 err: <nil>
0 err: EOF
0 err: EOF
bradfitz@laptop ~$ GOROOT=$HOME/go1.5 go run zread.go
go1.5.1
4 err: <nil>
0 err: EOF
0 err: EOF
bradfitz@laptop ~$ GOROOT=$HOME/go1.6 go run zread.go
go1.6
4 err: <nil>
0 err: EOF
0 err: unexpected EOF

@dsnet
Copy link
Member

dsnet commented Mar 6, 2016

I'll fix this. There's another unrelated bug here. The zlib package doesn't reset z.err when Reset is called.

Adding the following line to @AsamK's example still reports io.ErrUnexpectedEOF:

reader.(zlib.Resetter).Reset(bytes.NewReader(compress.Bytes()), nil)
fmt.Println(reader.Read(buf[:])) // 0 unexpected EOF

@gopherbot
Copy link

CL https://golang.org/cl/20292 mentions this issue.

@AsamK
Copy link
Author

AsamK commented Mar 7, 2016

Thanks for fixing this so quickly, will the fix be included in a 1.6.1 release?

@bradfitz
Copy link
Contributor

bradfitz commented Mar 7, 2016

No promises, but we'll consider it.

@bradfitz bradfitz changed the title compress/zlib.reader returns unexpected EOF when calling Read() after EOF compress/zlib: Reader returns unexpected EOF when calling Read after EOF Mar 7, 2016
@bradfitz bradfitz added this to the Go1.6.1 milestone Mar 7, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Mar 7, 2016

@rsc, @ianlancetaylor, @adg, this was a regression from Go 1.4 and Go 1.5 and presumably earlier. Maybe a candidate for Go 1.6.1.

Related is #14676

@rsc
Copy link
Contributor

rsc commented Mar 7, 2016

Not sure this is critical enough for 1.6.1. Is it breaking something important that can't be changed not to read multiple times at EOF or to use a fixed copy of the library?

@dsnet
Copy link
Member

dsnet commented Mar 7, 2016

Also, @bradfitz's fix in #14676 could easily be applied by the user as well.

@adg adg modified the milestones: Go1.6.1, Go1.6.2 Apr 7, 2016
@rsc
Copy link
Contributor

rsc commented Apr 7, 2016

This is unfortunate but a big change. The mime instance was the main trigger and we have a simpler mime fix for 1.6.2.

@rsc rsc modified the milestones: Go1.7, Go1.6.2 Apr 7, 2016
@golang golang locked and limited conversation to collaborators Apr 8, 2017
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

6 participants