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

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

Open
montanaflynn opened this issue Jun 26, 2017 · 8 comments

Comments

@montanaflynn
Copy link

commented Jun 26, 2017

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 bradfitz changed the title Decoding gif returns `unknown block type: 0x01` error image/gif: decoding gif returns `unknown block type: 0x01` error Jun 26, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jun 26, 2017

I can repro at tip (0d33a89), ~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.

@bradfitz bradfitz modified the milestones: Go1.9, Go1.10 Jun 26, 2017

@odeke-em

This comment has been minimized.

Copy link
Member

commented Jun 27, 2017

Wassup @montanaflynn!

/cc @nigeltao and @horgh

@opennota

This comment has been minimized.

Copy link

commented Jun 28, 2017

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

This comment has been minimized.

Copy link
Author

commented Jun 28, 2017

@odeke-em yo!

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

@horgh

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2017

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

This comment has been minimized.

Copy link
Member

commented Jul 4, 2017

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

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2017

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

@nigeltao

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.