image/gif: check that individual frame's bounds are within the overall
GIF's bounds.
Also change the implicit Config Width and Height to be the
Rectangle.Max, not the Dx and Dy, of the first frame's bounds. For the
case where the first frame's bounds is something like (5,5)-(8,8), the
overall width should be 8, not 3.
Change-Id: I3affc484f5e32941a36f15517a92ca8d189d9c22
Reviewed-on: https://go-review.googlesource.com/9465
Reviewed-by: Rob Pike <r@golang.org>
diff --git a/src/image/gif/reader.go b/src/image/gif/reader.go
index 07adeb3..bd452eb 100644
--- a/src/image/gif/reader.go
+++ b/src/image/gif/reader.go
@@ -430,10 +430,13 @@
Disposal []byte
// Config is the global color map (palette), width and height. A nil or
// empty-color.Palette Config.ColorModel means that each frame has its own
- // color map and there is no global color map. For backwards compatibility,
- // a zero-valued Config is valid to pass to EncodeAll, and implies that the
- // overall GIF's width and height equals the first frame's width and
- // height.
+ // color map and there is no global color map. Each frame's bounds must be
+ // within the rectangle defined by the two points (0, 0) and (Config.Width,
+ // Config.Height).
+ //
+ // For backwards compatibility, a zero-valued Config is valid to pass to
+ // EncodeAll, and implies that the overall GIF's width and height equals
+ // the first frame's bounds' Rectangle.Max point.
Config image.Config
// BackgroundIndex is the background index in the global color map, for use
// with the DisposalBackground disposal method.
diff --git a/src/image/gif/writer.go b/src/image/gif/writer.go
index a70fc40..322b353 100644
--- a/src/image/gif/writer.go
+++ b/src/image/gif/writer.go
@@ -191,6 +191,10 @@
e.err = errors.New("gif: image block is too large to encode")
return
}
+ if !b.In(image.Rectangle{Max: image.Point{e.g.Config.Width, e.g.Config.Height}}) {
+ e.err = errors.New("gif: image block is out of bounds")
+ return
+ }
transparentIndex := -1
for i, c := range pm.Palette {
@@ -297,9 +301,9 @@
return errors.New("gif: mismatched image and disposal lengths")
}
if e.g.Config == (image.Config{}) {
- b := g.Image[0].Bounds()
- e.g.Config.Width = b.Dx()
- e.g.Config.Height = b.Dy()
+ p := g.Image[0].Bounds().Max
+ e.g.Config.Width = p.X
+ e.g.Config.Height = p.Y
} else if e.g.Config.ColorModel != nil {
if _, ok := e.g.Config.ColorModel.(color.Palette); !ok {
return errors.New("gif: GIF color model must be a color.Palette")
diff --git a/src/image/gif/writer_test.go b/src/image/gif/writer_test.go
index 2248ac3..d661015 100644
--- a/src/image/gif/writer_test.go
+++ b/src/image/gif/writer_test.go
@@ -297,8 +297,66 @@
}
}
-// TODO: add test for when individual frames are out of the global bounds.
-// TODO: add test for when the first frame's bounds are not the same as the global bounds.
+func TestEncodeFrameOutOfBounds(t *testing.T) {
+ images := []*image.Paletted{
+ image.NewPaletted(image.Rect(0, 0, 5, 5), palette.Plan9),
+ image.NewPaletted(image.Rect(2, 2, 8, 8), palette.Plan9),
+ image.NewPaletted(image.Rect(3, 3, 4, 4), palette.Plan9),
+ }
+ for _, upperBound := range []int{6, 10} {
+ g := &GIF{
+ Image: images,
+ Delay: make([]int, len(images)),
+ Disposal: make([]byte, len(images)),
+ Config: image.Config{
+ Width: upperBound,
+ Height: upperBound,
+ },
+ }
+ err := EncodeAll(ioutil.Discard, g)
+ if upperBound >= 8 {
+ if err != nil {
+ t.Errorf("upperBound=%d: %v", upperBound, err)
+ }
+ } else {
+ if err == nil {
+ t.Errorf("upperBound=%d: got nil error, want non-nil", upperBound)
+ }
+ }
+ }
+}
+
+func TestEncodeImplicitConfigSize(t *testing.T) {
+ // For backwards compatibility for Go 1.4 and earlier code, the Config
+ // field is optional, and if zero, the width and height is implied by the
+ // first (and in this case only) frame's width and height.
+ //
+ // A Config only specifies a width and height (two integers) while an
+ // image.Image's Bounds method returns an image.Rectangle (four integers).
+ // For a gif.GIF, the overall bounds' top-left point is always implicitly
+ // (0, 0), and any frame whose bounds have a negative X or Y will be
+ // outside those overall bounds, so encoding should fail.
+ for _, lowerBound := range []int{-1, 0, 1} {
+ images := []*image.Paletted{
+ image.NewPaletted(image.Rect(lowerBound, lowerBound, 4, 4), palette.Plan9),
+ }
+ g := &GIF{
+ Image: images,
+ Delay: make([]int, len(images)),
+ }
+ err := EncodeAll(ioutil.Discard, g)
+ if lowerBound >= 0 {
+ if err != nil {
+ t.Errorf("lowerBound=%d: %v", lowerBound, err)
+ }
+ } else {
+ if err == nil {
+ t.Errorf("lowerBound=%d: got nil error, want non-nil", lowerBound)
+ }
+ }
+ }
+}
+
// TODO: add test for when a frame has the same color map (palette) as the global one.
func BenchmarkEncode(b *testing.B) {
