zip: add CheckFiles, CheckDir, and CheckZip
These functions may be used to check whether the files in an abstract
list, a directory, or a module zip file satisfy the module name and
size constraints listed in the package documentation. Each function
returns a CheckedFiles record that lists valid, omitted, and invalid
files, as well as any size-related error for the whole set of
files. The omitted and invalid lists have an error for each file,
saying why it was omitted or invalid.
Create, CreateFromDir, and Unzip are now implemented using these
functions (or common code). They now return errors based on
CheckedFiles errors. Most error messages won't change, but if multiple
files are invalid, they will be all be listed instead of just the
first one.
Fixes golang/go#36058
Updates golang/go#39091
Change-Id: I9d4d508288bbd821f93423e712232d8a68356529
Reviewed-on: https://go-review.googlesource.com/c/mod/+/235597
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/zip/testdata/check_dir/empty.txt b/zip/testdata/check_dir/empty.txt
new file mode 100644
index 0000000..529a082
--- /dev/null
+++ b/zip/testdata/check_dir/empty.txt
@@ -0,0 +1,6 @@
+-- want --
+valid:
+
+omitted:
+
+invalid:
diff --git a/zip/testdata/check_dir/various.txt b/zip/testdata/check_dir/various.txt
new file mode 100644
index 0000000..ee843be
--- /dev/null
+++ b/zip/testdata/check_dir/various.txt
@@ -0,0 +1,20 @@
+-- want --
+valid:
+$work/valid.go
+
+omitted:
+$work/.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
+$work/.git: directory is a version control repository
+$work/sub: directory is in another module
+$work/vendor/x/y: file is in vendor directory
+
+invalid:
+$work/GO.MOD: go.mod files must have lowercase names
+$work/invalid.go': malformed file path "invalid.go'": invalid char '\''
+-- valid.go --
+-- GO.MOD --
+-- invalid.go' --
+-- vendor/x/y --
+-- sub/go.mod --
+-- .hg_archival.txt --
+-- .git/x --
diff --git a/zip/testdata/check_files/empty.txt b/zip/testdata/check_files/empty.txt
new file mode 100644
index 0000000..529a082
--- /dev/null
+++ b/zip/testdata/check_files/empty.txt
@@ -0,0 +1,6 @@
+-- want --
+valid:
+
+omitted:
+
+invalid:
diff --git a/zip/testdata/check_files/various.txt b/zip/testdata/check_files/various.txt
new file mode 100644
index 0000000..a704a8a
--- /dev/null
+++ b/zip/testdata/check_files/various.txt
@@ -0,0 +1,25 @@
+-- want --
+valid:
+valid.go
+
+omitted:
+vendor/x/y: file is in vendor directory
+sub/go.mod: file is in another module
+.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
+
+invalid:
+not/../clean: file path is not clean
+GO.MOD: go.mod files must have lowercase names
+invalid.go': malformed file path "invalid.go'": invalid char '\''
+valid.go: multiple entries for file "valid.go"
+-- valid.go --
+-- not/../clean --
+-- GO.MOD --
+-- invalid.go' --
+-- vendor/x/y --
+-- sub/go.mod --
+-- .hg_archival.txt --
+-- valid.go --
+duplicate
+-- valid.go --
+another duplicate
diff --git a/zip/testdata/check_zip/empty.txt b/zip/testdata/check_zip/empty.txt
new file mode 100644
index 0000000..b71f05f
--- /dev/null
+++ b/zip/testdata/check_zip/empty.txt
@@ -0,0 +1,8 @@
+path=example.com/empty
+version=v1.0.0
+-- want --
+valid:
+
+omitted:
+
+invalid:
diff --git a/zip/testdata/check_zip/various.txt b/zip/testdata/check_zip/various.txt
new file mode 100644
index 0000000..13d406f
--- /dev/null
+++ b/zip/testdata/check_zip/various.txt
@@ -0,0 +1,21 @@
+path=example.com/various
+version=v1.0.0
+-- want --
+valid:
+example.com/various@v1.0.0/valid.go
+
+omitted:
+
+invalid:
+noprefix: path does not have prefix "example.com/various@v1.0.0/"
+example.com/various@v1.0.0/not/../clean: file path is not clean
+example.com/various@v1.0.0/invalid.go': malformed file path "invalid.go'": invalid char '\''
+example.com/various@v1.0.0/GO.MOD: go.mod files must have lowercase names
+example.com/various@v1.0.0/valid.go: multiple entries for file "valid.go"
+-- noprefix --
+-- example.com/various@v1.0.0/valid.go --
+-- example.com/various@v1.0.0/not/../clean --
+-- example.com/various@v1.0.0/invalid.go' --
+-- example.com/various@v1.0.0/GO.MOD --
+-- example.com/various@v1.0.0/valid.go --
+duplicate
diff --git a/zip/testdata/create/bad_gomod_case.txt b/zip/testdata/create/bad_gomod_case.txt
index 0a05278..76e7299 100644
--- a/zip/testdata/create/bad_gomod_case.txt
+++ b/zip/testdata/create/bad_gomod_case.txt
@@ -1,5 +1,5 @@
path=example.com/m
version=v1.0.0
-wantErr=found file named GO.MOD, want all lower-case go.mod
+wantErr=GO.MOD: go.mod files must have lowercase names
-- GO.MOD --
module example.com/m
diff --git a/zip/testdata/create_from_dir/bad_gomod_case.txt b/zip/testdata/create_from_dir/bad_gomod_case.txt
index 0a05278..76e7299 100644
--- a/zip/testdata/create_from_dir/bad_gomod_case.txt
+++ b/zip/testdata/create_from_dir/bad_gomod_case.txt
@@ -1,5 +1,5 @@
path=example.com/m
version=v1.0.0
-wantErr=found file named GO.MOD, want all lower-case go.mod
+wantErr=GO.MOD: go.mod files must have lowercase names
-- GO.MOD --
module example.com/m
diff --git a/zip/testdata/unzip/bad_gomod_case.txt b/zip/testdata/unzip/bad_gomod_case.txt
index 9c40d51..19400b9 100644
--- a/zip/testdata/unzip/bad_gomod_case.txt
+++ b/zip/testdata/unzip/bad_gomod_case.txt
@@ -1,5 +1,5 @@
path=example.com/m
version=v1.0.0
-wantErr=found file named example.com/m@v1.0.0/GO.MOD, want all lower-case go.mod
+wantErr=go.mod files must have lowercase names
-- example.com/m@v1.0.0/GO.MOD --
module example.com/m
diff --git a/zip/testdata/unzip/bad_submodule.txt b/zip/testdata/unzip/bad_submodule.txt
index 3d35010..83d10a9 100644
--- a/zip/testdata/unzip/bad_submodule.txt
+++ b/zip/testdata/unzip/bad_submodule.txt
@@ -1,6 +1,6 @@
path=example.com/m
version=v1.0.0
-wantErr=found go.mod file not in module root directory
+wantErr=go.mod file not in module root directory
-- example.com/m@v1.0.0/go.mod --
module example.com/m
diff --git a/zip/testdata/unzip/cap_go_mod_not_submodule.txt b/zip/testdata/unzip/cap_go_mod_not_submodule.txt
index a02bee4..2a95d10 100644
--- a/zip/testdata/unzip/cap_go_mod_not_submodule.txt
+++ b/zip/testdata/unzip/cap_go_mod_not_submodule.txt
@@ -1,6 +1,6 @@
path=example.com/m
version=v1.0.0
-wantErr=found go.mod file not in module root directory
+wantErr=go.mod file not in module root directory
-- example.com/m@v1.0.0/a.go --
package a
-- example.com/m@v1.0.0/b/GO.MOD --
diff --git a/zip/testdata/unzip/prefix_only.txt b/zip/testdata/unzip/prefix_only.txt
index 7c9f252..775cb37 100644
--- a/zip/testdata/unzip/prefix_only.txt
+++ b/zip/testdata/unzip/prefix_only.txt
@@ -1,6 +1,6 @@
path=example.com/m
version=v1.0.0
-wantErr=unexpected file name example.com/m@v1.0.0
+wantErr=example.com/m@v1.0.0: path does not have prefix "example.com/m@v1.0.0/"
-- example.com/m@v1.0.0 --
-- example.com/m@v1.0.0/go.mod --
module example.com/m
diff --git a/zip/zip.go b/zip/zip.go
index 6865895..5b401ad 100644
--- a/zip/zip.go
+++ b/zip/zip.go
@@ -48,6 +48,7 @@
import (
"archive/zip"
"bytes"
+ "errors"
"fmt"
"io"
"io/ioutil"
@@ -92,6 +93,381 @@
Open() (io.ReadCloser, error)
}
+// CheckedFiles reports whether a set of files satisfy the name and size
+// constraints required by module zip files. The constraints are listed in the
+// package documentation.
+//
+// Functions that produce this report may include slightly different sets of
+// files. See documentation for CheckFiles, CheckDir, and CheckZip for details.
+type CheckedFiles struct {
+ // Valid is a list of file paths that should be included in a zip file.
+ Valid []string
+
+ // Omitted is a list of files that are ignored when creating a module zip
+ // file, along with the reason each file is ignored.
+ Omitted []FileError
+
+ // Invalid is a list of files that should not be included in a module zip
+ // file, along with the reason each file is invalid.
+ Invalid []FileError
+
+ // SizeError is non-nil if the total uncompressed size of the valid files
+ // exceeds the module zip size limit or if the zip file itself exceeds the
+ // limit.
+ SizeError error
+}
+
+// Err returns an error if CheckedFiles does not describe a valid module zip
+// file. SizeError is returned if that field is set. A FileErrorList is returned
+// if there are one or more invalid files. Other errors may be returned in the
+// future.
+func (cf CheckedFiles) Err() error {
+ if cf.SizeError != nil {
+ return cf.SizeError
+ }
+ if len(cf.Invalid) > 0 {
+ return FileErrorList(cf.Invalid)
+ }
+ return nil
+}
+
+type FileErrorList []FileError
+
+func (el FileErrorList) Error() string {
+ buf := &strings.Builder{}
+ sep := ""
+ for _, e := range el {
+ buf.WriteString(sep)
+ buf.WriteString(e.Error())
+ sep = "\n"
+ }
+ return buf.String()
+}
+
+type FileError struct {
+ Path string
+ Err error
+}
+
+func (e FileError) Error() string {
+ return fmt.Sprintf("%s: %s", e.Path, e.Err)
+}
+
+func (e FileError) Unwrap() error {
+ return e.Err
+}
+
+var (
+ // Predefined error messages for invalid files. Not exhaustive.
+ errPathNotClean = errors.New("file path is not clean")
+ errPathNotRelative = errors.New("file path is not relative")
+ errGoModCase = errors.New("go.mod files must have lowercase names")
+ errGoModSize = fmt.Errorf("go.mod file too large (max size is %d bytes)", MaxGoMod)
+ errLICENSESize = fmt.Errorf("LICENSE file too large (max size is %d bytes)", MaxLICENSE)
+
+ // Predefined error messages for omitted files. Not exhaustive.
+ errVCS = errors.New("directory is a version control repository")
+ errVendored = errors.New("file is in vendor directory")
+ errSubmoduleFile = errors.New("file is in another module")
+ errSubmoduleDir = errors.New("directory is in another module")
+ errHgArchivalTxt = errors.New("file is inserted by 'hg archive' and is always omitted")
+ errSymlink = errors.New("file is a symbolic link")
+ errNotRegular = errors.New("not a regular file")
+)
+
+// CheckFiles reports whether a list of files satisfy the name and size
+// constraints listed in the package documentation. The returned CheckedFiles
+// record contains lists of valid, invalid, and omitted files. Every file in
+// the given list will be included in exactly one of those lists.
+//
+// CheckFiles returns an error if the returned CheckedFiles does not describe
+// a valid module zip file (according to CheckedFiles.Err). The returned
+// CheckedFiles is still populated when an error is returned.
+//
+// Note that CheckFiles will not open any files, so Create may still fail when
+// CheckFiles is successful due to I/O errors and reported size differences.
+func CheckFiles(files []File) (CheckedFiles, error) {
+ cf, _, _ := checkFiles(files)
+ return cf, cf.Err()
+}
+
+// checkFiles implements CheckFiles and also returns lists of valid files and
+// their sizes, corresponding to cf.Valid. These lists are used in Crewate to
+// avoid repeated calls to File.Lstat.
+func checkFiles(files []File) (cf CheckedFiles, validFiles []File, validSizes []int64) {
+ errPaths := make(map[string]struct{})
+ addError := func(path string, omitted bool, err error) {
+ if _, ok := errPaths[path]; ok {
+ return
+ }
+ errPaths[path] = struct{}{}
+ fe := FileError{Path: path, Err: err}
+ if omitted {
+ cf.Omitted = append(cf.Omitted, fe)
+ } else {
+ cf.Invalid = append(cf.Invalid, fe)
+ }
+ }
+
+ // Find directories containing go.mod files (other than the root).
+ // Files in these directories will be omitted.
+ // These directories will not be included in the output zip.
+ haveGoMod := make(map[string]bool)
+ for _, f := range files {
+ p := f.Path()
+ dir, base := path.Split(p)
+ if strings.EqualFold(base, "go.mod") {
+ info, err := f.Lstat()
+ if err != nil {
+ addError(p, false, err)
+ continue
+ }
+ if info.Mode().IsRegular() {
+ haveGoMod[dir] = true
+ }
+ }
+ }
+
+ inSubmodule := func(p string) bool {
+ for {
+ dir, _ := path.Split(p)
+ if dir == "" {
+ return false
+ }
+ if haveGoMod[dir] {
+ return true
+ }
+ p = dir[:len(dir)-1]
+ }
+ }
+
+ collisions := make(collisionChecker)
+ maxSize := int64(MaxZipFile)
+ for _, f := range files {
+ p := f.Path()
+ if p != path.Clean(p) {
+ addError(p, false, errPathNotClean)
+ continue
+ }
+ if path.IsAbs(p) {
+ addError(p, false, errPathNotRelative)
+ continue
+ }
+ if isVendoredPackage(p) {
+ addError(p, true, errVendored)
+ continue
+ }
+ if inSubmodule(p) {
+ addError(p, true, errSubmoduleFile)
+ continue
+ }
+ if p == ".hg_archival.txt" {
+ // Inserted by hg archive.
+ // The go command drops this regardless of the VCS being used.
+ addError(p, true, errHgArchivalTxt)
+ continue
+ }
+ if err := module.CheckFilePath(p); err != nil {
+ addError(p, false, err)
+ continue
+ }
+ if strings.ToLower(p) == "go.mod" && p != "go.mod" {
+ addError(p, false, errGoModCase)
+ continue
+ }
+ info, err := f.Lstat()
+ if err != nil {
+ addError(p, false, err)
+ continue
+ }
+ if err := collisions.check(p, info.IsDir()); err != nil {
+ addError(p, false, err)
+ continue
+ }
+ if info.Mode()&os.ModeType == os.ModeSymlink {
+ // Skip symbolic links (golang.org/issue/27093).
+ addError(p, true, errSymlink)
+ continue
+ }
+ if !info.Mode().IsRegular() {
+ addError(p, true, errNotRegular)
+ continue
+ }
+ size := info.Size()
+ if size >= 0 && size <= maxSize {
+ maxSize -= size
+ } else if cf.SizeError == nil {
+ cf.SizeError = fmt.Errorf("module source tree too large (max size is %d bytes)", MaxZipFile)
+ }
+ if p == "go.mod" && size > MaxGoMod {
+ addError(p, false, errGoModSize)
+ continue
+ }
+ if p == "LICENSE" && size > MaxLICENSE {
+ addError(p, false, errLICENSESize)
+ continue
+ }
+
+ cf.Valid = append(cf.Valid, p)
+ validFiles = append(validFiles, f)
+ validSizes = append(validSizes, info.Size())
+ }
+
+ return cf, validFiles, validSizes
+}
+
+// CheckDir reports whether the files in dir satisfy the name and size
+// constraints listed in the package documentation. The returned CheckedFiles
+// record contains lists of valid, invalid, and omitted files. If a directory is
+// omitted (for example, a nested module or vendor directory), it will appear in
+// the omitted list, but its files won't be listed.
+//
+// CheckDir returns an error if it encounters an I/O error or if the returned
+// CheckedFiles does not describe a valid module zip file (according to
+// CheckedFiles.Err). The returned CheckedFiles is still populated when such
+// an error is returned.
+//
+// Note that CheckDir will not open any files, so CreateFromDir may still fail
+// when CheckDir is successful due to I/O errors.
+func CheckDir(dir string) (CheckedFiles, error) {
+ // List files (as CreateFromDir would) and check which ones are omitted
+ // or invalid.
+ files, omitted, err := listFilesInDir(dir)
+ if err != nil {
+ return CheckedFiles{}, err
+ }
+ cf, cfErr := CheckFiles(files)
+ _ = cfErr // ignore this error; we'll generate our own after rewriting paths.
+
+ // Replace all paths with file system paths.
+ // Paths returned by CheckFiles will be slash-separated paths relative to dir.
+ // That's probably not appropriate for error messages.
+ for i := range cf.Valid {
+ cf.Valid[i] = filepath.Join(dir, cf.Valid[i])
+ }
+ cf.Omitted = append(cf.Omitted, omitted...)
+ for i := range cf.Omitted {
+ cf.Omitted[i].Path = filepath.Join(dir, cf.Omitted[i].Path)
+ }
+ for i := range cf.Invalid {
+ cf.Invalid[i].Path = filepath.Join(dir, cf.Invalid[i].Path)
+ }
+ return cf, cf.Err()
+}
+
+// CheckZip reports whether the files contained in a zip file satisfy the name
+// and size constraints listed in the package documentation.
+//
+// CheckZip returns an error if the returned CheckedFiles does not describe
+// a valid module zip file (according to CheckedFiles.Err). The returned
+// CheckedFiles is still populated when an error is returned. CheckZip will
+// also return an error if the module path or version is malformed or if it
+// encounters an error reading the zip file.
+//
+// Note that CheckZip does not read individual files, so Unzip may still fail
+// when CheckZip is successful due to I/O errors.
+func CheckZip(m module.Version, zipFile string) (CheckedFiles, error) {
+ f, err := os.Open(zipFile)
+ if err != nil {
+ return CheckedFiles{}, err
+ }
+ defer f.Close()
+ _, cf, err := checkZip(m, f)
+ return cf, err
+}
+
+// checkZip implements checkZip and also returns the *zip.Reader. This is
+// used in Unzip to avoid redundant I/O.
+func checkZip(m module.Version, f *os.File) (*zip.Reader, CheckedFiles, error) {
+ // Make sure the module path and version are valid.
+ if vers := module.CanonicalVersion(m.Version); vers != m.Version {
+ return nil, CheckedFiles{}, fmt.Errorf("version %q is not canonical (should be %q)", m.Version, vers)
+ }
+ if err := module.Check(m.Path, m.Version); err != nil {
+ return nil, CheckedFiles{}, err
+ }
+
+ // Check the total file size.
+ info, err := f.Stat()
+ if err != nil {
+ return nil, CheckedFiles{}, err
+ }
+ zipSize := info.Size()
+ if zipSize > MaxZipFile {
+ cf := CheckedFiles{SizeError: fmt.Errorf("module zip file is too large (%d bytes; limit is %d bytes)", zipSize, MaxZipFile)}
+ return nil, cf, cf.Err()
+ }
+
+ // Check for valid file names, collisions.
+ var cf CheckedFiles
+ addError := func(zf *zip.File, err error) {
+ cf.Invalid = append(cf.Invalid, FileError{Path: zf.Name, Err: err})
+ }
+ z, err := zip.NewReader(f, zipSize)
+ if err != nil {
+ return nil, CheckedFiles{}, err
+ }
+ prefix := fmt.Sprintf("%s@%s/", m.Path, m.Version)
+ collisions := make(collisionChecker)
+ var size int64
+ for _, zf := range z.File {
+ if !strings.HasPrefix(zf.Name, prefix) {
+ addError(zf, fmt.Errorf("path does not have prefix %q", prefix))
+ continue
+ }
+ name := zf.Name[len(prefix):]
+ if name == "" {
+ continue
+ }
+ isDir := strings.HasSuffix(name, "/")
+ if isDir {
+ name = name[:len(name)-1]
+ }
+ if path.Clean(name) != name {
+ addError(zf, errPathNotClean)
+ continue
+ }
+ if err := module.CheckFilePath(name); err != nil {
+ addError(zf, err)
+ continue
+ }
+ if err := collisions.check(name, isDir); err != nil {
+ addError(zf, err)
+ continue
+ }
+ if isDir {
+ continue
+ }
+ if base := path.Base(name); strings.EqualFold(base, "go.mod") {
+ if base != name {
+ addError(zf, fmt.Errorf("go.mod file not in module root directory"))
+ continue
+ }
+ if name != "go.mod" {
+ addError(zf, errGoModCase)
+ continue
+ }
+ }
+ sz := int64(zf.UncompressedSize64)
+ if sz >= 0 && MaxZipFile-size >= sz {
+ size += sz
+ } else if cf.SizeError == nil {
+ cf.SizeError = fmt.Errorf("total uncompressed size of module contents too large (max size is %d bytes)", MaxZipFile)
+ }
+ if name == "go.mod" && sz > MaxGoMod {
+ addError(zf, fmt.Errorf("go.mod file too large (max size is %d bytes)", MaxGoMod))
+ continue
+ }
+ if name == "LICENSE" && sz > MaxLICENSE {
+ addError(zf, fmt.Errorf("LICENSE file too large (max size is %d bytes)", MaxLICENSE))
+ continue
+ }
+ cf.Valid = append(cf.Valid, zf.Name)
+ }
+
+ return z, cf, cf.Err()
+}
+
// Create builds a zip archive for module m from an abstract list of files
// and writes it to w.
//
@@ -117,33 +493,11 @@
return err
}
- // Find directories containing go.mod files (other than the root).
- // These directories will not be included in the output zip.
- haveGoMod := make(map[string]bool)
- for _, f := range files {
- dir, base := path.Split(f.Path())
- if strings.EqualFold(base, "go.mod") {
- info, err := f.Lstat()
- if err != nil {
- return err
- }
- if info.Mode().IsRegular() {
- haveGoMod[dir] = true
- }
- }
- }
-
- inSubmodule := func(p string) bool {
- for {
- dir, _ := path.Split(p)
- if dir == "" {
- return false
- }
- if haveGoMod[dir] {
- return true
- }
- p = dir[:len(dir)-1]
- }
+ // Check whether files are valid, not valid, or should be omitted.
+ // Also check that the valid files don't exceed the maximum size.
+ cf, validFiles, validSizes := checkFiles(files)
+ if err := cf.Err(); err != nil {
+ return err
}
// Create the module zip file.
@@ -170,53 +524,9 @@
return nil
}
- collisions := make(collisionChecker)
- maxSize := int64(MaxZipFile)
- for _, f := range files {
+ for i, f := range validFiles {
p := f.Path()
- if p != path.Clean(p) {
- return fmt.Errorf("file path %s is not clean", p)
- }
- if path.IsAbs(p) {
- return fmt.Errorf("file path %s is not relative", p)
- }
- if isVendoredPackage(p) || inSubmodule(p) {
- continue
- }
- if p == ".hg_archival.txt" {
- // Inserted by hg archive.
- // The go command drops this regardless of the VCS being used.
- continue
- }
- if err := module.CheckFilePath(p); err != nil {
- return err
- }
- if strings.ToLower(p) == "go.mod" && p != "go.mod" {
- return fmt.Errorf("found file named %s, want all lower-case go.mod", p)
- }
- info, err := f.Lstat()
- if err != nil {
- return err
- }
- if err := collisions.check(p, info.IsDir()); err != nil {
- return err
- }
- if !info.Mode().IsRegular() {
- // Skip symbolic links (golang.org/issue/27093).
- continue
- }
- size := info.Size()
- if size < 0 || maxSize < size {
- return fmt.Errorf("module source tree too large (max size is %d bytes)", MaxZipFile)
- }
- maxSize -= size
- if p == "go.mod" && size > MaxGoMod {
- return fmt.Errorf("go.mod file too large (max size is %d bytes)", MaxGoMod)
- }
- if p == "LICENSE" && size > MaxLICENSE {
- return fmt.Errorf("LICENSE file too large (max size is %d bytes)", MaxLICENSE)
- }
-
+ size := validSizes[i]
if err := addFile(f, p, size); err != nil {
return err
}
@@ -245,61 +555,7 @@
}
}()
- var files []File
- err = filepath.Walk(dir, func(filePath string, info os.FileInfo, err error) error {
- if err != nil {
- return err
- }
- relPath, err := filepath.Rel(dir, filePath)
- if err != nil {
- return err
- }
- slashPath := filepath.ToSlash(relPath)
-
- if info.IsDir() {
- if filePath == dir {
- // Don't skip the top-level directory.
- return nil
- }
-
- // Skip VCS directories.
- // fossil repos are regular files with arbitrary names, so we don't try
- // to exclude them.
- switch filepath.Base(filePath) {
- case ".bzr", ".git", ".hg", ".svn":
- return filepath.SkipDir
- }
-
- // Skip some subdirectories inside vendor, but maintain bug
- // golang.org/issue/31562, described in isVendoredPackage.
- // We would like Create and CreateFromDir to produce the same result
- // for a set of files, whether expressed as a directory tree or zip.
- if isVendoredPackage(slashPath) {
- return filepath.SkipDir
- }
-
- // Skip submodules (directories containing go.mod files).
- if goModInfo, err := os.Lstat(filepath.Join(filePath, "go.mod")); err == nil && !goModInfo.IsDir() {
- return filepath.SkipDir
- }
- return nil
- }
-
- if info.Mode().IsRegular() {
- if !isVendoredPackage(slashPath) {
- files = append(files, dirFile{
- filePath: filePath,
- slashPath: slashPath,
- info: info,
- })
- }
- return nil
- }
-
- // Not a regular file or a directory. Probably a symbolic link.
- // Irregular files are ignored, so skip it.
- return nil
- })
+ files, _, err := listFilesInDir(dir)
if err != nil {
return err
}
@@ -356,89 +612,28 @@
}
}()
- if vers := module.CanonicalVersion(m.Version); vers != m.Version {
- return fmt.Errorf("version %q is not canonical (should be %q)", m.Version, vers)
- }
- if err := module.Check(m.Path, m.Version); err != nil {
- return err
- }
-
// Check that the directory is empty. Don't create it yet in case there's
// an error reading the zip.
- files, _ := ioutil.ReadDir(dir)
- if len(files) > 0 {
+ if files, _ := ioutil.ReadDir(dir); len(files) > 0 {
return fmt.Errorf("target directory %v exists and is not empty", dir)
}
- // Open the zip file and ensure it's under the size limit.
+ // Open the zip and check that it satisfies all restrictions.
f, err := os.Open(zipFile)
if err != nil {
return err
}
defer f.Close()
- info, err := f.Stat()
+ z, cf, err := checkZip(m, f)
if err != nil {
return err
}
- zipSize := info.Size()
- if zipSize > MaxZipFile {
- return fmt.Errorf("module zip file is too large (%d bytes; limit is %d bytes)", zipSize, MaxZipFile)
- }
-
- z, err := zip.NewReader(f, zipSize)
- if err != nil {
+ if err := cf.Err(); err != nil {
return err
}
- // Check total size, valid file names.
- collisions := make(collisionChecker)
+ // Unzip, enforcing sizes declared in the zip file.
prefix := fmt.Sprintf("%s@%s/", m.Path, m.Version)
- var size int64
- for _, zf := range z.File {
- if !strings.HasPrefix(zf.Name, prefix) {
- return fmt.Errorf("unexpected file name %s", zf.Name)
- }
- name := zf.Name[len(prefix):]
- if name == "" {
- continue
- }
- isDir := strings.HasSuffix(name, "/")
- if isDir {
- name = name[:len(name)-1]
- }
- if path.Clean(name) != name {
- return fmt.Errorf("invalid file name %s", zf.Name)
- }
- if err := module.CheckFilePath(name); err != nil {
- return err
- }
- if err := collisions.check(name, isDir); err != nil {
- return err
- }
- if isDir {
- continue
- }
- if base := path.Base(name); strings.EqualFold(base, "go.mod") {
- if base != name {
- return fmt.Errorf("found go.mod file not in module root directory (%s)", zf.Name)
- } else if name != "go.mod" {
- return fmt.Errorf("found file named %s, want all lower-case go.mod", zf.Name)
- }
- }
- s := int64(zf.UncompressedSize64)
- if s < 0 || MaxZipFile-size < s {
- return fmt.Errorf("total uncompressed size of module contents too large (max size is %d bytes)", MaxZipFile)
- }
- size += s
- if name == "go.mod" && s > MaxGoMod {
- return fmt.Errorf("go.mod file too large (max size is %d bytes)", MaxGoMod)
- }
- if name == "LICENSE" && s > MaxLICENSE {
- return fmt.Errorf("LICENSE file too large (max size is %d bytes)", MaxLICENSE)
- }
- }
-
- // Unzip, enforcing sizes checked earlier.
if err := os.MkdirAll(dir, 0777); err != nil {
return err
}
@@ -515,6 +710,72 @@
return nil
}
+// listFilesInDir walks the directory tree rooted at dir and returns a list of
+// files, as well as a list of directories and files that were skipped (for
+// example, nested modules and symbolic links).
+func listFilesInDir(dir string) (files []File, omitted []FileError, err error) {
+ err = filepath.Walk(dir, func(filePath string, info os.FileInfo, err error) error {
+ if err != nil {
+ return err
+ }
+ relPath, err := filepath.Rel(dir, filePath)
+ if err != nil {
+ return err
+ }
+ slashPath := filepath.ToSlash(relPath)
+
+ // Skip some subdirectories inside vendor, but maintain bug
+ // golang.org/issue/31562, described in isVendoredPackage.
+ // We would like Create and CreateFromDir to produce the same result
+ // for a set of files, whether expressed as a directory tree or zip.
+ if isVendoredPackage(slashPath) {
+ omitted = append(omitted, FileError{Path: slashPath, Err: errVendored})
+ return nil
+ }
+
+ if info.IsDir() {
+ if filePath == dir {
+ // Don't skip the top-level directory.
+ return nil
+ }
+
+ // Skip VCS directories.
+ // fossil repos are regular files with arbitrary names, so we don't try
+ // to exclude them.
+ switch filepath.Base(filePath) {
+ case ".bzr", ".git", ".hg", ".svn":
+ omitted = append(omitted, FileError{Path: slashPath, Err: errVCS})
+ return filepath.SkipDir
+ }
+
+ // Skip submodules (directories containing go.mod files).
+ if goModInfo, err := os.Lstat(filepath.Join(filePath, "go.mod")); err == nil && !goModInfo.IsDir() {
+ omitted = append(omitted, FileError{Path: slashPath, Err: errSubmoduleDir})
+ return filepath.SkipDir
+ }
+ return nil
+ }
+
+ // Skip irregular files and files in vendor directories.
+ // Irregular files are ignored. They're typically symbolic links.
+ if !info.Mode().IsRegular() {
+ omitted = append(omitted, FileError{Path: slashPath, Err: errNotRegular})
+ return nil
+ }
+
+ files = append(files, dirFile{
+ filePath: filePath,
+ slashPath: slashPath,
+ info: info,
+ })
+ return nil
+ })
+ if err != nil {
+ return nil, nil, err
+ }
+ return files, omitted, nil
+}
+
type zipError struct {
verb, path string
err error
diff --git a/zip/zip_test.go b/zip/zip_test.go
index fbe2626..2444de3 100644
--- a/zip/zip_test.go
+++ b/zip/zip_test.go
@@ -78,6 +78,57 @@
return test, nil
}
+func extractTxtarToTempDir(arc *txtar.Archive) (dir string, err error) {
+ dir, err = ioutil.TempDir("", "zip_test-*")
+ if err != nil {
+ return "", err
+ }
+ defer func() {
+ if err != nil {
+ os.RemoveAll(dir)
+ }
+ }()
+ for _, f := range arc.Files {
+ filePath := filepath.Join(dir, f.Name)
+ if err := os.MkdirAll(filepath.Dir(filePath), 0777); err != nil {
+ return "", err
+ }
+ if err := ioutil.WriteFile(filePath, f.Data, 0666); err != nil {
+ return "", err
+ }
+ }
+ return dir, nil
+}
+
+func extractTxtarToTempZip(arc *txtar.Archive) (zipPath string, err error) {
+ zipFile, err := ioutil.TempFile("", "zip_test-*.zip")
+ if err != nil {
+ return "", err
+ }
+ defer func() {
+ if cerr := zipFile.Close(); err == nil && cerr != nil {
+ err = cerr
+ }
+ if err != nil {
+ os.Remove(zipFile.Name())
+ }
+ }()
+ zw := zip.NewWriter(zipFile)
+ for _, f := range arc.Files {
+ zf, err := zw.Create(f.Name)
+ if err != nil {
+ return "", err
+ }
+ if _, err := zf.Write(f.Data); err != nil {
+ return "", err
+ }
+ }
+ if err := zw.Close(); err != nil {
+ return "", err
+ }
+ return zipFile.Name(), nil
+}
+
type fakeFile struct {
name string
size uint64
@@ -116,6 +167,211 @@
return len(b), nil
}
+func formatCheckedFiles(cf modzip.CheckedFiles) string {
+ buf := &bytes.Buffer{}
+ fmt.Fprintf(buf, "valid:\n")
+ for _, f := range cf.Valid {
+ fmt.Fprintln(buf, f)
+ }
+ fmt.Fprintf(buf, "\nomitted:\n")
+ for _, f := range cf.Omitted {
+ fmt.Fprintf(buf, "%s: %v\n", f.Path, f.Err)
+ }
+ fmt.Fprintf(buf, "\ninvalid:\n")
+ for _, f := range cf.Invalid {
+ fmt.Fprintf(buf, "%s: %v\n", f.Path, f.Err)
+ }
+ return buf.String()
+}
+
+// TestCheckFiles verifies behavior of CheckFiles. Note that CheckFiles is also
+// covered by TestCreate, TestCreateDir, and TestCreateSizeLimits, so this test
+// focuses on how multiple errors and omissions are reported, rather than trying
+// to cover every case.
+func TestCheckFiles(t *testing.T) {
+ testPaths, err := filepath.Glob(filepath.FromSlash("testdata/check_files/*.txt"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, testPath := range testPaths {
+ testPath := testPath
+ name := strings.TrimSuffix(filepath.Base(testPath), ".txt")
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+
+ // Load the test.
+ test, err := readTest(testPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ files := make([]modzip.File, 0, len(test.archive.Files))
+ var want string
+ for _, tf := range test.archive.Files {
+ if tf.Name == "want" {
+ want = string(tf.Data)
+ continue
+ }
+ files = append(files, fakeFile{
+ name: tf.Name,
+ size: uint64(len(tf.Data)),
+ data: tf.Data,
+ })
+ }
+
+ // Check the files.
+ cf, _ := modzip.CheckFiles(files)
+ got := formatCheckedFiles(cf)
+ if got != want {
+ t.Errorf("got:\n%s\n\nwant:\n%s", got, want)
+ }
+
+ // Check that the error (if any) is just a list of invalid files.
+ // SizeError is not covered in this test.
+ var gotErr, wantErr string
+ if len(cf.Invalid) > 0 {
+ wantErr = modzip.FileErrorList(cf.Invalid).Error()
+ }
+ if err := cf.Err(); err != nil {
+ gotErr = err.Error()
+ }
+ if gotErr != wantErr {
+ t.Errorf("got error:\n%s\n\nwant error:\n%s", gotErr, wantErr)
+ }
+ })
+ }
+}
+
+// TestCheckDir verifies behavior of the CheckDir function. Note that CheckDir
+// relies on CheckFiles and listFilesInDir (called by CreateFromDir), so this
+// test focuses on how multiple errors and omissions are reported, rather than
+// trying to cover every case.
+func TestCheckDir(t *testing.T) {
+ testPaths, err := filepath.Glob(filepath.FromSlash("testdata/check_dir/*.txt"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, testPath := range testPaths {
+ testPath := testPath
+ name := strings.TrimSuffix(filepath.Base(testPath), ".txt")
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+
+ // Load the test and extract the files to a temporary directory.
+ test, err := readTest(testPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ var want string
+ for i, f := range test.archive.Files {
+ if f.Name == "want" {
+ want = string(f.Data)
+ test.archive.Files = append(test.archive.Files[:i], test.archive.Files[i+1:]...)
+ break
+ }
+ }
+ tmpDir, err := extractTxtarToTempDir(test.archive)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer func() {
+ if err := os.RemoveAll(tmpDir); err != nil {
+ t.Errorf("removing temp directory: %v", err)
+ }
+ }()
+
+ // Check the directory.
+ cf, err := modzip.CheckDir(tmpDir)
+ if err != nil && err.Error() != cf.Err().Error() {
+ // I/O error
+ t.Fatal(err)
+ }
+ rep := strings.NewReplacer(tmpDir, "$work", `'\''`, `'\''`, string(os.PathSeparator), "/")
+ got := rep.Replace(formatCheckedFiles(cf))
+ if got != want {
+ t.Errorf("got:\n%s\n\nwant:\n%s", got, want)
+ }
+
+ // Check that the error (if any) is just a list of invalid files.
+ // SizeError is not covered in this test.
+ var gotErr, wantErr string
+ if len(cf.Invalid) > 0 {
+ wantErr = modzip.FileErrorList(cf.Invalid).Error()
+ }
+ if err := cf.Err(); err != nil {
+ gotErr = err.Error()
+ }
+ if gotErr != wantErr {
+ t.Errorf("got error:\n%s\n\nwant error:\n%s", gotErr, wantErr)
+ }
+ })
+ }
+}
+
+// TestCheckZip verifies behavior of CheckZip. Note that CheckZip is also
+// covered by TestUnzip, so this test focuses on how multiple errors are
+// reported, rather than trying to cover every case.
+func TestCheckZip(t *testing.T) {
+ testPaths, err := filepath.Glob(filepath.FromSlash("testdata/check_zip/*.txt"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, testPath := range testPaths {
+ testPath := testPath
+ name := strings.TrimSuffix(filepath.Base(testPath), ".txt")
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+
+ // Load the test and extract the files to a temporary zip file.
+ test, err := readTest(testPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ var want string
+ for i, f := range test.archive.Files {
+ if f.Name == "want" {
+ want = string(f.Data)
+ test.archive.Files = append(test.archive.Files[:i], test.archive.Files[i+1:]...)
+ break
+ }
+ }
+ tmpZipPath, err := extractTxtarToTempZip(test.archive)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer func() {
+ if err := os.Remove(tmpZipPath); err != nil {
+ t.Errorf("removing temp zip file: %v", err)
+ }
+ }()
+
+ // Check the zip.
+ m := module.Version{Path: test.path, Version: test.version}
+ cf, err := modzip.CheckZip(m, tmpZipPath)
+ if err != nil && err.Error() != cf.Err().Error() {
+ // I/O error
+ t.Fatal(err)
+ }
+ got := formatCheckedFiles(cf)
+ if got != want {
+ t.Errorf("got:\n%s\n\nwant:\n%s", got, want)
+ }
+
+ // Check that the error (if any) is just a list of invalid files.
+ // SizeError is not covered in this test.
+ var gotErr, wantErr string
+ if len(cf.Invalid) > 0 {
+ wantErr = modzip.FileErrorList(cf.Invalid).Error()
+ }
+ if err := cf.Err(); err != nil {
+ gotErr = err.Error()
+ }
+ if gotErr != wantErr {
+ t.Errorf("got error:\n%s\n\nwant error:\n%s", gotErr, wantErr)
+ }
+ })
+ }
+}
+
func TestCreate(t *testing.T) {
testDir := filepath.FromSlash("testdata/create")
testInfos, err := ioutil.ReadDir(testDir)
@@ -205,20 +461,15 @@
}
// Write files to a temporary directory.
- tmpDir, err := ioutil.TempDir("", "TestCreateFromDir")
+ tmpDir, err := extractTxtarToTempDir(test.archive)
if err != nil {
t.Fatal(err)
}
- defer os.RemoveAll(tmpDir)
- for _, f := range test.archive.Files {
- filePath := filepath.Join(tmpDir, f.Name)
- if err := os.MkdirAll(filepath.Dir(filePath), 0777); err != nil {
- t.Fatal(err)
+ defer func() {
+ if err := os.RemoveAll(tmpDir); err != nil {
+ t.Errorf("removing temp directory: %v", err)
}
- if err := ioutil.WriteFile(filePath, f.Data, 0666); err != nil {
- t.Fatal(err)
- }
- }
+ }()
// Create zip from the directory.
tmpZip, err := ioutil.TempFile("", "TestCreateFromDir-*.zip")
@@ -353,31 +604,15 @@
}
// Convert txtar to temporary zip file.
- tmpZipFile, err := ioutil.TempFile("", "TestUnzip-*.zip")
+ tmpZipPath, err := extractTxtarToTempZip(test.archive)
if err != nil {
t.Fatal(err)
}
- tmpZipPath := tmpZipFile.Name()
defer func() {
- tmpZipFile.Close()
- os.Remove(tmpZipPath)
+ if err := os.Remove(tmpZipPath); err != nil {
+ t.Errorf("removing temp zip file: %v", err)
+ }
}()
- zw := zip.NewWriter(tmpZipFile)
- for _, f := range test.archive.Files {
- zf, err := zw.Create(f.Name)
- if err != nil {
- t.Fatal(err)
- }
- if _, err := zf.Write(f.Data); err != nil {
- t.Fatal(err)
- }
- }
- if err := zw.Close(); err != nil {
- t.Fatal(err)
- }
- if err := tmpZipFile.Close(); err != nil {
- t.Fatal(err)
- }
// Extract to a temporary directory.
tmpDir, err := ioutil.TempDir("", "TestUnzip")
@@ -410,9 +645,13 @@
}
type sizeLimitTest struct {
- desc string
- files []modzip.File
- wantErr, wantCreateErr, wantUnzipErr string
+ desc string
+ files []modzip.File
+ wantErr string
+ wantCheckFilesErr string
+ wantCreateErr string
+ wantCheckZipErr string
+ wantUnzipErr string
}
// sizeLimitTests is shared by TestCreateSizeLimits and TestUnzipSizeLimits.
@@ -429,8 +668,10 @@
name: "large.go",
size: modzip.MaxZipFile + 1,
}},
- wantCreateErr: "module source tree too large",
- wantUnzipErr: "total uncompressed size of module contents too large",
+ wantCheckFilesErr: "module source tree too large",
+ wantCreateErr: "module source tree too large",
+ wantCheckZipErr: "total uncompressed size of module contents too large",
+ wantUnzipErr: "total uncompressed size of module contents too large",
}, {
desc: "total_large",
files: []modzip.File{
@@ -455,8 +696,10 @@
size: modzip.MaxZipFile - 9,
},
},
- wantCreateErr: "module source tree too large",
- wantUnzipErr: "total uncompressed size of module contents too large",
+ wantCheckFilesErr: "module source tree too large",
+ wantCreateErr: "module source tree too large",
+ wantCheckZipErr: "total uncompressed size of module contents too large",
+ wantUnzipErr: "total uncompressed size of module contents too large",
}, {
desc: "large_gomod",
files: []modzip.File{fakeFile{
@@ -508,23 +751,36 @@
size: 1,
data: []byte(`package large`),
}},
- wantErr: "larger than declared size",
+ wantCreateErr: "larger than declared size",
})
for _, test := range tests {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()
- wantErr := test.wantCreateErr
- if wantErr == "" {
- wantErr = test.wantErr
+
+ wantCheckFilesErr := test.wantCheckFilesErr
+ if wantCheckFilesErr == "" {
+ wantCheckFilesErr = test.wantErr
}
- if err := modzip.Create(ioutil.Discard, sizeLimitVersion, test.files); err == nil && wantErr != "" {
- t.Fatalf("unexpected success; want error containing %q", wantErr)
- } else if err != nil && wantErr == "" {
- t.Fatalf("got error %q; want success", err)
- } else if err != nil && !strings.Contains(err.Error(), wantErr) {
- t.Fatalf("got error %q; want error containing %q", err, wantErr)
+ if _, err := modzip.CheckFiles(test.files); err == nil && wantCheckFilesErr != "" {
+ t.Fatalf("CheckFiles: unexpected success; want error containing %q", wantCheckFilesErr)
+ } else if err != nil && wantCheckFilesErr == "" {
+ t.Fatalf("CheckFiles: got error %q; want success", err)
+ } else if err != nil && !strings.Contains(err.Error(), wantCheckFilesErr) {
+ t.Fatalf("CheckFiles: got error %q; want error containing %q", err, wantCheckFilesErr)
+ }
+
+ wantCreateErr := test.wantCreateErr
+ if wantCreateErr == "" {
+ wantCreateErr = test.wantErr
+ }
+ if err := modzip.Create(ioutil.Discard, sizeLimitVersion, test.files); err == nil && wantCreateErr != "" {
+ t.Fatalf("Create: unexpected success; want error containing %q", wantCreateErr)
+ } else if err != nil && wantCreateErr == "" {
+ t.Fatalf("Create: got error %q; want success", err)
+ } else if err != nil && !strings.Contains(err.Error(), wantCreateErr) {
+ t.Fatalf("Create: got error %q; want error containing %q", err, wantCreateErr)
}
})
}
@@ -545,7 +801,9 @@
tmpZipPath := tmpZipFile.Name()
defer func() {
tmpZipFile.Close()
- os.Remove(tmpZipPath)
+ if err := os.Remove(tmpZipPath); err != nil {
+ t.Errorf("removing temp zip file: %v", err)
+ }
}()
zw := zip.NewWriter(tmpZipFile)
@@ -576,17 +834,38 @@
if err != nil {
t.Fatal(err)
}
- defer os.RemoveAll(tmpDir)
- wantErr := test.wantUnzipErr
- if wantErr == "" {
- wantErr = test.wantErr
+ defer func() {
+ if err := os.RemoveAll(tmpDir); err != nil {
+ t.Errorf("removing temp dir: %v", err)
+ }
+ }()
+
+ wantCheckZipErr := test.wantCheckZipErr
+ if wantCheckZipErr == "" {
+ wantCheckZipErr = test.wantErr
}
- if err := modzip.Unzip(tmpDir, sizeLimitVersion, tmpZipPath); err == nil && wantErr != "" {
- t.Fatalf("unexpected success; want error containing %q", wantErr)
- } else if err != nil && wantErr == "" {
- t.Fatalf("got error %q; want success", err)
- } else if err != nil && !strings.Contains(err.Error(), wantErr) {
- t.Fatalf("got error %q; want error containing %q", err, wantErr)
+ cf, err := modzip.CheckZip(sizeLimitVersion, tmpZipPath)
+ if err == nil {
+ err = cf.Err()
+ }
+ if err == nil && wantCheckZipErr != "" {
+ t.Fatalf("CheckZip: unexpected success; want error containing %q", wantCheckZipErr)
+ } else if err != nil && wantCheckZipErr == "" {
+ t.Fatalf("CheckZip: got error %q; want success", err)
+ } else if err != nil && !strings.Contains(err.Error(), wantCheckZipErr) {
+ t.Fatalf("CheckZip: got error %q; want error containing %q", err, wantCheckZipErr)
+ }
+
+ wantUnzipErr := test.wantUnzipErr
+ if wantUnzipErr == "" {
+ wantUnzipErr = test.wantErr
+ }
+ if err := modzip.Unzip(tmpDir, sizeLimitVersion, tmpZipPath); err == nil && wantUnzipErr != "" {
+ t.Fatalf("Unzip: unexpected success; want error containing %q", wantUnzipErr)
+ } else if err != nil && wantUnzipErr == "" {
+ t.Fatalf("Unzip: got error %q; want success", err)
+ } else if err != nil && !strings.Contains(err.Error(), wantUnzipErr) {
+ t.Fatalf("Unzip: got error %q; want error containing %q", err, wantUnzipErr)
}
})
}
