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

@@ -19,8 +19,11 @@ import (
 )
 
 var (
-	ErrInvalidFilename = errors.New("filename is invalid")
+	errInvalidFilenameEmpty               = errors.New("name is invalid, must not be empty")
-	ErrNotRelative     = errors.New("not a relative path")
+	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

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) {

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

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.
+				// We can't pull an invalid file. Grab the error again since
-				f.newPullError(file.Name, fs.ErrInvalidFilename)
+				// 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--
 			}