Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

image/gif: EncodeAll panics when a palette is too large to encode #20249

Closed
topherbullock opened this issue May 4, 2017 · 5 comments
Closed

image/gif: EncodeAll panics when a palette is too large to encode #20249

topherbullock opened this issue May 4, 2017 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.10

Comments

@topherbullock
Copy link

topherbullock commented May 4, 2017 •
edited

What version of Go are you using (go version)?

1.8.1

What operating system and processor architecture are you using (go env)?

darwin / amd64

What did you do?

Created a gif image with a palette ( either global or local color table for a frame ) containing > 256 colors.

https://play.golang.org/p/xfDZCcSIFF

What did you expect to see?

The EncodeAll function returns errors when image blocks are too large, on a zero-length gif, etc. As such, in this case since we know the gif can't be encoded due to the palette length being > 256, I would expect a similar error, along the lines of errors.New("gif: image palette is too large to encode") to match the style here

What did you see instead?

It panics here trying to read log2Lookup[-1] defined here

I have a change-set ready with the proposed functionality and accompanying tests.
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label May 4, 2017
@bradfitz bradfitz added this to the Go1.10 milestone May 4, 2017
@bradfitz
Copy link
Contributor

bradfitz commented May 4, 2017

Go 1.9 is fine too if trivial/safe.

/cc @nigeltao

@gopherbot
Copy link

CL https://golang.org/cl/42790 mentions this issue.

@topherbullock
Copy link
Author

topherbullock commented May 5, 2017 •
edited

@bradfitz + @nigeltao Ah, okay.. Welp, that looks a lot like what I had ready to submit, so LGTM :D

@nigeltao
Copy link
Contributor

nigeltao commented May 6, 2017

Ah, I didn't read your final line that said you had a change-set ready. Sorry.

@topherbullock
Copy link
Author

@nigeltao No worries 😄

@golang golang locked and limited conversation to collaborators May 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
4 participants
@bradfitz @topherbullock @nigeltao @gopherbot