Commit 9364868a authored by Rob Pike's avatar Rob Pike

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
parent ec89171e
......@@ -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")
......
......@@ -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 {
......
......@@ -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{
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment