image/gif: decoding gif returns `unknown block type: 0x01` error

[montanaflynn]

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8.3 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/montanaflynn/Development/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/w7/gd1mgc9n05q_9hrtt2sd75dw0000gn/T/go-build873139005=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

Tried to decode this gif:

package main

import (
    "flag"
    "fmt"
    "image/gif"
    "net/http"
    "os"
)

func exitIfError(err error) {
    if err == nil {
        return
    }

    fmt.Fprintf(os.Stderr, "%v\n", err)
    os.Exit(1)
}

func main() {
    gifURL := flag.String("url", "http://i.imgur.com/cjbY0nE.gif", "the URL of the GIF")
    flag.Parse()
    res, err := http.Get(*gifURL)
    exitIfError(err)
    _, err = gif.DecodeAll(res.Body)
    _ = res.Body.Close()
    exitIfError(err)
}

What did you expect to see?

The gif decode not to error

What did you see instead?

gif: unknown block type: 0x01

[bradfitz]

I can repro at tip (0d33a896d9eea), ~Go 1.9beta2, and at Go 1.7, Go 1.6, Go 1.5, and Go 1.4.

So not a regression. Targetting Go 1.10.

[odeke-em]

Wassup @montanaflynn!

/cc @nigeltao and @horgh

[ghost]

The gif is probably invalid. With giflib 4.1.6 and the following program

#include <stdio.h>
#include <gif_lib.h>

int main(int argc, char *argv[]){
    GifFileType *m = DGifOpenFileName(argv[1]);
    if (m == NULL) {
        printf("DGifOpenFileName: error\n");
        return -1;
    }
    int e = DGifSlurp(m);
    if (e != GIF_OK) {
        printf("DGifSlurp: error\n");
        return -2;
    }
    return 0;
}

I get DGifSlurp: error.

[montanaflynn]

@odeke-em yo!

@opennota it's almost certainly malformed. Although ffmpeg, chrome, safari, firefox, OSX preview, etc... can all open it.

[horgh]

I've examined the file. It is invalid. It's nearly a valid GIF but not quite.

Why invalid

The decoder reads the file (including 34 Graphic Blocks) until it reaches the last 2 bytes. Up until that point the file is valid.

At that point it looks for the next block to read and finds these last 2 bytes: 0x01 0x3b.

It is not valid for a Data block to begin with 0x01. 0x01 is entirely out of place and the decoder rightly realizes this.

0x3b is a Trailer block, so if this 0x01 was not present the GIF would be valid. We'd recognize the GIF terminates as it should.

What to do

I'm not sure there's a great way to deal with this. Anything we do will mean making the decoder go outside the specification. However, here are some ideas:

1. Since we did read almost all of the GIF as valid (34 images as I mentioned) we could return as much of the GIF as we have when we encounter an unknown block. We should probably also still return the error. Since it's only the Trailer we're missing in this case it would be okay (assuming nothing else about the GIF is malformed which I've not checked). Currently we return the *GIF as nil if there is an error.
2. We could have a special case for this exact situation. If we see 0x01 followed by 0x3b then treat it as a Trailer (which would normally be only 0x3b). Not very nice.
3. We could skip invalid blocks. In this case we'd skip over 0x01 and then see the Trailer and be fine. This could be dangerous!

The first idea seems like it might be tolerable. It means returning as much as we were able to decode (which in some cases may not be much useful at all), but still tell the caller something went wrong. Although this behaviour would probably be of limited usefulness. Callers would have to know that sometimes there could be useful data despite an error.

I imagine this is why browsers and other decoders are able to show something. They may be returning whatever is valid.

[odeke-em]

Thank you very much @horgh, your diagnoses are always enlightening and educating. I'll let @nigeltao and others chime in for the way forward for hopefully Go1.10.

[horgh]

Sounds good @odeke-em. Thanks for the kind words! And for pinging me on this!

[nigeltao]

Yeah, the short story is that it's an invalid GIF. The browser may be showing valid frames up until the bad block type, but there's still a bad block type.

In general, the Go image codec APIs do not return partial results in case of error. Nor do they e.g. show the in-progress decodings of an interlaced PNG or progressive JPEG while decoding. There is no Go API to register a callback during png.Decode or jpeg.Decode.

Any way to ask for partial results would probably have to be new API, due to Go 1.x compat, and would ideally be consistent across all of the various image packages, not just image/gif.

Relatedly, it would be nice to have a consistent API for decoding moving images (both video like mp4 and animated GIFs), which might let the OP decode valid frames up until the bad block type, but again, that's an API design problem with larger scope than just patching image/gif.