From 9e0b924d57dd6d296472d6af39878437dd7e10bb Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 28 Sep 2020 10:22:50 +0200 Subject: [PATCH] lib/fs: Be more clear about invalid file names (ref #7010) (#7011) Add specific errors for the failures, resulting in this rather than just the generic "invalid filename": [MRIW7] 08:50:50 INFO: Puller (folder default, item "NUL"): syncing: filename is invalid: name is reserved [MRIW7] 08:50:50 INFO: Puller (folder default, item "fail."): syncing: filename is invalid: name ends with space or period [MRIW7] 08:50:50 INFO: Puller (folder default, item "sup:yo"): syncing: filename is invalid: name contains reserved character [MRIW7] 08:50:50 INFO: default: Failed to sync 3 items --- lib/fs/basicfs.go | 9 ++++++--- lib/fs/filesystem.go | 6 +++--- lib/fs/util.go | 17 +++++++++-------- lib/model/folder_sendrecv.go | 7 ++++--- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/lib/fs/basicfs.go b/lib/fs/basicfs.go index 46ad15bf0..cf39985f0 100644 --- a/lib/fs/basicfs.go +++ b/lib/fs/basicfs.go @@ -19,8 +19,11 @@ import ( ) var ( - ErrInvalidFilename = errors.New("filename is invalid") - ErrNotRelative = errors.New("not a relative path") + errInvalidFilenameEmpty = errors.New("name is invalid, must not be empty") + errInvalidFilenameWindowsSpacePeriod = errors.New("name is invalid, must not end in space or period on Windows") + errInvalidFilenameWindowsReservedName = errors.New("name is invalid, contains Windows reserved name (NUL, COM1, etc.)") + errInvalidFilenameWindowsReservedChar = errors.New("name is invalid, contains Windows reserved character (?, *, etc.)") + errNotRelative = errors.New("not a relative path") ) func WithJunctionsAsDirs() Option { @@ -95,7 +98,7 @@ func (f *BasicFilesystem) rooted(rel string) (string, error) { func rooted(rel, root string) (string, error) { // The root must not be empty. if root == "" { - return "", ErrInvalidFilename + return "", errInvalidFilenameEmpty } var err error diff --git a/lib/fs/filesystem.go b/lib/fs/filesystem.go index cb5ac3819..13735927b 100644 --- a/lib/fs/filesystem.go +++ b/lib/fs/filesystem.go @@ -234,7 +234,7 @@ func Canonicalize(file string) (string, error) { // The relative path may pretend to be an absolute path within // the root, but the double path separator on Windows implies // something else and is out of spec. - return "", ErrNotRelative + return "", errNotRelative } // The relative path should be clean from internal dotdots and similar @@ -244,10 +244,10 @@ func Canonicalize(file string) (string, error) { // It is not acceptable to attempt to traverse upwards. switch file { case "..": - return "", ErrNotRelative + return "", errNotRelative } if strings.HasPrefix(file, ".."+pathSep) { - return "", ErrNotRelative + return "", errNotRelative } if strings.HasPrefix(file, pathSep) { diff --git a/lib/fs/util.go b/lib/fs/util.go index 8abfc252c..8203160e6 100644 --- a/lib/fs/util.go +++ b/lib/fs/util.go @@ -7,7 +7,6 @@ package fs import ( - "errors" "fmt" "os" "path/filepath" @@ -15,8 +14,6 @@ import ( "strings" ) -var errNoHome = errors.New("no home directory found - set $HOME (or the platform equivalent)") - func ExpandTilde(path string) (string, error) { if path == "~" { return getHomeDir() @@ -55,7 +52,7 @@ var windowsDisallowedCharacters = string([]rune{ 31, }) -func WindowsInvalidFilename(name string) bool { +func WindowsInvalidFilename(name string) error { // None of the path components should end in space or period, or be a // reserved name. // (https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file) @@ -66,19 +63,23 @@ func WindowsInvalidFilename(name string) bool { switch part[len(part)-1] { case ' ', '.': // Names ending in space or period are not valid. - return true + return errInvalidFilenameWindowsSpacePeriod } - switch part { + switch strings.ToUpper(part) { case "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9": // These reserved names are not valid. - return true + return errInvalidFilenameWindowsReservedName } } // The path must not contain any disallowed characters - return strings.ContainsAny(name, windowsDisallowedCharacters) + if strings.ContainsAny(name, windowsDisallowedCharacters) { + return errInvalidFilenameWindowsReservedChar + } + + return nil } // IsParent compares paths purely lexicographically, meaning it returns false diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go index d02791b51..29433c777 100644 --- a/lib/model/folder_sendrecv.go +++ b/lib/model/folder_sendrecv.go @@ -337,7 +337,7 @@ func (f *sendReceiveFolder) processNeeded(snap *db.Snapshot, dbUpdateChan chan<- l.Debugln(f, "Handling ignored file", file) dbUpdateChan <- dbUpdateJob{file, dbUpdateInvalidate} - case runtime.GOOS == "windows" && fs.WindowsInvalidFilename(file.Name): + case runtime.GOOS == "windows" && fs.WindowsInvalidFilename(file.Name) != nil: if file.IsDeleted() { // Just pretend we deleted it, no reason to create an error // about a deleted file that we can't have anyway. @@ -345,8 +345,9 @@ func (f *sendReceiveFolder) processNeeded(snap *db.Snapshot, dbUpdateChan chan<- // ignored at some point. dbUpdateChan <- dbUpdateJob{file, dbUpdateDeleteFile} } else { - // We can't pull an invalid file. - f.newPullError(file.Name, fs.ErrInvalidFilename) + // We can't pull an invalid file. Grab the error again since + // we couldn't assign it directly in the case clause. + f.newPullError(file.Name, fs.WindowsInvalidFilename(file.Name)) // No reason to retry for this changed-- }