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

Jump to bottom

cmd/go: panic when GOPROXY sends incorrect zip format #26536

Closed
marwan-at-work opened this issue Jul 22, 2018 · 1 comment
Closed

cmd/go: panic when GOPROXY sends incorrect zip format #26536

marwan-at-work opened this issue Jul 22, 2018 · 1 comment
Labels
FrozenDueToAge modules

Comments

@marwan-at-work
Copy link
Contributor

marwan-at-work commented Jul 22, 2018 •
edited

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

Tip: go version devel +a2419221fd Fri Jul 13 00:03:31 2018 +0000 darwin/amd64

Does this issue reproduce with the latest release?

No

What did you do?

Set a GOPROXY env var to a server where it might send incorrect/corrupt zip format when cmd/go sends a GET request to /{module}/@v/{version}.zip to download the zip.

What did you expect to see?

cmd/go should not panic. I expect to see an error indicating that the zip file was incorrect.

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1203f62]

goroutine 47 [running]:
archive/zip.(*ReadCloser).Close(0x0, 0x4c, 0x0)
	/Users/208581/Desktop/gosource/go/src/archive/zip/reader.go:139 +0x22
cmd/go/internal/modfetch.downloadZip(0xc0000201c1, 0x15, 0xc000120118, 0x6, 0xc00013e320, 0x4b, 0x0, 0x0)
	/Users/208581/Desktop/gosource/go/src/cmd/go/internal/modfetch/fetch.go:93 +0x79c

The error is happening because we're calling z.Close() in modfetch/fetch.go:93 when err != nil -- looking at zip.OpenReader it always returns nil if there was an error, so I believe it's safe to remove z.Close() from the err check.
marwan-at-work added a commit to marwan-at-work/go that referenced this issue Jul 22, 2018
@marwan-at-work
cmd/go/internal/modfetch: do not panic when zip is incorrect from GOP…
88040a9 
…ROXY

If GOPROXY sends the wrong zip data when cmd/go asks for a module's zip ball,
we should not panic. zip.OpenReader always returns a nil ReadCLoser if err is not nil.

Therefore, it's Okay to remove zip.Close in the err block.

Fixes golang#26536
@marwan-at-work marwan-at-work mentioned this issue Jul 22, 2018
Closed
@gopherbot
Copy link

Change https://golang.org/cl/125436 mentions this issue: cmd/go/internal/modfetch: do not panic when zip is incorrect

@oiooj oiooj added the modules label Jul 23, 2018
@marwan-at-work marwan-at-work mentioned this issue Jul 28, 2018
Closed
@golang golang locked and limited conversation to collaborators Jul 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge modules
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@oiooj @gopherbot @marwan-at-work