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

diff --git a/src/pkg/archive/tar/common.go b/src/pkg/archive/tar/common.go
index 693076efced96b27b01451ee9e25d7460ba364c4..1b961e3ec637c4630018d17ac69131cc3b364277 100644 (file)
--- 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 dd6310313afa8bf6b5b307df3a32ea12c3f6c285..616a9cc57eee6884a500ec10499beb7243832b2c 100644 (file)
--- 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 73972d41cf0c9d8d95628bbd3322a8f51dba9144..65e5238c3b45cd155f9299881ff37008d059381b 100644 (file)
--- 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{