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

x/image/webp: "non-Alpha VP8X is not implemented" error while doing DecodeConfig #38341

Closed
andrius4669 opened this issue Apr 9, 2020 · 7 comments
Closed

x/image/webp: "non-Alpha VP8X is not implemented" error while doing DecodeConfig #38341

andrius4669 opened this issue Apr 9, 2020 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@andrius4669
Copy link
Contributor

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

$ go version
go version go1.14.1 linux/amd64

Does this issue reproduce with the latest release?

yep.

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

(probably irrelevant so won't include yet)

What did you do?

Quick repro program:

package main

import (
	_ "golang.org/x/image/webp"
	"image"
	"os"
	"fmt"
)

func main() {
	if len(os.Args) <= 1 {
		fmt.Fprintf(os.Stderr, "Usage: %s [filename...]", os.Args[0])
	}
	for i := 1; i < len(os.Args); i++ {
		fn := os.Args[i]
		f, e := os.Open(fn)
		if e != nil {
			panic("os.Open: " + e.Error())
		}
		cfg, ifmt, e := image.DecodeConfig(f)
		_ = f.Close()
		if e == nil {
			fmt.Printf("%s ok: %s %#v\n", fn, ifmt, cfg)
		} else {
			fmt.Printf("%s err: %v\n", fn, e)
		}
	}
}

ran it against https://0x0.st/iS94.webp

What did you expect to see?

No error. If it's able to recognize format and extract metadata, it should return them without raising error, even if it wouldn't be able to fully decode image.

What did you see instead?

tumblr_py63povN441yw147jo1_250.webp err: webp: non-Alpha VP8X is not implemented
@gopherbot gopherbot added this to the Unreleased milestone Apr 9, 2020
@andrius4669
Copy link
Contributor Author

oops, didn't notice already closed #25738 before posting this.
but it wasn't fixed, and closed by reporter because they found some cgo based lib with different API, and that's not very acceptable solution for me.

@andrius4669
Copy link
Contributor Author

quite simple tweak of source should sorta fix this at least for me (I only use DecodeConfig to detect file format and check image dimensions)

diff --git a/webp/decode.go b/webp/decode.go
index f77a4eb..26d7dc1 100644
--- a/webp/decode.go
+++ b/webp/decode.go
@@ -126,9 +126,6 @@ func decode(r io.Reader, configOnly bool) (image.Image, image.Config, error) {
                                alphaBit        = 1 << 4
                                iccProfileBit   = 1 << 5
                        )
-                       if buf[0] != alphaBit {
-                               return nil, image.Config{}, errors.New("webp: non-Alpha VP8X is not implemented")
-                       }
                        widthMinusOne = uint32(buf[4]) | uint32(buf[5])<<8 | uint32(buf[6])<<16
                        heightMinusOne = uint32(buf[7]) | uint32(buf[8])<<8 | uint32(buf[9])<<16
                        if configOnly {
@@ -138,6 +135,9 @@ func decode(r io.Reader, configOnly bool) (image.Image, image.Config, error) {
                                        Height:     int(heightMinusOne) + 1,
                                }, nil
                        }
+                       if buf[0] != alphaBit {
+                               return nil, image.Config{}, errors.New("webp: non-Alpha VP8X is not implemented")
+                       }
                        wantAlpha = true

                default:

I'm not really sure how to contribute to x/image repo yet.
Maybe someone could just land this tweak?
It doesn't fix general lack of robustness for decoding, and ColorModel will probably be wrong in some cases, but ColorModel was already wrong if VP8L would've came after alpha so I don't think this makes it worse, and it would also make webp usable for my use case (just get format and dimensions) (right now it isn't, because it fails on images ocuring in real life).

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 10, 2020
@andybons
Copy link
Member

@nigeltao

@gopherbot
Copy link

Change https://golang.org/cl/230137 mentions this issue: webp: make DecodeConfig less strict re VP8X flags

@andrius4669
Copy link
Contributor Author

https://play.golang.org/p/Yf77mdmNEZM small self-contained repro (not smallest possible though, just random small gif I found online and converted to webp)

@narrowizard narrowizard mentioned this issue Aug 20, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/249445 mentions this issue: fix(webp): decode non-alpha vp8x error.

gopherbot pushed a commit to golang/image that referenced this issue Sep 21, 2020
@narrowizard @nigeltao
webp: don't reject VP8X that isn't just VP8 + alpha
3a743ba 
We already support VP8 + alpha, but reject e.g. VP8 + EXIF. After this
commit, we still don't implement VP8 + EXIF (or ANIM, ICCP, etc.), but
we now silently ignore the EXIF chunk instead of rejecting it.

Fixes golang/go#25738, golang/go#38341

Change-Id: I4e9cdb718f0768f34336eab9528b82d2c40a3ee1
GitHub-Last-Rev: a0c2e53
GitHub-Pull-Request: #5
Reviewed-on: https://go-review.googlesource.com/c/image/+/249445
Trust: David Symonds <dsymonds@golang.org>
Reviewed-by: Nigel Tao <nigeltao@golang.org>
@andrius4669
Copy link
Contributor Author

Seems to be fixed by golang/image@3a743ba
Repro now executes without problems in both local testing and play.golang.org.
Therefore closing.

@golang golang locked and limited conversation to collaborators Sep 21, 2021
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 3, 2023
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
Unreleased
Development

No branches or pull requests
4 participants
@andybons @dmitshur @andrius4669 @gopherbot