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 EOF error for tiff image #39705

Open
xajler opened this issue Jun 19, 2020 · 7 comments
Open

x/image: CCITT reader EOF error for tiff image #39705

xajler opened this issue Jun 19, 2020 · 7 comments

Comments

@xajler
Copy link

@xajler xajler commented Jun 19, 2020

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

$ go version
go version go1.14.4 linux/amd64

Does this issue reproduce with the latest release?

With last master branch.

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/x/go/bin"
GOCACHE="/home/x/.cache/go-build"
GOENV="/home/x/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/x/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build670162663=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Tiff file used to get an error debug_26428.zip (compressed with zip, extract it for usage). Length of tiff file is: 26428 bytes.

I've created test cases that can be used for tiff and ccitt package. Because it is called with tiff.Decode(), but error is in ccitt.decodeEOL().

tiff

Copy extracted tiff to root testdata and copy this test case to tiff.reader_test.go

func TestTiffDecodeOfCCITTEndOfLine(t *testing.T) {
	b, err := ioutil.ReadFile("../testdata/debug_26428.tiff")
	if err != nil {
		t.Fatalf("Read File: %v", err)
	}
	if _, err = Decode(bytes.NewReader(b)); err != nil {
		t.Errorf("Shouldn't error, but got err: %v", err)
	}
}

Output:

--- FAIL: TestTiffDecodeOfCCITTEndOfLine (0.02s)
    reader_test.go:68: Shouldn't error, but got err: ccitt: missing End-of-Line

ccitt

Copy extracted tiff to ccitt testdata and copy this test to ccitt.reader_test.go

Data taken from actual debugging for this file, when called with tiff.Decode().

func TestReadEndOfLineError(t *testing.T) {
	f, err := os.Open("testdata/debug_26428.tiff")
	if err != nil {
		t.Fatalf("Open: %v", err)
	}
	defer f.Close()
	const width, height = 2481, 3508
	opts := &Options{
		Align:  false,
		Invert: true,
	}
	offset := 160
	n := 26268
	if _, err := ioutil.ReadAll(NewReader(io.NewSectionReader(f, int64(offset), int64(n)), MSB, Group4, width, height, opts)); err != nil {
		t.Fatalf("Shouldn't error but got error: %v", err)
	}
}

Output:

--- FAIL: TestReadEndOfLineError (0.03s)
    reader_test.go:411: Shouldn't error but got error: ccitt: missing End-of-Line

What works for this case

I'm not even remotely expert on specs for TIFF and ccitt, but I've changed last return from return errMissingEOL to return nil and I'm getting correct image and everything works fine and tried in many files (mostly all scanned textual with images, and all B/W) and I've see no problems with this fix/hack.

// decodeEOL decodes the 12-bit EOL code 0000_0000_0001.
func decodeEOL(b *bitReader) error {
	nBitsRead, bitsRead := uint32(0), uint64(0)
	for {
		bit, err := b.nextBit()
		if err != nil {
			if err == io.EOF {
				err = errMissingEOL
			}
			return err
		}
		bitsRead |= bit << (63 - nBitsRead)
		nBitsRead++

		if nBitsRead < 12 {
			if bit&1 == 0 {
				continue
			}
		} else if bit&1 != 0 {
			return nil
		}

		// Unread the bits we've read, then return errMissingEOL.
		b.bits = (b.bits >> nBitsRead) | bitsRead
		b.nBits += nBitsRead

                // Changing from errMissingEOL to nil works for this case
		// return errMissingEOL
               return nil
	}
}

Request

Can anybody see in TIFF provided, run this tests, maybe do better "fix". Because with this applied ccitt test TestRead fails with output:

--- FAIL: TestRead (0.00s)
    reader_test.go:437: AutoDetectHeight produced different output.

Thanks

@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented Jun 23, 2020

/cc @nigeltao

Loading

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Aug 24, 2020

Loading

@hhrutter
Copy link

@hhrutter hhrutter commented Aug 31, 2020

Hi @xajler!
Your fix is not feasible since it is ignoring any missing/corrupt EOLs.
I am working on this.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 1, 2020

Change https://golang.org/cl/252037 mentions this issue: ccitt: relax decoding for incomplete eol trailer

Loading

@xajler
Copy link
Author

@xajler xajler commented Sep 1, 2020

Thanks for looking into it, @hhrutter.
I thought it was a more of a hack than fix, but so far it worked in cases I was using.

Loading

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Oct 10, 2020

Well, if I understand the spec correctly, missing EOLs are actually malformed CCITT images (or malformed TIFF images), and see also (https://en.wikipedia.org/wiki/Robustness_principle#Criticism). Two questions.

  1. Do you know what program generated that failing TIFF image?

  2. Do you know whether other image decoders (e.g. ImageMagick, GIMP) accept such a TIFF image with missing EOLs?

Loading

@hhrutter
Copy link

@hhrutter hhrutter commented Oct 11, 2020

The failing TIFF opens up nicely with Mac Preview and ImageMagick does not seem to have any problem with it:

Go-> magick identify debug_26428.tiff
debug_26428.tiff TIFF 2481x3508 2481x3508+0+0 1-bit Bilevel Gray 26428B 0.000u 0:00.003

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants