Merge pull request #1245 from dmcgowan/archive-more-tar-fixes

Update tar path resolution
This commit is contained in:
Michael Crosby 2017-07-25 09:49:10 -04:00 committed by GitHub
commit 1251413a96
3 changed files with 291 additions and 45 deletions

View File

@ -28,7 +28,7 @@ func rootPath(root, path string) (string, error) {
}
path = newpath
if i == linksWalked {
newpath = rootJoin(newpath)
newpath = filepath.Join("/", newpath)
if path == newpath {
return filepath.Join(root, newpath), nil
}
@ -37,28 +37,12 @@ func rootPath(root, path string) (string, error) {
}
}
// rootJoin joins a path with root, cleaning up any links that
// reference above root.
func rootJoin(path string) string {
if filepath.IsAbs(path) {
path = filepath.Clean(path)
}
// Resolve any ".." or "/.." before joining to root
for !filepath.IsAbs(path) {
path = "/" + path
path = filepath.Clean(path)
}
return path
}
func walkLink(root, path string, linksWalked *int) (newpath string, islink bool, err error) {
if *linksWalked > 255 {
return "", false, errTooManyLinks
}
path = rootJoin(path)
path = filepath.Join("/", path)
if path == "/" {
return path, false, nil
}

View File

@ -126,19 +126,30 @@ func Apply(ctx context.Context, root string, r io.Reader) (int64, error) {
continue
}
// Note as these operations are platform specific, so must the slash be.
if !strings.HasSuffix(hdr.Name, string(os.PathSeparator)) {
// Not the root directory, ensure that the parent directory exists.
// This happened in some tests where an image had a tarfile without any
// parent directories.
parent := filepath.Dir(hdr.Name)
parentPath, err := rootPath(root, parent)
if err != nil {
return 0, err
}
// Split name and resolve symlinks for root directory.
ppath, base := filepath.Split(hdr.Name)
ppath, err = rootPath(root, ppath)
if err != nil {
return 0, errors.Wrap(err, "failed to get root path")
}
// Join to root before joining to parent path to ensure relative links are
// already resolved based on the root before adding to parent.
path := filepath.Join(ppath, filepath.Join("/", base))
if path == root {
log.G(ctx).Debugf("file %q ignored: resolved to root", hdr.Name)
continue
}
// If file is not directly under root, ensure parent directory
// exists or is created.
if ppath != root {
parentPath := ppath
if base == "" {
parentPath = filepath.Dir(path)
}
if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) {
err = mkdirAll(parentPath, 0600)
err = mkdirAll(parentPath, 0700)
if err != nil {
return 0, err
}
@ -173,13 +184,6 @@ func Apply(ctx context.Context, root string, r io.Reader) (int64, error) {
}
}
path, err := rootPath(root, hdr.Name)
if err != nil {
return 0, errors.Wrap(err, "failed to get root path")
}
base := filepath.Base(path)
if strings.HasPrefix(base, whiteoutPrefix) {
dir := filepath.Dir(path)
if base == whiteoutOpaqueDir {

View File

@ -89,6 +89,83 @@ func TestRelativeSymlinks(t *testing.T) {
}
}
func TestSymlinks(t *testing.T) {
links := [][2]fstest.Applier{
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateFile("/bin/superbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("../bin/superbinary", "/bin/other1"),
),
fstest.Apply(
fstest.Remove("/bin/other1"),
fstest.Symlink("/bin/superbinary", "/bin/other1"),
fstest.Symlink("../bin/superbinary", "/bin/other2"),
fstest.Symlink("superbinary", "/bin/other3"),
),
},
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateDir("/sbin/", 0755),
fstest.CreateFile("/sbin/superbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("/sbin/superbinary", "/bin/superbinary"),
fstest.Symlink("../bin/superbinary", "/bin/other1"),
),
fstest.Apply(
fstest.Remove("/bin/other1"),
fstest.Symlink("/bin/superbinary", "/bin/other1"),
fstest.Symlink("superbinary", "/bin/other2"),
),
},
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateDir("/sbin/", 0755),
fstest.CreateFile("/sbin/superbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("../sbin/superbinary", "/bin/superbinary"),
fstest.Symlink("../bin/superbinary", "/bin/other1"),
),
fstest.Apply(
fstest.Remove("/bin/other1"),
fstest.Symlink("/bin/superbinary", "/bin/other1"),
),
},
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateFile("/bin/actualbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("actualbinary", "/bin/superbinary"),
fstest.Symlink("../bin/superbinary", "/bin/other1"),
fstest.Symlink("superbinary", "/bin/other2"),
),
fstest.Apply(
fstest.Remove("/bin/other1"),
fstest.Remove("/bin/other2"),
fstest.Symlink("/bin/superbinary", "/bin/other1"),
fstest.Symlink("superbinary", "/bin/other2"),
),
},
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateFile("/bin/actualbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("actualbinary", "/bin/myapp"),
),
fstest.Apply(
fstest.Remove("/bin/myapp"),
fstest.CreateDir("/bin/myapp", 0755),
),
},
}
for i, l := range links {
if err := testDiffApply(l[0], l[1]); err != nil {
t.Fatalf("Test[%d] apply failed: %+v", i+1, err)
}
}
}
func TestBreakouts(t *testing.T) {
tc := TarContext{}.WithUidGid(os.Getuid(), os.Getgid()).WithModTime(time.Now().UTC())
expected := "unbroken"
@ -102,10 +179,17 @@ func TestBreakouts(t *testing.T) {
}
return nil
}
errFileDiff := errors.New("files differ")
sameFile := func(f1, f2 string) func(string) error {
return func(root string) error {
p1 := filepath.Join(root, f1)
p2 := filepath.Join(root, f2)
p1, err := rootPath(root, f1)
if err != nil {
return err
}
p2, err := rootPath(root, f2)
if err != nil {
return err
}
s1, err := os.Stat(p1)
if err != nil {
return err
@ -115,7 +199,20 @@ func TestBreakouts(t *testing.T) {
return err
}
if !os.SameFile(s1, s2) {
return errors.Errorf("files differ: %#v and %#v", s1, s2)
return errors.Wrapf(errFileDiff, "%#v and %#v", s1, s2)
}
return nil
}
}
notSameFile := func(f1, f2 string) func(string) error {
same := sameFile(f1, f2)
return func(root string) error {
err := same(root)
if err == nil {
return errors.New("files are the same, expected diff")
}
if errors.Cause(err) != errFileDiff {
return err
}
return nil
}
@ -124,6 +221,7 @@ func TestBreakouts(t *testing.T) {
breakouts := []struct {
name string
w WriterToTar
apply fstest.Applier
validator func(string) error
}{
{
@ -277,10 +375,103 @@ func TestBreakouts(t *testing.T) {
),
validator: sameFile("/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkParentDirectory",
w: TarAll(
tc.Dir("etc", 0770),
tc.File("/etc/passwd", []byte("inside"), 0644),
tc.Symlink("/etc/", ".."),
tc.Link("/etc/passwd", "localpasswd"),
),
validator: sameFile("/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkEmptyFilename",
w: TarAll(
tc.Dir("etc", 0770),
tc.File("/etc/passwd", []byte("inside"), 0644),
tc.Symlink("/etc/", ""),
tc.Link("/etc/passwd", "localpasswd"),
),
validator: sameFile("/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkParentRelative",
w: TarAll(
tc.Dir("etc", 0770),
tc.File("/etc/passwd", []byte("inside"), 0644),
tc.Symlink("/etc/", "localetc/sub/.."),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkSlashEnded",
w: TarAll(
tc.Dir("etc", 0770),
tc.File("/etc/passwd", []byte("inside"), 0644),
tc.Dir("localetc/", 0770),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkOverrideDirectory",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
fstest.CreateDir("/localetc/", 0755),
),
w: TarAll(
tc.Symlink("/etc", "localetc"),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkOverrideDirectoryRelative",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
fstest.CreateDir("/localetc/", 0755),
),
w: TarAll(
tc.Symlink("../../etc", "localetc"),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "DirectoryOverrideSymlink",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
fstest.Symlink("/etc", "localetc"),
),
w: TarAll(
tc.Dir("/localetc/", 0755),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "DirectoryOverrideSymlinkAndHardlink",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
fstest.Symlink("etc", "localetc"),
fstest.Link("/etc/passwd", "/localetc/localpasswd"),
),
w: TarAll(
tc.Dir("/localetc/", 0755),
tc.File("/localetc/localpasswd", []byte("different"), 0644),
),
validator: notSameFile("/localetc/localpasswd", "/etc/passwd"),
},
}
for _, bo := range breakouts {
t.Run(bo.name, makeWriterToTarTest(bo.w, bo.validator))
t.Run(bo.name, makeWriterToTarTest(bo.w, bo.apply, bo.validator))
}
}
@ -288,6 +479,56 @@ func TestDiffApply(t *testing.T) {
fstest.FSSuite(t, diffApplier{})
}
func TestApplyTar(t *testing.T) {
tc := TarContext{}.WithUidGid(os.Getuid(), os.Getgid()).WithModTime(time.Now().UTC())
directoriesExist := func(dirs ...string) func(string) error {
return func(root string) error {
for _, d := range dirs {
p, err := rootPath(root, d)
if err != nil {
return err
}
if _, err := os.Stat(p); err != nil {
return errors.Wrapf(err, "failure checking existance for %v", d)
}
}
return nil
}
}
tests := []struct {
name string
w WriterToTar
apply fstest.Applier
validator func(string) error
}{
{
name: "DirectoryCreation",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
),
w: TarAll(
tc.Dir("/etc/subdir", 0755),
tc.Dir("/etc/subdir2/", 0755),
tc.Dir("/etc/subdir2/more", 0755),
tc.Dir("/other/noparent-1/1", 0755),
tc.Dir("/other/noparent-2/2/", 0755),
),
validator: directoriesExist(
"etc/subdir",
"etc/subdir2",
"etc/subdir2/more",
"other/noparent-1/1",
"other/noparent-2/2",
),
},
}
for _, at := range tests {
t.Run(at.name, makeWriterToTarTest(at.w, at.apply, at.validator))
}
}
func testApply(a fstest.Applier) error {
td, err := ioutil.TempDir("", "test-apply-")
if err != nil {
@ -356,7 +597,7 @@ func testBaseDiff(a fstest.Applier) error {
return fstest.CheckDirectoryEqual(td, dest)
}
func testDiffApply(a fstest.Applier) error {
func testDiffApply(appliers ...fstest.Applier) error {
td, err := ioutil.TempDir("", "test-diff-apply-")
if err != nil {
return errors.Wrap(err, "failed to create temp dir")
@ -368,11 +609,22 @@ func testDiffApply(a fstest.Applier) error {
}
defer os.RemoveAll(dest)
if err := a.Apply(td); err != nil {
return errors.Wrap(err, "failed to apply filesystem changes")
for _, a := range appliers {
if err := a.Apply(td); err != nil {
return errors.Wrap(err, "failed to apply filesystem changes")
}
}
diffBytes, err := ioutil.ReadAll(Diff(context.Background(), "", td))
// Apply base changes before diff
if len(appliers) > 1 {
for _, a := range appliers[:len(appliers)-1] {
if err := a.Apply(dest); err != nil {
return errors.Wrap(err, "failed to apply base filesystem changes")
}
}
}
diffBytes, err := ioutil.ReadAll(Diff(context.Background(), dest, td))
if err != nil {
return errors.Wrap(err, "failed to create diff")
}
@ -384,7 +636,7 @@ func testDiffApply(a fstest.Applier) error {
return fstest.CheckDirectoryEqual(td, dest)
}
func makeWriterToTarTest(wt WriterToTar, validate func(string) error) func(*testing.T) {
func makeWriterToTarTest(wt WriterToTar, a fstest.Applier, validate func(string) error) func(*testing.T) {
return func(t *testing.T) {
td, err := ioutil.TempDir("", "test-writer-to-tar-")
if err != nil {
@ -392,6 +644,12 @@ func makeWriterToTarTest(wt WriterToTar, validate func(string) error) func(*test
}
defer os.RemoveAll(td)
if a != nil {
if err := a.Apply(td); err != nil {
t.Fatalf("Failed to apply filesystem to directory: %v", err)
}
}
tr := TarFromWriterTo(wt)
if _, err := Apply(context.Background(), td, tr); err != nil {