From 9364868a07e9fe1cc58d963e4523bc5883201206 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 21 Aug 2013 08:29:41 +1000 Subject: [PATCH] archive/tar,zip: implement the os.FileInfo interface correctly. This is potentially an API-breaking change, but it is an important bug fix. The CL https://golang.org/cl/7305072/ added stuff to make the tar file look more like a file system internally, including providing an implementation of os.FileInfo for the file headers within the archive. But the code is incorrect because FileInfo.Name is supposed to return the base name only; this implementation returns the full path. A round trip test added in the same shows this in action, as the slashes are preserved as we create a header using the local implementation of FileInfo. The CL here changes the behavior of the tar (and zip) FileInfo to honor the Go spec for that interface. It also clarifies that the FileInfoHeader function, which takes a FileInfo as an argument, will therefore create a header with only the base name of the file recorded, and that subsequent adjustment may be necessary. There may be code out there that depends on the broken behavior. We can call out the risk in the release notes. Fixes #6180. R=golang-dev, dsymonds, adg, bradfitz CC=golang-dev https://golang.org/cl/13118043 --- src/pkg/archive/tar/common.go | 7 +++++-- src/pkg/archive/tar/tar_test.go | 11 ++++++++++- src/pkg/archive/zip/struct.go | 6 +++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/pkg/archive/tar/common.go b/src/pkg/archive/tar/common.go index 693076efce..1b961e3ec6 100644 --- a/src/pkg/archive/tar/common.go +++ b/src/pkg/archive/tar/common.go @@ -83,9 +83,9 @@ func (fi headerFileInfo) Sys() interface{} { return fi.h } // Name returns the base name of the file. func (fi headerFileInfo) Name() string { if fi.IsDir() { - return path.Clean(fi.h.Name) + return path.Base(path.Clean(fi.h.Name)) } - return fi.h.Name + return path.Base(fi.h.Name) } // Mode returns the permission and mode bits for the headerFileInfo. @@ -195,6 +195,9 @@ const ( // FileInfoHeader creates a partially-populated Header from fi. // If fi describes a symlink, FileInfoHeader records link as the link target. // If fi describes a directory, a slash is appended to the name. +// Because os.FileInfo's Name method returns only the base name of +// the file it describes, it may be necessary to modify the Name field +// of the returned header to provide the full path name of the file. func FileInfoHeader(fi os.FileInfo, link string) (*Header, error) { if fi == nil { return nil, errors.New("tar: FileInfo is nil") diff --git a/src/pkg/archive/tar/tar_test.go b/src/pkg/archive/tar/tar_test.go index dd6310313a..616a9cc57e 100644 --- a/src/pkg/archive/tar/tar_test.go +++ b/src/pkg/archive/tar/tar_test.go @@ -8,7 +8,9 @@ import ( "bytes" "io/ioutil" "os" + "path" "reflect" + "strings" "testing" "time" ) @@ -249,7 +251,14 @@ func TestHeaderRoundTrip(t *testing.T) { t.Error(err) continue } - if got, want := h2.Name, g.h.Name; got != want { + if strings.Contains(fi.Name(), "/") { + t.Errorf("FileInfo of %q contains slash: %q", g.h.Name, fi.Name()) + } + name := path.Base(g.h.Name) + if fi.IsDir() { + name += "/" + } + if got, want := h2.Name, name; got != want { t.Errorf("i=%d: Name: got %v, want %v", i, got, want) } if got, want := h2.Size, g.h.Size; got != want { diff --git a/src/pkg/archive/zip/struct.go b/src/pkg/archive/zip/struct.go index 73972d41cf..65e5238c3b 100644 --- a/src/pkg/archive/zip/struct.go +++ b/src/pkg/archive/zip/struct.go @@ -21,6 +21,7 @@ package zip import ( "os" + "path" "time" ) @@ -99,7 +100,7 @@ type headerFileInfo struct { fh *FileHeader } -func (fi headerFileInfo) Name() string { return fi.fh.Name } +func (fi headerFileInfo) Name() string { return path.Base(fi.fh.Name) } func (fi headerFileInfo) Size() int64 { if fi.fh.UncompressedSize64 > 0 { return int64(fi.fh.UncompressedSize64) @@ -113,6 +114,9 @@ func (fi headerFileInfo) Sys() interface{} { return fi.fh } // FileInfoHeader creates a partially-populated FileHeader from an // os.FileInfo. +// Because os.FileInfo's Name method returns only the base name of +// the file it describes, it may be necessary to modify the Name field +// of the returned header to provide the full path name of the file. func FileInfoHeader(fi os.FileInfo) (*FileHeader, error) { size := fi.Size() fh := &FileHeader{ -- 2.48.1