archive/zip: NewReader panics on negative size

[LIUHUANUCAS]

Please answer these questions before submitting your issue. Thanks!

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

go version go1.7 linux/amd64

Does this issue reproduce with the latest release?

have no yet test

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

GOARCH="amd64"
GOBIN="/usr/local/go/bin/"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/usr/local/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build197527898=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

zipReader, err := zip.NewReader(bytes.NewReader(respValue), -1)

• -1 may be the http.Response.ContentLength

What did you expect to see?

panic(0x60a540, 0xc42000d190)
	/usr/local/go/src/runtime/panic.go:500 +0x1a1
archive/zip.readDirectoryEnd(0x93eec0, 0xc420013230, 0xffffffffffffffff, 0x6672a9, 0xc41ffff1d9, 0x0)
	/usr/local/go/src/archive/zip/reader.go:397 +0x9d
archive/zip.(*Reader).init(0xc42001c480, 0x93eec0, 0xc420013230, 0xffffffffffffffff, 0x620ba0, 0x1)
	/usr/local/go/src/archive/zip/reader.go:79 +0x5d
archive/zip.NewReader(0x93eec0, 0xc420013230, 0xffffffffffffffff, 0x4, 0x0, 0x0)
	/usr/local/go/src/archive/zip/reader.go:72 +0x62

My Question

• The second parameter of function zip.NewReader is int64,and -1 has passed ,the program panic but not return a error.
• because the function zip.NewReader has an error as returned value, so the way of design is Reasonable ？

[zegl]

This is reproducible with Go 1.10: playground.

I suggest that we add a check to zip.NewReader to make sure that size >= 0, and return an error if it's not. Alternatively improve the documentation to mention that the function will panic if size is smaller than 0.

[iamoryanmoshe]

What is causing the panic though? No input checks?

[bradfitz]

I suppose we could just return an error there instead, as it already returns an error.

I think adding documentation would be overkill, as -1 for a size seems like something you'd only do by mistake anyway.

[jeet-parekh]

I see @dsnet is assigned. Mind if I go ahead and submit a CL to fix this?

[bradfitz]

Go for it. Note that we won't review it until Go 1.11 is out in August, though, so no rush.

[jeet-parekh]

Got it.

[dsnet]

Yep, go for it.

[gopherbot]

Change https://golang.org/cl/126617 mentions this issue: archive/zip: return error from NewReader when negative size is passed