x/image/riff: (*Reader) Next() drops io.ReadFull error

[richardlehane]

I'm getting panics and memory issues using golang.org/x/image/riff package with malformed riff files.

E.g. https://play.golang.org/p/cCN1VQ4Dz2

These seem to be due to an IO error being dropped in the (*Reader) Next() method.

From line 132 in https://github.com/golang/image/blob/master/riff/riff.go:

    if _, err = io.ReadFull(z.r, z.buf[:chunkHeaderSize]); err != nil {
        if z.err == io.EOF || z.err == io.ErrUnexpectedEOF {
            z.err = errShortChunkHeader
        }
        return FourCC{}, 0, nil, z.err
    }
    chunkID = FourCC{z.buf[0], z.buf[1], z.buf[2], z.buf[3]}
    z.chunkLen = u32(z.buf[4:])
    z.padded = z.chunkLen&1 == 1
    z.chunkReader = &chunkReader{z}
    return chunkID, z.chunkLen, z.chunkReader, nil

The err var in the first line of that snippet is a named return value but it is never used/ returned by the method.

Can fix by assigning the io.ReadFull error to z.err instead:

    if _, z.err = io.ReadFull(z.r, z.buf[:chunkHeaderSize]); z.err != nil {
        if z.err == io.EOF || z.err == io.ErrUnexpectedEOF {
            z.err = errShortChunkHeader
        }
        return FourCC{}, 0, nil, z.err
    }
    chunkID = FourCC{z.buf[0], z.buf[1], z.buf[2], z.buf[3]}
    z.chunkLen = u32(z.buf[4:])
    z.padded = z.chunkLen&1 == 1
    z.chunkReader = &chunkReader{z}
    return chunkID, z.chunkLen, z.chunkReader, nil

as at golang/image@ 8550bb5

[gopherbot]

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