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

archive/zip: possible bug #18378

Closed
bobjalex opened this issue Dec 19, 2016 · 2 comments
Closed

archive/zip: possible bug #18378

bobjalex opened this issue Dec 19, 2016 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bobjalex
Copy link

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8beta2 windows/amd64

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

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\GoLib_beta
set GORACE=
set GOROOT=C:\Go_beta
set GOTOOLDIR=C:\Go_beta\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\Bob\AppData\Local\Temp\go-build889712907=/tmp/go-build -gno-record-gcc-switches
set CXX=g++
set CGO_ENABLED=1
set PKG_CONFIG=pkg-config
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2

What did you do?

I've been playing with the archive/zip code recently, and spotted some suspicious code. You might want to have a quick look to see if I am right, or crazy.

In file reader.go around line 300, there is a labelled loop named Extras. The loop is for processing the ZIP extra fields in a DirectoryHeader. There are a lot of "break Extras" statements in that loop that seem like they might prevent all of the extra fields from being processed. For example, after a "zip64ExtraId" field is processed, it breaks out of the loop, preventing a possibly following "exttsExtraId" field from being processed.

Am I interpreting that correctly? If not, very sorry for wasting your time!!
But if I read it correctly, it's a subtle bug that easily could be missed in testing.

@bradfitz bradfitz changed the title Possible bug in archive/zip archive/zip: possible bug Dec 19, 2016
@dsnet
Copy link
Member

dsnet commented Dec 19, 2016

Thanks for the report, you are right that this seems buggy. This loop was introduced in Go1.8 for the extended timestamp logic.

@dsnet dsnet added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 19, 2016
@dsnet dsnet added this to the Go1.8 milestone Dec 19, 2016
@gopherbot
Copy link

CL https://golang.org/cl/34651 mentions this issue.

@golang golang locked and limited conversation to collaborators Dec 20, 2017
@rsc rsc unassigned dsnet Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants