| From 1e55f2a83b1f644803b640b72171b4ae0d95217b Mon Sep 17 00:00:00 2001 |
| From: "W. Trevor King" <wking@tremily.us> |
| Date: Thu, 20 Oct 2016 23:30:22 -0700 |
| Subject: [PATCH 2/2] image/manifest: Split unpackLayerEntry into its own |
| function |
| |
| To help address: |
| |
| $ make lint |
| checking lint |
| image/manifest.go:140::warning: cyclomatic complexity 39 of function unpackLayer() is high (> 35) (gocyclo) |
| ... |
| |
| Signed-off-by: W. Trevor King <wking@tremily.us> |
| --- |
| image/manifest.go | 185 +++++++++++++++++++++++++++++------------------------- |
| 1 file changed, 100 insertions(+), 85 deletions(-) |
| |
| diff --git a/image/manifest.go b/image/manifest.go |
| index 144bd4f62219..dfd5a83f70e4 100644 |
| --- a/src/import/image/manifest.go |
| +++ b/src/import/image/manifest.go |
| @@ -218,116 +218,131 @@ loop: |
| return errors.Wrapf(err, "error advancing tar stream") |
| } |
| |
| - hdr.Name = filepath.Clean(hdr.Name) |
| - if !strings.HasSuffix(hdr.Name, string(os.PathSeparator)) { |
| - // Not the root directory, ensure that the parent directory exists |
| - parent := filepath.Dir(hdr.Name) |
| - parentPath := filepath.Join(dest, parent) |
| - if _, err2 := os.Lstat(parentPath); err2 != nil && os.IsNotExist(err2) { |
| - if err3 := os.MkdirAll(parentPath, 0755); err3 != nil { |
| - return err3 |
| - } |
| - } |
| - } |
| - path := filepath.Join(dest, hdr.Name) |
| - if entries[path] { |
| - return fmt.Errorf("duplicate entry for %s", path) |
| - } |
| - entries[path] = true |
| - rel, err := filepath.Rel(dest, path) |
| + var whiteout bool |
| + whiteout, err = unpackLayerEntry(dest, hdr, tr, &entries) |
| if err != nil { |
| return err |
| } |
| - info := hdr.FileInfo() |
| - if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { |
| - return fmt.Errorf("%q is outside of %q", hdr.Name, dest) |
| + if whiteout { |
| + continue loop |
| } |
| |
| - if strings.HasPrefix(info.Name(), ".wh.") { |
| - path = strings.Replace(path, ".wh.", "", 1) |
| + // Directory mtimes must be handled at the end to avoid further |
| + // file creation in them to modify the directory mtime |
| + if hdr.Typeflag == tar.TypeDir { |
| + dirs = append(dirs, hdr) |
| + } |
| + } |
| + for _, hdr := range dirs { |
| + path := filepath.Join(dest, hdr.Name) |
| |
| - if err := os.RemoveAll(path); err != nil { |
| - return errors.Wrap(err, "unable to delete whiteout path") |
| + finfo := hdr.FileInfo() |
| + // I believe the old version was using time.Now().UTC() to overcome an |
| + // invalid error from chtimes.....but here we lose hdr.AccessTime like this... |
| + if err := os.Chtimes(path, time.Now().UTC(), finfo.ModTime()); err != nil { |
| + return errors.Wrap(err, "error changing time") |
| + } |
| + } |
| + return nil |
| +} |
| + |
| +// unpackLayerEntry unpacks a single entry from a layer. |
| +func unpackLayerEntry(dest string, header *tar.Header, reader io.Reader, entries *map[string]bool) (whiteout bool, err error) { |
| + header.Name = filepath.Clean(header.Name) |
| + if !strings.HasSuffix(header.Name, string(os.PathSeparator)) { |
| + // Not the root directory, ensure that the parent directory exists |
| + parent := filepath.Dir(header.Name) |
| + parentPath := filepath.Join(dest, parent) |
| + if _, err2 := os.Lstat(parentPath); err2 != nil && os.IsNotExist(err2) { |
| + if err3 := os.MkdirAll(parentPath, 0755); err3 != nil { |
| + return false, err3 |
| } |
| + } |
| + } |
| + path := filepath.Join(dest, header.Name) |
| + if (*entries)[path] { |
| + return false, fmt.Errorf("duplicate entry for %s", path) |
| + } |
| + (*entries)[path] = true |
| + rel, err := filepath.Rel(dest, path) |
| + if err != nil { |
| + return false, err |
| + } |
| + info := header.FileInfo() |
| + if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { |
| + return false, fmt.Errorf("%q is outside of %q", header.Name, dest) |
| + } |
| |
| - continue loop |
| + if strings.HasPrefix(info.Name(), ".wh.") { |
| + path = strings.Replace(path, ".wh.", "", 1) |
| + |
| + if err = os.RemoveAll(path); err != nil { |
| + return true, errors.Wrap(err, "unable to delete whiteout path") |
| } |
| |
| - if hdr.Typeflag != tar.TypeDir { |
| - err = os.RemoveAll(path) |
| - if err != nil && !os.IsNotExist(err) { |
| - return err |
| - } |
| + return true, nil |
| + } |
| + |
| + if header.Typeflag != tar.TypeDir { |
| + err = os.RemoveAll(path) |
| + if err != nil && !os.IsNotExist(err) { |
| + return false, err |
| } |
| + } |
| |
| - switch hdr.Typeflag { |
| - case tar.TypeDir: |
| - fi, err := os.Lstat(path) |
| + switch header.Typeflag { |
| + case tar.TypeDir: |
| + fi, err := os.Lstat(path) |
| + if err != nil && !os.IsNotExist(err) { |
| + return false, err |
| + } |
| + if os.IsNotExist(err) || !fi.IsDir() { |
| + err = os.RemoveAll(path) |
| if err != nil && !os.IsNotExist(err) { |
| - return err |
| - } |
| - if os.IsNotExist(err) || !fi.IsDir() { |
| - err = os.RemoveAll(path) |
| - if err != nil && !os.IsNotExist(err) { |
| - return err |
| - } |
| - err = os.MkdirAll(path, info.Mode()) |
| - if err != nil { |
| - return err |
| - } |
| + return false, err |
| } |
| - |
| - case tar.TypeReg, tar.TypeRegA: |
| - f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, info.Mode()) |
| + err = os.MkdirAll(path, info.Mode()) |
| if err != nil { |
| - return errors.Wrap(err, "unable to open file") |
| + return false, err |
| } |
| + } |
| |
| - if _, err := io.Copy(f, tr); err != nil { |
| - f.Close() |
| - return errors.Wrap(err, "unable to copy") |
| - } |
| - f.Close() |
| + case tar.TypeReg, tar.TypeRegA: |
| + f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, info.Mode()) |
| + if err != nil { |
| + return false, errors.Wrap(err, "unable to open file") |
| + } |
| |
| - case tar.TypeLink: |
| - target := filepath.Join(dest, hdr.Linkname) |
| + if _, err := io.Copy(f, reader); err != nil { |
| + f.Close() |
| + return false, errors.Wrap(err, "unable to copy") |
| + } |
| + f.Close() |
| |
| - if !strings.HasPrefix(target, dest) { |
| - return fmt.Errorf("invalid hardlink %q -> %q", target, hdr.Linkname) |
| - } |
| + case tar.TypeLink: |
| + target := filepath.Join(dest, header.Linkname) |
| |
| - if err := os.Link(target, path); err != nil { |
| - return err |
| - } |
| + if !strings.HasPrefix(target, dest) { |
| + return false, fmt.Errorf("invalid hardlink %q -> %q", target, header.Linkname) |
| + } |
| |
| - case tar.TypeSymlink: |
| - target := filepath.Join(filepath.Dir(path), hdr.Linkname) |
| + if err := os.Link(target, path); err != nil { |
| + return false, err |
| + } |
| |
| - if !strings.HasPrefix(target, dest) { |
| - return fmt.Errorf("invalid symlink %q -> %q", path, hdr.Linkname) |
| - } |
| + case tar.TypeSymlink: |
| + target := filepath.Join(filepath.Dir(path), header.Linkname) |
| |
| - if err := os.Symlink(hdr.Linkname, path); err != nil { |
| - return err |
| - } |
| - case tar.TypeXGlobalHeader: |
| - return nil |
| + if !strings.HasPrefix(target, dest) { |
| + return false, fmt.Errorf("invalid symlink %q -> %q", path, header.Linkname) |
| } |
| - // Directory mtimes must be handled at the end to avoid further |
| - // file creation in them to modify the directory mtime |
| - if hdr.Typeflag == tar.TypeDir { |
| - dirs = append(dirs, hdr) |
| - } |
| - } |
| - for _, hdr := range dirs { |
| - path := filepath.Join(dest, hdr.Name) |
| |
| - finfo := hdr.FileInfo() |
| - // I believe the old version was using time.Now().UTC() to overcome an |
| - // invalid error from chtimes.....but here we lose hdr.AccessTime like this... |
| - if err := os.Chtimes(path, time.Now().UTC(), finfo.ModTime()); err != nil { |
| - return errors.Wrap(err, "error changing time") |
| + if err := os.Symlink(header.Linkname, path); err != nil { |
| + return false, err |
| } |
| + case tar.TypeXGlobalHeader: |
| + return false, nil |
| } |
| - return nil |
| + |
| + return false, nil |
| } |
| -- |
| 2.4.0.53.g8440f74 |
| |