io: Copy implicilty requires WriterTo and ReaderFrom to not return io.EOF

[cyphar]

What version of Go are you using (go version)?

$ go version
go version go1.14.15 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/cyphar/.cache/go-build"
GOENV="/home/cyphar/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/cyphar/.local"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib64/go/1.14"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib64/go/1.14/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build440126677=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/6dFx2mL7f5r

package main

import (
	"fmt"
	"io"
	"io/ioutil"
)

type dummyReader struct {}

func (r dummyReader) Read(p []byte) (n int, err error) {
	return 0, io.EOF
}

type dummyWriteTo struct {
	dummyReader
}

func (r dummyWriteTo) WriteTo(w io.Writer) (n int64, err error) {
	return 0, io.EOF
}

func main() {
	n, err := io.Copy(ioutil.Discard, dummyReader{})
	fmt.Printf("io.Copy using io.Reader -- n=%v, err=%v\n", n, err)
	
	n, err = io.Copy(ioutil.Discard, dummyWriteTo{})
	fmt.Printf("io.Copy using io.WriterTo -- n=%v, err=%v\n", n, err)
}

The primary issue is that if you have an io.Reader which implements WriteTo such that it returns io.EOF at the end of the stream, io.Copy will blindly forward that error. klauspost/pgzip#38 is an example of a fairly popular project which made this mistake, and I hit this in opencontainers/umoci#360.

The same thing happens if you return io.EOF from ReadFrom (which is a little bit weirder to do, but if you don't use io.Copy in the implementation of io.ReaderFrom you might do this by accident).

It seems this is either:

• A documentation issue, since as far as I can tell, the io.WriterTo and io.ReaderFrom documentation doesn't mention that you shouldn't return io.EOF from those methods. The wording does kind of imply that you shouldn't return io.EOF but IMHO this should be much more explicit because any WriteTo implementation that doesn't use io.Copy internally will have to handle this explicitly and should be warned that this is the case.
• A bug in io.Copy (and possibly related stdlib users of io.WriterTo and io.ReaderFrom), because io.EOF is handled when it is returned by Read.

What did you expect to see?

io.Copy using io.Reader -- n=0, err=<nil>
io.Copy using io.WriterTo -- n=0, err=<nil>

or documentation which mentions that you shouldn't return io.EOF from WriteTo (or ReadFrom).

What did you see instead?

io.Copy using io.Reader -- n=0, err=<nil>
io.Copy using io.WriterTo -- n=0, err=EOF

[ianlancetaylor]

I think this is a documentation issue. I can't see any reason why WriteTo or ReadFrom should return io.EOF.

[dreamerjackson]

@ianlancetaylor i make a PR, can you help view it ? thanks!

[gopherbot]

Change https://golang.org/cl/294709 mentions this issue: fix: io/copy document. fix #44411

[cyphar]

I can't see any reason why WriteTo or ReadFrom should return io.EOF.

You could do it by accident if you don't use io.Copy internally and instead use some other I/O methods, but if the semantics are basically "more efficient io.Copy" then I agree it should never return io.EOF. Thanks.

[gopherbot]

Change https://golang.org/cl/294769 mentions this issue: io: unify EOF documentation for ReadFrom and WriteTo