archive/zip: Reader.Open lies about directory fs.FileMode

[colin-sitehost]

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

$ go version
go version go1.17 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/go/pkg/mod"
GOOS="linux"
GOPATH="/home/user/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build2958766942=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/-MVV8qRhxxC roughly:

r, _ := NewReader(b, len)
f, _ := r.Open("path/to/dir")
s, _ := f.Stat()
fmt.Print(s.Mode())

What did you expect to see?

drw-rw-rw- (as derived from zip.File.FileHeader.Mode())

What did you see instead?

dr-xr-xr-x

Additional context?

It appears that the implementation of Reader.Open, specifically fileListEntry.Mode, instead of returning the actual mode contained in fileListEntry.file.FileHeader.Mode, will returns a hardcoded fs.FileMode:

package zip // import "archive/zip"

func (f *fileListEntry) Mode() fs.FileMode { return fs.ModeDir | 0555 }

I am guessing, but it seems like this was done since fileListEntry.file can be nil when the directory entry is synthetic, e.g. the zip contains a file without directory files leading to it, see #48084.

If we wanted to hit two birds with one stone, since this would be a clear signal that "the directory file is missing", I think it would be reasonable to: (though it is a breaking change)

package zip // import "archive/zip"

func (f *fileListEntry) Mode() fs.FileMode {
        if f.file == nil {
                return 0
        }
        return f.file.FileHeader.Mode
}

[mengzhuo]

cc @rolandshoemaker @dsnet

[cherrymui]

I think the executable bit for a directory generally means you can enter that directory, so 0555 makes sense to me. Why do you expect drw-rw-rw- ?

The 0555 is from CL https://go-review.googlesource.com/c/go/+/243937 . cc @rsc

[colin-sitehost]

so, this is kind of a poor reproducer, since I am using Writer.Create for both the file and directory, thus we are getting a "default" mode, I can happily modify the example to do a full Writer.CreateHeader and set a custom mode, but this seemed like the minimum viable reproducer.

My comment is not that drw-rw-rw- (0o20000000555) is a better or worse file mode than drw-rw-rw- (0o20000000666), but that the zip bytes stored in b define dir/ to have a mode of drw-rw-rw-, thus that is what should be returned by archive/zipfor both Reader.File and Reader.Open.