proposal: io: add NoEOF to convert EOF to ErrUnexpectedEOF

[dsnet]

Proposal Details

The EOF error is a common error that is reported in IO operations and is often exactly the right error at a finer granularity. However, when composed as part of a larger grammar or file format, many EOF errors should actually be converted into ErrUnexpectedEOF. However, it is often forgotten, leading to bugs where parsing a truncated file or input silently fails. In fact, there have been several bugs of this nature even within the Go standard library:

• image/jpeg: return io.ErrUnexpectedEOF on truncated data
• compress/gzip: return unexpected EOF for certain truncated streams
• archive/tar: fix and cleanup readOldGNUSparseMap
  
• image/gif: check handling of truncated GIF files

...and certainly many bugs outside of the standard library (hence the motivation for this helper).

I propose the addition of the following helper function:

func NoEOF(err error) error {
    if err == EOF {
        return ErrUnexpectedEOF
    }
    return err
}

This won't inherently prevent bugs of this nature, but at least provides a helper making it easier to provide the appropriate fix. In our own code, we have the equivalent function declared in multiple places.

This proposal does not use errors.Is based on the result of #39155.

[gabyhelp]

Related Issues

• proposal: change standard library to check for io.EOF using errors.Is #39155 (closed)
• io: ReadFull should not forward io.ErrUnexpectedEOF from underlying reader #47677
• proposal: errors: add Ignore #74264
• proposal: strconv: Teach UnquoteChar to distinguish unexpected EOF from syntax errors #19107 (closed)
• encoding/binary: Read(U)varint should return ErrUnexpectedEOF #33575 (closed)
• io: use errors.Is in reader and writer helpers #53086 (closed)

Related Code Changes

• image/gif: check handling of truncated GIF files
• mime/multipart: change %v to %w for EOF error
• io: use errors.Is in reader and writer helpers
• all: Add errors handlings via errors package

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[apparentlymart]

My read of #39155 is that the decision was exclusively for code directly interacting with the result from io.Reader.Read: that method is documented as returning exactly io.EOF, and so any caller that is calling an implementation of that method can assume that it will not be wrapped.

Applying that decision here seems tricky because NoEOF is not constrained to be used only with the result of io.Reader.Read, but perhaps that could be addressed just with documentation.

---

An alternative I considered while thinking about this was a function that wraps a call to io.Reader.Read:

package io

// ReadNoEOF calls Read on the given reader and returns exactly what it returns
// unless its error is exactly [EOF], in which case this function returns
// [ErrUnexpectedEOF] instead.
func ReadNoEOF(r io.Reader, p []byte) (int, error) {
    n, err := r.Read(p)
    if err == EOF {
        err = ErrUnexpectedEOF
    }
    return n, err
}

This does ensure that this helper can only apply when directly calling Read, but at a significant ergonomic cost. So I think that documenting that NoEOF is only applicable to the direct result of Read is probably good enough. I've shared the above only for the sake of comparison and am not actually proposing it.

[dsnet]

I'm not sure ReadNoEOF will be sufficient since many (if not most?) of the patterns that need to convert EOF to ErrUnexpectedEOF are not just from io.Reader.Read, but also other helper functionality in the io package itself.

[apparentlymart]

Hmm, indeed.

While most of those helpers seem pretty marginal in the context of the problems you discussed in this issue, io.ReadAtLeast is currently defined as returning io.EOF when there are no bytes at all to read and io.ErrUnexpectedEOF for all other EOF situations, and that seems like a likely situation where a caller would want to treat a zero-length read as "unexpected" instead, for the reasons you mentioned.

[earthboundkid]

The connection here to #74264 as a mirror is pretty clear, but since the proposed errors.Ignore uses errors.Is, should we also have io.IgnoreEOF(error) error that uses == instead?