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.

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: "not enough image data" on gif that works in browser #38958

Open
skyqinsc opened this issue May 8, 2020 · 6 comments
Open

image/gif: "not enough image data" on gif that works in browser #38958

skyqinsc opened this issue May 8, 2020 · 6 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@skyqinsc
Copy link

skyqinsc commented May 8, 2020 •
edited

There are images which browsers can render but which image/gif chokes on.

The following runs successfully for most gifs:

import (
    "fmt"
    "image/gif"
    "os"
)

const example = "sample.gif"

func main() {
    fmt.Println("attempting to read gif")

    file, err := os.Open(example)
    if err != nil {
        fmt.Println("Can't open this file:", err)
        return
    }

    img, err := gif.Decode(file)
    if err != nil {
        fmt.Println("Can't decode this as a gif:", err)
        return
    }

    _ = img.Bounds()

    fmt.Println("gif decoded")
}

But when trying to run it on a particular gif (below), this happens:

$ go run main.go

attempting to read gif
Can't decode this as a gif: gif: not enough image data

Here’s the offending gif
girl
@ALTree
Copy link
Member

ALTree commented May 8, 2020 •
edited

While my browser does indeed display it, that image is definitely corrupted and/or badly formed; imagemagick says:

$ curl -s -o img.gif https://user-images.githubusercontent.com/12746710/81430635-5412ea80-9192-11ea-9a62-80208204bf45.gif
$ identify -verbose img.gif 
identify-im6.q16: corrupt image `img.gif' @ error/gif.c/DecodeImage/513.
identify-im6.q16: corrupt image `img.gif' @ error/gif.c/ReadGIFImage/1384.

and my system's image viewer also refuses to open it.

This is similar to #9856, when image/gif was fixed to handle bad gifs a little better.

cc @nigeltao

@ALTree ALTree changed the title There are images which browsers can render but which image/gif chokes on image/gif: "not enough image data" on gif that works in browser May 8, 2020
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 8, 2020
as added a commit to as/goissues that referenced this issue May 8, 2020
@as
information about issue golang/go#38958
30f23c6
@as
Copy link
Contributor

as commented May 8, 2020

To read this image, there need to be two changes to the gif decoder. I have made these changes in a modified reader.go (see below), which fix the problem, but the solution needs a more-refined implementation that I don't have time to implement.

(1) Allow the lzw reader to encounter short reads in the stream.

go/src/image/gif/reader.go

Line 427 in 4f65fb3

return errNotEnough

Example:
https://github.com/as/goissues/blob/173a03c197057be2f0fd7a842ddb4542e7b2e1e3/38958/image_gif_reader.go.txt#L430

(2) Handle zero-bytes sandwiched inside parts of the image.

go/src/image/gif/reader.go

Lines 243 to 258 in 4f65fb3

case sExtension:
if err = d.readExtension(); err != nil {
return err
}
case sImageDescriptor:
if err = d.readImageDescriptor(keepAllFrames); err != nil {
return err
}
case sTrailer:
if len(d.image) == 0 {
return fmt.Errorf("gif: missing image data")
}
return nil

Example:
https://github.com/as/goissues/blob/173a03c197057be2f0fd7a842ddb4542e7b2e1e3/38958/image_gif_reader.go.txt#L252-L255

After this, the author's test program correctly decodes the image.

cp /go/src/image/gif/reader.go /go/src/image/gif/reader.go.old
curl https://raw.githubusercontent.com/as/goissues/master/38958/image_gif_reader.go.txt > /go/src/image/gif/reader.go

@nigeltao
Copy link
Contributor

nigeltao commented May 8, 2020

Handle zero-bytes sandwiched inside parts of the image.

Coincidentally,
https://go-review.googlesource.com/c/go/+/232057
is in review. Does patching in that change fix it?

@as
Copy link
Contributor

as commented May 8, 2020

@nigeltao That patch is necessary to fix it, but alone isn't sufficient. The LZW reader case (1) also needs to be handled.

@skyqinsc
Copy link
Author

skyqinsc commented May 9, 2020 •
edited

This is another sample which browsers also can render but which image/gif chokes on, but something is different.
$ go run main.go

attempting to read gif
Can't decode this as a gif: gif: reading image data: lzw: invalid code

car
@as @nigeltao

@as
Copy link
Contributor

as commented May 9, 2020 •
edited

The above image is even-more problematic. It contains a byte interpreted as an invalid LZW code (0x66) instead of (0x00) like the first image. To fix that, I had to elide almost all error checks and then ignore the 0x66 byte.

Perhaps other decoders silently ignore garbage input entirely and march forward hoping to eventually draw a valid-looking image. Which encoder produced these?

The image contains this metadata (which may give us a clue above the question above):

NETSCAPE2.0�����!��XMP DataXMP<?xpacket begin="" id="W5M0MpCehiHzreSzNTczkc9d"?> <x:xmpmeta xmlns:x="adobe:ns:meta/" x:xmptk="Adobe XMP Core 5.6-c142 79.160924, 2017/07/13-01:06:39        "> <rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"> <rdf:Description rdf:about="" xmlns:xmpMM="http://ns.adobe.com/xap/1.0/mm/" xmlns:stRef="http://ns.adobe.com/xap/1.0/sType/ResourceRef#" xmlns:xmp="http://ns.adobe.com/xap/1.0/" xmpMM:OriginalDocumentID="xmp.did:7fc53bc1-ea46-e144-975e-5ee1c3561563" xmpMM:DocumentID="xmp.did:8EB38125D86411E9B26CC13B008A9B86" xmpMM:InstanceID="xmp.iid:8EB38124D86411E9B26CC13B008A9B86" xmp:CreatorTool="Adobe Photoshop CC 2018 (Windows)"> <xmpMM:DerivedFrom stRef:instanceID="xmp.iid:60b609bd-629e-ce49-ade4-1fc8e36fac43" stRef:documentID="adobe:docid:photoshop:bea01de1-6c51-3446-9bed-3ffc92398997"/> </rdf:Description> </rdf:RDF> </x:xmpmeta> <?xpacket end="r"?>

@seankhliao seankhliao added this to the Unplanned milestone Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@ALTree @as @nigeltao @seankhliao @skyqinsc