Skip to content

x/image: bmp decode of attached image will result in all alpha values to be 0 #54368

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
Closed
maaft opened this issue Aug 10, 2022 · 7 comments
Closed

x/image: bmp decode of attached image will result in all alpha values to be 0 #54368

maaft opened this issue Aug 10, 2022 · 7 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unreleased

Comments

@maaft
Copy link

maaft commented Aug 10, 2022 •
edited
Loading

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

$ go version

go version go1.19 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home//.cache/go-build"
GOENV="/home//.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home//go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home//go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build115352423=/tmp/go-build -gno-record-gcc-switches"

What did you do?

download this BMP image

execute this bmp-to-png conversion "script":

package main

import (
	"bytes"
	"image/png"
	"os"

	"golang.org/x/image/bmp"
)

func must(err error) {
	if err != nil {
		panic(err)
	}
}

func main() {

	f, err := os.Open("<path to bmp file>")

	must(err)

	bmp_decode, err := bmp.Decode(f)

	must(err)

	buffer := &bytes.Buffer{}
	err = png.Encode(buffer, bmp_decode)

	must(err)

	err = os.WriteFile("test.png", buffer.Bytes(), 0644)

	must(err)

}

What did you expect to see?

I expect to see the original image in png format or an error while opening the bmp file if its corrupt.

What did you see instead?

I saw no errors and the resulting png image is completely transparent. All alpha values of bmp_decode are 0.
@gopherbot gopherbot added this to the Unreleased milestone Aug 10, 2022
@thanm thanm added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 10, 2022
@thanm
Copy link
Contributor

thanm commented Aug 10, 2022

I am getting a 404 on the link cited (https://file.io/3QG9p08hAz71)

@maaft
Copy link
Author

maaft commented Aug 16, 2022

@thanm Oh, sorry about that. That provider seems to delete the file after first download. Since I can't upload BMP files here directly, any idea what to use for a hosting service?

@seankhliao
Copy link
Member

zip the file and upload here

@maaft
Copy link
Author

maaft commented Aug 18, 2022

lol, yes. how could I not think about zipping it -.- thanks
30Sep23-84.BMP.zip

@ZekeLu
Copy link
Contributor

ZekeLu commented Aug 25, 2022

Here is the information of the image:

Offset Size Hex value Value Description
BMP Header
00 2 42 4D "BM" ID field
02 4 96 38 01 00 80022 bytes (0x013896) Size of the BMP file (54 bytes header + 79968 bytes data)
06 2 00 00 Unused Application specific
08 2 00 00 Unused Application specific
0A 4 36 00 00 00 54 bytes (14+40) Offset where the pixel array (bitmap data) can be found
DIB Header (BITMAPINFOHEADER)
0E 4 28 00 00 00 🔴 40 bytes Number of bytes in the DIB header (from this point)
12 4 88 00 00 00 136 pixels Width of the bitmap in pixels
16 4 93 00 00 00 147 pixels Height of the bitmap in pixels
1A 2 01 00 🔴 1 plane Number of color planes being used
1C 2 20 00 🔴 32 bits Number of bits per pixel
1E 4 00 00 00 00 🔴 0 BI_RGB, no pixel array compression used
22 4 60 38 01 00 79968 bytes (0x013860) Size of the raw bitmap data
26 4 00 00 00 00 horizontal resolution Not provided
2A 4 00 00 00 00 vertical resolution
2E 4 00 00 00 00 0 colors Number of colors in the palette
32 4 00 00 00 00 0 important colors 0 means all colors are important
Start of pixel array (bitmap data)
36 4 7A 7A 7A 00 🔴 The last byte (00) in the pixel
is incorrectly treated as the alpha value
3A 4 7A 7A 7A 00
3E 4 78 78 78 00
pixel array truncated...

We are interested in these attributes:

  • The size of the DIB header is 40, which means it's a BITMAPINFOHEADER
  • The bits per pixel (bpp) is 32
  • The type of compression is BI_RGB (no pixel array compression used)

For BITMAPINFOHEADER with compression set to BI_RGB, the alpha channel is not supported (see the references in the end of this comment). So the alpha component in the decoded image data should be 0xFF.

I think this is a bug in the x/image/bmp package. I will send a CL to fix it.

It's interesting that the x/image repository already contains two test images which looks different but decoded to the same image data (covered by a test):

  • testdata/yellow_rose-small.bmp:

  • testdata/yellow_rose-small.png:

My fix will make this test fail. Obviously, the two images look different and in my opinion, the failure is expected. I will modify testdata/yellow_rose-small.png to make it pass again.

BTW, GraphicsMagick 1.3.38 has the same issue. Running the following command will generate a transparent image:

gm convert 30Sep23-84.BMP 30Sep23-84.png

References:

@ZekeLu ZekeLu mentioned this issue Aug 25, 2022
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/425418 mentions this issue: bmp: ignore alpha in Decode when the alpha channel is not supported

@thanm
Copy link
Contributor

thanm commented Aug 25, 2022

@ZekeLu thanks for sending that CL, looks good to me (although I have very little familiarity with image/bmp).

@golang golang locked and limited conversation to collaborators Sep 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

bmp: ignore alpha in Decode when the alpha channel is not supported ZekeLu/image
5 participants
@ZekeLu @maaft @gopherbot @seankhliao @thanm