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

x/image: CCITT reader fails to return last row of data #34809

Open
dchang-dchang opened this issue Oct 10, 2019 · 5 comments

Comments

@dchang-dchang
Copy link

commented Oct 10, 2019

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

$ go version
go version go1.12.8 linux/amd64

Does this issue reproduce with the latest release?

This reproduces on the master branch of x/image.

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

go env Output
$ go env
GOARCH=amd64
GOBIN=
GOCACHE=/home/dchang/.cache/go-build
GOEXE=
GOFLAGS=
GOHOSTARCH=amd64
GOHOSTOS=linux
GOOS=linux
GOPATH=/home/dchang/go
GOPROXY=
GORACE=
GOROOT=/usr/local/go
GOTMPDIR=
GOTOOLDIR=/usr/local/go/pkg/tool/linux_amd64
GCCGO=gccgo
CC=gcc
CXX=g++
CGO_ENABLED=1
GOMOD=
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=/home/dchang/tmp/go-build386659025=/tmp/go-build -gno-record-gcc-switches

What did you do?

package main

import (
	"net/http"

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

func main() {

	resp, err := http.Get("https://github.com/golang/image/raw/cc248b1a6b0721f0d86b47a99c46fe460b2faac5/testdata/group3_test.tiff")
	if err != nil {
		panic(err)
	}
	defer resp.Body.Close()
	img, err := tiff.Decode(resp.Body)
	if err != nil {
		panic(err)
	}
	fmt.Println(img.Bounds())
}

What did you expect to see?

(0,0)-(1728,1103)

What did you see instead?

panic: tiff: invalid format: not enough pixel data

First, thank you @nigeltao and others for your work on the CCITT Group3 TIFF reader. Really coming in handy right as we're trying to parse TIFF fax files!

Second, let me preface this with the face that I had never heard of CCITT before this morning. I'm likely to have gotten a bunch wrong so for that, thanks for your patience and help!

The TLDR is that I think the CCITT reader is, in certain cases, decoding but not "packing" the last line

It happens when the io.Reader returns n bytes and also a io.EOF error [0]. This causes the CCITT reader to break immediately, and thus doesn't actually pack and thus results in the last row not part of the returned bytes, causing the TIFF package to be confused by the missing data (tiff: invalid format: not enough pixel data).

I found a group3 compressed tiff file from libtiff [1] that seems to exhibit this problem and the code in this PR [2] seems to address it.

Thanks for all the help!

Thanks!
David

[0] https://golang.org/pkg/io/#Reader

When Read encounters an error or end-of-file condition after
successfully reading n > 0 bytes, it returns the number of bytes read.
It may return the (non-nil) error from the same call or return the error
(and n == 0) from a subsequent call.

[1] See http://www.simplesystems.org/libtiff/images.html

[2] Created the PR first, but sounds like the issue tracker is here and so duplicating. Sorry for the noise and feel free to work off which ever one is convenient and we can close the other one.

@gopherbot

This comment has been minimized.

Copy link

commented Oct 15, 2019

Change https://golang.org/cl/200162 mentions this issue: ccitt: return all data from CCITT reader

@nigeltao

This comment has been minimized.

Copy link
Contributor

commented Oct 18, 2019

First, thank you @nigeltao and others

The others, @hhrutter and @bsiegert deserve more thanks than me.

decoding but not "packing" the last line

This is possibly related to https://go-review.googlesource.com/c/image/+/198547 which @hhrutter has coincidentally also sent out recently.

As for this issue (and CL 200162), perhaps @hhrutter and @bsiegert can take a first look at this and how it relates to CL 198547. I'm busier than usual with other things for the next few weeks.

@hhrutter

This comment has been minimized.

Copy link

commented Oct 18, 2019

This issue should be taken care of by CL 201938.
Note: The mentioned CL is tentative.

@dchang-dchang just for verification, please state which file of libtiff/images was giving you problems.

@dchang-dchang

This comment has been minimized.

Copy link
Author

commented Oct 21, 2019

Great! Totally agree that Cl 201938 will address this.

g3test.tif is the file that I was using. Thanks!

@gopherbot

This comment has been minimized.

Copy link

commented Oct 21, 2019

Change https://golang.org/cl/201938 mentions this issue: ccitt: suggested reader.go changes

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