golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.31k stars 17.58k forks source link

x/image/tiff: grayscale tiled images are not decoded correctly #36121

Open zerkms opened 4 years ago

zerkms commented 4 years ago

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

$ go version
go version go1.13.4 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/ivan/.cache/go-build"
GOENV="/home/ivan/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/srv/work/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/ivan/.local/share/umake/go/go-lang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/ivan/.local/share/umake/go/go-lang/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/srv/work/go/src/go.googlesource.com/image/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build974283161=/tmp/go-build -gno-record-gcc-switches"

What did you do?

When you decode tiled tiff grayscale image you may find that tiles placed on the image boundary (right) are decoded incorrectly.

It happens because the tiff file contains the complete tile compressed, while the loop iterates only over the part of it that fits the image boundary:

            for y := ymin; y < rMaxY; y++ {
                for x := xmin; x < rMaxX; x++ {

So if you read pixels from the non-complete tiles (from the right edge) all lines but the first one would contain real data mixed with garbage (bits from outside the image boundary).

What did you expect to see?

What did you see instead?

gopherbot commented 4 years ago

Change https://golang.org/cl/211237 mentions this issue: go.image/tiff: fix for decoding grayscale tiled images

toothrot commented 4 years ago

/cc @nigeltao

nigeltao commented 4 years ago

/cc @bsiegert

zerkms commented 4 years ago

Hey team, any chance this to be dragged forward any time soon? It's a pretty significant bug, yet with a simple fix.

nigeltao commented 4 years ago

I dropped some review comments on https://go-review.googlesource.com/c/image/+/211237

Sorry for the late reply.

zerkms commented 3 years ago

Given how the progress ended last year, and given I want to invest some time into finally having it merging now: does anyone from Go team have understanding of what exactly I need to do to have it merged?

zerkms commented 3 years ago

Okay, I jumped into the code once again, and intermediate results/discoveries:

  1. mRGB 16bpp also affected, the same trivial fix
  2. mRGB (non 16bpp) is not affected, because offsets are calculated before each stride iteration
  3. mPaletted is affected, the same trivial fix
  4. mNRGBA 16bpp probably is affected, but I don't have an image to test
  5. mRGBA 16bpp probably is affected, but I don't have an image to test
  6. mNRGBA (non 16bpp) - most likely not affected, cannot verify without image
  7. mRGBA (non 16bpp) - most likely not affected, cannot verify without image

To summarise: I can fix 3 of them, and I need mNRGBA and mRGBA encoded images to fix the rest.