image/gif: panics for several GIFs #5401
Labels
Comments
These are actually two different issues: A) crash1.gif has an no palette information and thus the decoded image has a zero length Palette and image.Palletted.At returns a nil color.Color in this case. See http://tip.golang.org/src/pkg/image/image.go?s=22798:22839#L814 The final panic happens in http://tip.golang.org/src/pkg/image/draw/draw.go?s=14011:14050#L473 which does not check on this nil value. image.Paletted is the only image type which returns a nil color.Color in a call to At(x,y). Possible fixes: Either check for nil Color during draw or have image.Paletted.At return a color.RGBA{} default value like in the case of (x,y) not in the image bounds. B) chrash2.gif has a palette with 128 entries but pixel (0,35) references color no 255 and thus panics in http://tip.golang.org/src/pkg/image/image.go?s=22929:22948#L814 It could be argued, that the gif is malformed and decoding should fail. It seems that other tools (checked GIMP only) handle both cases gracefully by substituting black in both cases above. |
|
Labels changed: added priority-soon, removed priority-triage. Owner changed to @nigeltao. Status changed to Accepted. |
I'd be willing to work on this (especially with the great diagnosis from dr.volker, thanks!) The last time we were looking at fixing a problem like this (issue #3565) I decided it was better to not fix the bug and continue rejecting the GIF because the breakage was in its LZW encoding. This time, it seems reasonable to accept these images because while they are clearly invalid, the fix is within image/gif/*.go and the behavior of adding a black pixel in is not very controversial. Nigel, please give a "pre-LGTM" before I start on this. :) |
I'd prefer to reject invalid images (as an error) instead of implying black pixels. I think both cases should be fixed in image/gif instead of image or image/draw. Specifically, gif.DecodeXxx should return an error where it currently allows returning an image.Paletted with (A) an empty Palette or (B) a Pix element >= len(Palette). The second check (B) can be elided if len(Palette) == 255. The spec (http://www.w3.org/Graphics/GIF/spec-gif89a.txt) section 22 "Table Based Image Data" says that "Each index must be within the range of the size of the active color table, starting at 0." |
|
This issue was closed by revision 8192017. Status changed to Fixed. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
I've observed several panics in image/draw when working with real-world GIFs that work with other libraries (e.g., they display in Chrome.) Attached are two representative images that induce different failures and a program to repro. I've tested with two versions: Version: devel +62bf913b4f40 Wed Feb 27 20:55:01 2013 -0800 Version: devel +1d079908dd84 Thu Apr 25 18:47:12 2013 +0200 Stack traces are below. Please let me know if there's anything else you need. Thanks! From 62bf913b4f40 -- For crash1.gif -- $ go run imagecrash.go crash1.gif Version: devel +62bf913b4f40 Wed Feb 27 20:55:01 2013 -0800 panic: runtime error: invalid memory address or nil pointer dereference [signal 0xb code=0x1 addr=0x20 pc=0x44786f] goroutine 1 [running]: image/draw.drawRGBA(0x4c20006f0c0, 0x0, 0x0, 0x1, 0x1, ...) /usr/lib/google-golang/src/pkg/image/draw/draw.go:473 +0x42f image/draw.DrawMask(0x4c20006f100, 0x4c20006f0c0, 0x0, 0x0, 0x1, ...) /usr/lib/google-golang/src/pkg/image/draw/draw.go:114 +0x35d image/draw.Draw(0x4c20006f100, 0x4c20006f0c0, 0x0, 0x0, 0x1, ...) /usr/lib/google-golang/src/pkg/image/draw/draw.go:37 +0xbb main.main() /usr/local/google/home/piatek/speed/piatek-pss-git2/google3/experimental/users/piatek/imagecrash.go:30 +0x447 exit status 2 For crash2.gif -- $ go run imagecrash.go crash2.gif Version: devel +62bf913b4f40 Wed Feb 27 20:55:01 2013 -0800 panic: runtime error: index out of range goroutine 1 [running]: image.(*Paletted).At(0x4c2000700c0, 0x0, 0x23, 0x4c200059480, 0xfffbfbfb, ...) /usr/lib/google-golang/src/pkg/image/image.go:822 +0x15e image/draw.drawRGBA(0x4c20006f0c0, 0x0, 0x0, 0x12c, 0x24, ...) /usr/lib/google-golang/src/pkg/image/draw/draw.go:473 +0x421 image/draw.DrawMask(0x4c20006f100, 0x4c20006f0c0, 0x0, 0x0, 0x12c, ...) /usr/lib/google-golang/src/pkg/image/draw/draw.go:114 +0x35d image/draw.Draw(0x4c20006f100, 0x4c20006f0c0, 0x0, 0x0, 0x12c, ...) /usr/lib/google-golang/src/pkg/image/draw/draw.go:37 +0xbb main.main() /usr/local/google/home/piatek/speed/piatek-pss-git2/google3/experimental/users/piatek/imagecrash.go:30 +0x447 exit status 2Attachments:
The text was updated successfully, but these errors were encountered: