image/gif: check handling of truncated GIF files

All the prefixes of the testGIF produce errors today,
but they differ wildly in which errors: some are io.EOF,
others are io.ErrUnexpectedEOF, and others are gif-specific.
Make them all gif-specific to explain context, and make
any complaining about EOF be sure to mention the EOF
is unexpected.

Fixes #11390.

Change-Id: I742c39c88591649276268327ea314e68d1de1845
Reviewed-on: https://go-review.googlesource.com/17493
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/image/gif/reader.go b/src/image/gif/reader.go
index 6181a946fad15b5ff651e19d1184cf25114a615e..e61112817b7c02209ad9bebf7b58bb27c9746861 100644 (file)
--- a/src/image/gif/reader.go
+++ b/src/image/gif/reader.go
@@ -63,6 +63,22 @@ const (
        eApplication    = 0xFF // Application
 )
 
+func readFull(r io.Reader, b []byte) error {
+       _, err := io.ReadFull(r, b)
+       if err == io.EOF {
+               err = io.ErrUnexpectedEOF
+       }
+       return err
+}
+
+func readByte(r io.ByteReader) (byte, error) {
+       b, err := r.ReadByte()
+       if err == io.EOF {
+               err = io.ErrUnexpectedEOF
+       }
+       return b, err
+}
+
 // decoder is the type used to decode a GIF file.
 type decoder struct {
        r reader
@@ -124,7 +140,7 @@ func (b *blockReader) Read(p []byte) (int, error) {
                        return 0, b.err
                }
                b.slice = b.tmp[:blockLen]
-               if _, b.err = io.ReadFull(b.r, b.slice); b.err != nil {
+               if b.err = readFull(b.r, b.slice); b.err != nil {
                        return 0, b.err
                }
        }
@@ -151,9 +167,9 @@ func (d *decoder) decode(r io.Reader, configOnly bool) error {
        }
 
        for {
-               c, err := d.r.ReadByte()
+               c, err := readByte(d.r)
                if err != nil {
-                       return err
+                       return fmt.Errorf("gif: reading frames: %v", err)
                }
                switch c {
                case sExtension:
@@ -198,9 +214,9 @@ func (d *decoder) decode(r io.Reader, configOnly bool) error {
                                        m.Palette = p
                                }
                        }
-                       litWidth, err := d.r.ReadByte()
+                       litWidth, err := readByte(d.r)
                        if err != nil {
-                               return err
+                               return fmt.Errorf("gif: reading image data: %v", err)
                        }
                        if litWidth < 2 || litWidth > 8 {
                                return fmt.Errorf("gif: pixel size in decode out of range: %d", litWidth)
@@ -209,9 +225,9 @@ func (d *decoder) decode(r io.Reader, configOnly bool) error {
                        br := &blockReader{r: d.r}
                        lzwr := lzw.NewReader(br, lzw.LSB, int(litWidth))
                        defer lzwr.Close()
-                       if _, err = io.ReadFull(lzwr, m.Pix); err != nil {
+                       if err = readFull(lzwr, m.Pix); err != nil {
                                if err != io.ErrUnexpectedEOF {
-                                       return err
+                                       return fmt.Errorf("gif: reading image data: %v", err)
                                }
                                return errNotEnough
                        }
@@ -228,13 +244,13 @@ func (d *decoder) decode(r io.Reader, configOnly bool) error {
                        // See https://golang.org/issue/9856 for an example GIF.
                        if n, err := lzwr.Read(d.tmp[:1]); n != 0 || (err != io.EOF && err != io.ErrUnexpectedEOF) {
                                if err != nil {
-                                       return err
+                                       return fmt.Errorf("gif: reading image data: %v", err)
                                }
                                return errTooMuch
                        }
                        if n, err := br.Read(d.tmp[:1]); n != 0 || err != io.EOF {
                                if err != nil {
-                                       return err
+                                       return fmt.Errorf("gif: reading image data: %v", err)
                                }
                                return errTooMuch
                        }
@@ -264,7 +280,7 @@ func (d *decoder) decode(r io.Reader, configOnly bool) error {
 
                case sTrailer:
                        if len(d.image) == 0 {
-                               return io.ErrUnexpectedEOF
+                               return fmt.Errorf("gif: missing image data")
                        }
                        return nil
 
@@ -275,13 +291,13 @@ func (d *decoder) decode(r io.Reader, configOnly bool) error {
 }
 
 func (d *decoder) readHeaderAndScreenDescriptor() error {
-       _, err := io.ReadFull(d.r, d.tmp[:13])
+       err := readFull(d.r, d.tmp[:13])
        if err != nil {
-               return err
+               return fmt.Errorf("gif: reading header: %v", err)
        }
        d.vers = string(d.tmp[:6])
        if d.vers != "GIF87a" && d.vers != "GIF89a" {
-               return fmt.Errorf("gif: can't recognize format %s", d.vers)
+               return fmt.Errorf("gif: can't recognize format %q", d.vers)
        }
        d.width = int(d.tmp[6]) + int(d.tmp[7])<<8
        d.height = int(d.tmp[8]) + int(d.tmp[9])<<8
@@ -298,9 +314,9 @@ func (d *decoder) readHeaderAndScreenDescriptor() error {
 
 func (d *decoder) readColorTable(fields byte) (color.Palette, error) {
        n := 1 << (1 + uint(fields&fColorTableBitsMask))
-       _, err := io.ReadFull(d.r, d.tmp[:3*n])
+       err := readFull(d.r, d.tmp[:3*n])
        if err != nil {
-               return nil, fmt.Errorf("gif: short read on color table: %s", err)
+               return nil, fmt.Errorf("gif: reading color table: %s", err)
        }
        j, p := 0, make(color.Palette, n)
        for i := range p {
@@ -311,9 +327,9 @@ func (d *decoder) readColorTable(fields byte) (color.Palette, error) {
 }
 
 func (d *decoder) readExtension() error {
-       extension, err := d.r.ReadByte()
+       extension, err := readByte(d.r)
        if err != nil {
-               return err
+               return fmt.Errorf("gif: reading extension: %v", err)
        }
        size := 0
        switch extension {
@@ -324,9 +340,9 @@ func (d *decoder) readExtension() error {
        case eComment:
                // nothing to do but read the data.
        case eApplication:
-               b, err := d.r.ReadByte()
+               b, err := readByte(d.r)
                if err != nil {
-                       return err
+                       return fmt.Errorf("gif: reading extension: %v", err)
                }
                // The spec requires size be 11, but Adobe sometimes uses 10.
                size = int(b)
@@ -334,8 +350,8 @@ func (d *decoder) readExtension() error {
                return fmt.Errorf("gif: unknown extension 0x%.2x", extension)
        }
        if size > 0 {
-               if _, err := io.ReadFull(d.r, d.tmp[:size]); err != nil {
-                       return err
+               if err := readFull(d.r, d.tmp[:size]); err != nil {
+                       return fmt.Errorf("gif: reading extension: %v", err)
                }
        }
 
@@ -343,8 +359,11 @@ func (d *decoder) readExtension() error {
        // this extension defines a loop count.
        if extension == eApplication && string(d.tmp[:size]) == "NETSCAPE2.0" {
                n, err := d.readBlock()
-               if n == 0 || err != nil {
-                       return err
+               if err != nil {
+                       return fmt.Errorf("gif: reading extension: %v", err)
+               }
+               if n == 0 {
+                       return nil
                }
                if n == 3 && d.tmp[0] == 1 {
                        d.loopCount = int(d.tmp[1]) | int(d.tmp[2])<<8
@@ -352,14 +371,17 @@ func (d *decoder) readExtension() error {
        }
        for {
                n, err := d.readBlock()
-               if n == 0 || err != nil {
-                       return err
+               if err != nil {
+                       return fmt.Errorf("gif: reading extension: %v", err)
+               }
+               if n == 0 {
+                       return nil
                }
        }
 }
 
 func (d *decoder) readGraphicControl() error {
-       if _, err := io.ReadFull(d.r, d.tmp[:6]); err != nil {
+       if err := readFull(d.r, d.tmp[:6]); err != nil {
                return fmt.Errorf("gif: can't read graphic control: %s", err)
        }
        if d.tmp[0] != 4 {
@@ -379,7 +401,7 @@ func (d *decoder) readGraphicControl() error {
 }
 
 func (d *decoder) newImageFromDescriptor() (*image.Paletted, error) {
-       if _, err := io.ReadFull(d.r, d.tmp[:9]); err != nil {
+       if err := readFull(d.r, d.tmp[:9]); err != nil {
                return nil, fmt.Errorf("gif: can't read image descriptor: %s", err)
        }
        left := int(d.tmp[0]) + int(d.tmp[1])<<8
@@ -399,11 +421,14 @@ func (d *decoder) newImageFromDescriptor() (*image.Paletted, error) {
 }
 
 func (d *decoder) readBlock() (int, error) {
-       n, err := d.r.ReadByte()
+       n, err := readByte(d.r)
        if n == 0 || err != nil {
                return 0, err
        }
-       return io.ReadFull(d.r, d.tmp[:n])
+       if err := readFull(d.r, d.tmp[:n]); err != nil {
+               return 0, err
+       }
+       return int(n), nil
 }
 
 // interlaceScan defines the ordering for a pass of the interlace algorithm.

diff --git a/src/image/gif/reader_test.go b/src/image/gif/reader_test.go
index 90c81493cba5442a792932facf0bd9d1e25a40c8..1267ba06a9d20e011ae663fe025f546c0ab17a73 100644 (file)
--- a/src/image/gif/reader_test.go
+++ b/src/image/gif/reader_test.go
@@ -10,6 +10,7 @@ import (
        "image"
        "image/color"
        "reflect"
+       "strings"
        "testing"
 )
 
@@ -292,3 +293,19 @@ func TestLoopCount(t *testing.T) {
                t.Errorf("loop count mismatch: %d vs %d", img.LoopCount, img1.LoopCount)
        }
 }
+
+func TestUnexpectedEOF(t *testing.T) {
+       for i := len(testGIF) - 1; i >= 0; i-- {
+               _, err := Decode(bytes.NewReader(testGIF[:i]))
+               if err == errNotEnough {
+                       continue
+               }
+               text := ""
+               if err != nil {
+                       text = err.Error()
+               }
+               if !strings.HasPrefix(text, "gif:") || !strings.HasSuffix(text, ": unexpected EOF") {
+                       t.Errorf("Decode(testGIF[:%d]) = %v, want gif: ...: unexpected EOF", i, err)
+               }
+       }
+}