lib/fs,model: Clarify errors for Android filenames (fixes syncthing#9395

)

Currently, Syncthing tries to sync files to Android devices with no
consideration for its filename restrictions (see [1]). This commit makes
it so that similarly to Windows, Syncthing relies on a hard-coded list
of characters which are invalid on Android and displays a specific error
message when user tries to sync a file with one of the characters in its
name.

[1] https://stackoverflow.com/a/64021421

Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>

lib/fs/basicfs.go

@@ -24,6 +24,7 @@ var (
 	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")
 	errInvalidFilenameWindowsReservedChar = errors.New("name is invalid, contains Windows reserved character")
+	errInvalidFilenameAndroidReservedChar = errors.New("name is invalid, contains Android reserved character")
 )
 
 type OptionJunctionsAsDirs struct{}

lib/fs/util.go

@@ -48,32 +48,40 @@ func getHomeDir() (string, error) {
 	return os.UserHomeDir()
 }
 
-const windowsDisallowedCharacters = (`<>:"|?*` +
+const fatDisallowedCharacters = (`/\<>:"|?*` +
 	"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" +
 	"\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f")
 
-func WindowsInvalidFilename(name string) error {
+func OsInvalidFilename(name string) error {
 	// The path must not contain any disallowed characters.
-	if idx := strings.IndexAny(name, windowsDisallowedCharacters); idx != -1 {
-		return fmt.Errorf("%w: %q", errInvalidFilenameWindowsReservedChar, name[idx:idx+1])
+	var errMsg error
+	if build.IsAndroid {
+		errMsg = errInvalidFilenameAndroidReservedChar
+	} else if build.IsWindows {
+		errMsg = errInvalidFilenameWindowsReservedChar
+	}
+	if idx := strings.IndexAny(name, fatDisallowedCharacters); idx != -1 && errMsg != nil {
+		return fmt.Errorf("%w: %q", errMsg, name[idx:idx+1])
 	}
 
-	// None of the path components should end in space or period, or be a
-	// reserved name.
-	for len(name) > 0 {
-		part, rest, _ := strings.Cut(name, `\`)
-		name = rest
-
-		if part == "" {
-			continue
-		}
-		switch part[len(part)-1] {
-		case ' ', '.':
-			// Names ending in space or period are not valid.
-			return errInvalidFilenameWindowsSpacePeriod
-		}
-		if reserved := windowsReservedNamePart(part); reserved != "" {
-			return fmt.Errorf("%w: %q", errInvalidFilenameWindowsReservedName, reserved)
+	if build.IsWindows {
+		// None of the path components should end in space or period, or be a
+		// reserved name.
+		for len(name) > 0 {
+			part, rest, _ := strings.Cut(name, `\`)
+			name = rest
+
+			if part == "" {
+				continue
+			}
+			switch part[len(part)-1] {
+			case ' ', '.':
+				// Names ending in space or period are not valid.
+				return errInvalidFilenameWindowsSpacePeriod
+			}
+			if reserved := windowsReservedNamePart(part); reserved != "" {
+				return fmt.Errorf("%w: %q", errInvalidFilenameWindowsReservedName, reserved)
+			}
 		}
 	}
 
@@ -102,7 +110,7 @@ func WindowsInvalidFilename(name string) error {
 func SanitizePath(path string) string {
 	var b strings.Builder
 
-	const disallowed = `'/\[]{};:!@$%&^#` + windowsDisallowedCharacters
+	const disallowed = `'[]{};!@$%&^#` + fatDisallowedCharacters
 	prev := ' '
 	for _, c := range path {
 		if !unicode.IsPrint(c) || c == unicode.ReplacementChar ||

lib/fs/util_test.go

@@ -69,7 +69,7 @@ func TestWindowsInvalidFilename(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		err := WindowsInvalidFilename(tc.name)
+		err := OsInvalidFilename(tc.name)
 		if !errors.Is(err, tc.err) {
 			t.Errorf("For %q, got %v, expected %v", tc.name, err, tc.err)
 		}
@@ -123,7 +123,7 @@ func TestSanitizePathFuzz(t *testing.T) {
 
 func benchmarkWindowsInvalidFilename(b *testing.B, name string) {
 	for i := 0; i < b.N; i++ {
-		WindowsInvalidFilename(name)
+		OsInvalidFilename(name)
 	}
 }
 

lib/model/folder_sendrecv.go

@@ -348,7 +348,7 @@ func (f *sendReceiveFolder) processNeeded(snap *db.Snapshot, dbUpdateChan chan<-
 			l.Debugln(f, "Handling ignored file", file)
 			dbUpdateChan <- dbUpdateJob{file, dbUpdateInvalidate}
 
-		case build.IsWindows && fs.WindowsInvalidFilename(file.Name) != nil:
+		case (build.IsAndroid || build.IsWindows) && fs.OsInvalidFilename(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.
@@ -358,7 +358,7 @@ func (f *sendReceiveFolder) processNeeded(snap *db.Snapshot, dbUpdateChan chan<-
 			} else {
 				// 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))
+				f.newPullError(file.Name, fs.OsInvalidFilename(file.Name))
 				// No reason to retry for this
 				changed--
 			}