From a8914293fbb1328f9a5c931bd51e34f5d6584e17 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sun, 14 Sep 2014 18:13:40 +0300 Subject: [PATCH] Improve ValidateContextDirectory error handling. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Errors sent to the walker callback functions were ignored. This meant that one could get a panic when calling methods on a nil FileInfo object. For example when the file did not exists any more. - Lstat calls inside walker callback are reduntant because walker already calls Lstat and passes the result to the callback. - Error returned from filepath.Rel() can never be EACCES because it compares strings and does not care about actual files. - If Matched() returns error then ValidateContextDirectory() must return error. Currently it still kept walking although the outcome was already known. - Function will now fail in case of unknown error(not EACCES nor ENOENT). Previous implementation did not make a clear decision about this (but panicked because of the issues above). Signed-off-by: Tõnis Tiigi (github: tonistiigi) --- utils/utils.go | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index f9b41c0bbd..eb2f31cecd 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -523,48 +523,44 @@ func TreeSize(dir string) (size int64, err error) { // can be read and returns an error if some files can't be read // symlinks which point to non-existing files don't trigger an error func ValidateContextDirectory(srcPath string, excludes []string) error { - var finalError error - - filepath.Walk(filepath.Join(srcPath, "."), func(filePath string, f os.FileInfo, err error) error { + return filepath.Walk(filepath.Join(srcPath, "."), func(filePath string, f os.FileInfo, err error) error { // skip this directory/file if it's not in the path, it won't get added to the context - relFilePath, err := filepath.Rel(srcPath, filePath) - if err != nil && os.IsPermission(err) { - return nil - } - - skip, err := Matches(relFilePath, excludes) - if err != nil { - finalError = err - } - if skip { + if relFilePath, err := filepath.Rel(srcPath, filePath); err != nil { + return err + } else if skip, err := Matches(relFilePath, excludes); err != nil { + return err + } else if skip { if f.IsDir() { return filepath.SkipDir } return nil } - if _, err := os.Stat(filePath); err != nil && os.IsPermission(err) { - finalError = fmt.Errorf("can't stat '%s'", filePath) + if err != nil { + if os.IsPermission(err) { + return fmt.Errorf("can't stat '%s'", filePath) + } + if os.IsNotExist(err) { + return nil + } return err } + // skip checking if symlinks point to non-existing files, such symlinks can be useful // also skip named pipes, because they hanging on open - lstat, _ := os.Lstat(filePath) - if lstat != nil && lstat.Mode()&(os.ModeSymlink|os.ModeNamedPipe) != 0 { + if f.Mode()&(os.ModeSymlink|os.ModeNamedPipe) != 0 { return nil } if !f.IsDir() { currentFile, err := os.Open(filePath) if err != nil && os.IsPermission(err) { - finalError = fmt.Errorf("no permission to read from '%s'", filePath) - return err + return fmt.Errorf("no permission to read from '%s'", filePath) } currentFile.Close() } return nil }) - return finalError } func StringsContainsNoCase(slice []string, s string) bool {