diff --git a/src/image/gif/reader.go b/src/image/gif/reader.go index 2805fbad5b4..b1335e61259 100644 --- a/src/image/gif/reader.go +++ b/src/image/gif/reader.go @@ -231,8 +231,8 @@ func (d *decoder) decode(r io.Reader, configOnly bool) error { } return errNotEnough } - // Both lzwr and br should be exhausted. Reading from them should - // yield (0, io.EOF). + // In theory, both lzwr and br should be exhausted. Reading from them + // should yield (0, io.EOF). // // The spec (Appendix F - Compression), says that "An End of // Information code... must be the last code output by the encoder @@ -248,11 +248,21 @@ func (d *decoder) decode(r io.Reader, configOnly bool) error { } return errTooMuch } - if n, err := br.Read(d.tmp[:1]); n != 0 || err != io.EOF { + + // In practice, some GIFs have an extra byte in the data sub-block + // stream, which we ignore. See https://golang.org/issue/16146. + for nExtraBytes := 0; ; { + n, err := br.Read(d.tmp[:2]) + nExtraBytes += n + if nExtraBytes > 1 { + return errTooMuch + } + if err == io.EOF { + break + } if err != nil { return fmt.Errorf("gif: reading image data: %v", err) } - return errTooMuch } // Check that the color indexes are inside the palette. diff --git a/src/image/gif/reader_test.go b/src/image/gif/reader_test.go index 1267ba06a9d..51c64b7328f 100644 --- a/src/image/gif/reader_test.go +++ b/src/image/gif/reader_test.go @@ -37,16 +37,35 @@ func lzwEncode(in []byte) []byte { } func TestDecode(t *testing.T) { + // extra contains superfluous bytes to inject into the GIF, either at the end + // of an existing data sub-block (past the LZW End of Information code) or in + // a separate data sub-block. The 0x02 values are arbitrary. + const extra = "\x02\x02\x02\x02" + testCases := []struct { - nPix int // The number of pixels in the image data. - extra bool // Whether to write an extra block after the LZW-encoded data. - wantErr error + nPix int // The number of pixels in the image data. + // If non-zero, write this many extra bytes inside the data sub-block + // containing the LZW end code. + extraExisting int + // If non-zero, write an extra block of this many bytes. + extraSeparate int + wantErr error }{ - {0, false, errNotEnough}, - {1, false, errNotEnough}, - {2, false, nil}, - {2, true, errTooMuch}, - {3, false, errTooMuch}, + {0, 0, 0, errNotEnough}, + {1, 0, 0, errNotEnough}, + {2, 0, 0, nil}, + // An extra data sub-block after the compressed section with 1 byte which we + // silently skip. + {2, 0, 1, nil}, + // An extra data sub-block after the compressed section with 2 bytes. In + // this case we complain that there is too much data. + {2, 0, 2, errTooMuch}, + // Too much pixel data. + {3, 0, 0, errTooMuch}, + // An extra byte after LZW data, but inside the same data sub-block. + {2, 1, 0, nil}, + // Two extra bytes after LZW data, but inside the same data sub-block. + {2, 2, 0, nil}, } for _, tc := range testCases { b := &bytes.Buffer{} @@ -59,22 +78,35 @@ func TestDecode(t *testing.T) { b.WriteString("\x2c\x00\x00\x00\x00\x02\x00\x01\x00\x00\x02") if tc.nPix > 0 { enc := lzwEncode(make([]byte, tc.nPix)) - if len(enc) > 0xff { - t.Errorf("nPix=%d, extra=%t: compressed length %d is too large", tc.nPix, tc.extra, len(enc)) + if len(enc)+tc.extraExisting > 0xff { + t.Errorf("nPix=%d, extraExisting=%d, extraSeparate=%d: compressed length %d is too large", + tc.nPix, tc.extraExisting, tc.extraSeparate, len(enc)) continue } - b.WriteByte(byte(len(enc))) + + // Write the size of the data sub-block containing the LZW data. + b.WriteByte(byte(len(enc) + tc.extraExisting)) + + // Write the LZW data. b.Write(enc) + + // Write extra bytes inside the same data sub-block where LZW data + // ended. Each arbitrarily 0x02. + b.WriteString(extra[:tc.extraExisting]) } - if tc.extra { - b.WriteString("\x01\x02") // A 1-byte payload with an 0x02 byte. + + if tc.extraSeparate > 0 { + // Data sub-block size. This indicates how many extra bytes follow. + b.WriteByte(byte(tc.extraSeparate)) + b.WriteString(extra[:tc.extraSeparate]) } b.WriteByte(0x00) // An empty block signifies the end of the image data. b.WriteString(trailerStr) got, err := Decode(b) if err != tc.wantErr { - t.Errorf("nPix=%d, extra=%t\ngot %v\nwant %v", tc.nPix, tc.extra, err, tc.wantErr) + t.Errorf("nPix=%d, extraExisting=%d, extraSeparate=%d\ngot %v\nwant %v", + tc.nPix, tc.extraExisting, tc.extraSeparate, err, tc.wantErr) } if tc.wantErr != nil { @@ -90,7 +122,8 @@ func TestDecode(t *testing.T) { }, } if !reflect.DeepEqual(got, want) { - t.Errorf("nPix=%d, extra=%t\ngot %v\nwant %v", tc.nPix, tc.extra, got, want) + t.Errorf("nPix=%d, extraExisting=%d, extraSeparate=%d\ngot %v\nwant %v", + tc.nPix, tc.extraExisting, tc.extraSeparate, got, want) } } }