image/gif: fix frame-inside-image bounds checking.

The semantics of the Go image.Rectangle type is that the In and
Intersects methods treat empty rectangles specially. There are multiple
valid representations of an empty image.Rectangle. One of them is the
zero image.Rectangle but there are others. They're obviously not all
equal in the == sense, so we shouldn't use != to check GIF's semantics.

This change will allow us to re-roll
a855da29dbd7a80c4d87a421c1f88a8603c020fa "image: fix the overlap check
in Rectangle.Intersect" which was rolled back in
14347ee480968c712ea885a4ea62779fd8a0dc44.

Change-Id: Ie1a0d092510a7bb6170e61adbf334b21361ff9e6
Reviewed-on: https://go-review.googlesource.com/36639
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/image/gif/reader.go b/src/image/gif/reader.go
index e61112817b7c02209ad9bebf7b58bb27c9746861..2805fbad5b4d3c4db1b419e895d5f8503d699475 100644 (file)
--- a/src/image/gif/reader.go
+++ b/src/image/gif/reader.go
@@ -410,14 +410,29 @@ func (d *decoder) newImageFromDescriptor() (*image.Paletted, error) {
        height := int(d.tmp[6]) + int(d.tmp[7])<<8
        d.imageFields = d.tmp[8]
 
-       // The GIF89a spec, Section 20 (Image Descriptor) says:
-       // "Each image must fit within the boundaries of the Logical
-       // Screen, as defined in the Logical Screen Descriptor."
-       bounds := image.Rect(left, top, left+width, top+height)
-       if bounds != bounds.Intersect(image.Rect(0, 0, d.width, d.height)) {
+       // The GIF89a spec, Section 20 (Image Descriptor) says: "Each image must
+       // fit within the boundaries of the Logical Screen, as defined in the
+       // Logical Screen Descriptor."
+       //
+       // This is conceptually similar to testing
+       //      frameBounds := image.Rect(left, top, left+width, top+height)
+       //      imageBounds := image.Rect(0, 0, d.width, d.height)
+       //      if !frameBounds.In(imageBounds) { etc }
+       // but the semantics of the Go image.Rectangle type is that r.In(s) is true
+       // whenever r is an empty rectangle, even if r.Min.X > s.Max.X. Here, we
+       // want something stricter.
+       //
+       // Note that, by construction, left >= 0 && top >= 0, so we only have to
+       // explicitly compare frameBounds.Max (left+width, top+height) against
+       // imageBounds.Max (d.width, d.height) and not frameBounds.Min (left, top)
+       // against imageBounds.Min (0, 0).
+       if left+width > d.width || top+height > d.height {
                return nil, errors.New("gif: frame bounds larger than image bounds")
        }
-       return image.NewPaletted(bounds, nil), nil
+       return image.NewPaletted(image.Rectangle{
+               Min: image.Point{left, top},
+               Max: image.Point{left + width, top + height},
+       }, nil), nil
 }
 
 func (d *decoder) readBlock() (int, error) {