lib/fs: Clarify errors for Windows filenames (fixes #8968) (#8969)

With this change, error messages include the offending characters or
name parts. Examples:

    nul.txt: name is invalid, contains Windows reserved name: "nul"
    foo>bar.txt: name is invalid, contains Windows reserved character: ">"
    foo \bar.txt: name is invalid, must not end in space or period on Windows

3 files changed, 16 insertions, 12 deletions

diff --git a/lib/fs/basicfs.go b/lib/fs/basicfs.go
index 51e598ed2..58afc0e27 100644
--- a/lib/fs/basicfs.go
+++ b/lib/fs/basicfs.go
@@ -22,8 +22,8 @@ import (
 var (
 	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.)")
+	errInvalidFilenameWindowsReservedName = errors.New("name is invalid, contains Windows reserved name")
+	errInvalidFilenameWindowsReservedChar = errors.New("name is invalid, contains Windows reserved character")
 )
 
 type OptionJunctionsAsDirs struct{}
diff --git a/lib/fs/util.go b/lib/fs/util.go
index 9e977cb37..41ee39300 100644
--- a/lib/fs/util.go
+++ b/lib/fs/util.go
@@ -54,8 +54,8 @@ const windowsDisallowedCharacters = (`<>:"|?*` +
 
 func WindowsInvalidFilename(name string) error {
 	// The path must not contain any disallowed characters.
-	if strings.ContainsAny(name, windowsDisallowedCharacters) {
-		return errInvalidFilenameWindowsReservedChar
+	if idx := strings.IndexAny(name, windowsDisallowedCharacters); idx != -1 {
+		return fmt.Errorf("%w: %q", errInvalidFilenameWindowsReservedChar, name[idx:idx+1])
 	}
 
 	// None of the path components should end in space or period, or be a
@@ -72,8 +72,8 @@ func WindowsInvalidFilename(name string) error {
 			// Names ending in space or period are not valid.
 			return errInvalidFilenameWindowsSpacePeriod
 		}
-		if windowsIsReserved(part) {
-			return errInvalidFilenameWindowsReservedName
+		if reserved := windowsReservedNamePart(part); reserved != "" {
+			return fmt.Errorf("%w: %q", errInvalidFilenameWindowsReservedName, reserved)
 		}
 	}
 
@@ -117,13 +117,13 @@ func SanitizePath(path string) string {
 	}
 
 	path = strings.TrimSpace(b.String())
-	if windowsIsReserved(path) {
+	if reserved := windowsReservedNamePart(path); reserved != "" {
 		path = "-" + path
 	}
 	return path
 }
 
-func windowsIsReserved(part string) bool {
+func windowsReservedNamePart(part string) string {
 	// nul.txt.jpg is also disallowed.
 	dot := strings.IndexByte(part, '.')
 	if dot != -1 {
@@ -132,7 +132,7 @@ func windowsIsReserved(part string) bool {
 
 	// Check length to skip allocating ToUpper.
 	if len(part) != 3 && len(part) != 4 {
-		return false
+		return ""
 	}
 
 	// COM0 and LPT0 are missing from the Microsoft docs,
@@ -144,9 +144,9 @@ func windowsIsReserved(part string) bool {
 		"COM5", "COM6", "COM7", "COM8", "COM9",
 		"LPT0", "LPT1", "LPT2", "LPT3", "LPT4",
 		"LPT5", "LPT6", "LPT7", "LPT8", "LPT9":
-		return true
+		return part
 	}
-	return false
+	return ""
 }
 
 // IsParent compares paths purely lexicographically, meaning it returns false
diff --git a/lib/fs/util_test.go b/lib/fs/util_test.go
index 1727044d2..0be64b0f4 100644
--- a/lib/fs/util_test.go
+++ b/lib/fs/util_test.go
@@ -7,6 +7,7 @@
 package fs
 
 import (
+	"errors"
 	"math/rand"
 	"testing"
 	"unicode"
@@ -69,9 +70,10 @@ func TestWindowsInvalidFilename(t *testing.T) {
 
 	for _, tc := range cases {
 		err := WindowsInvalidFilename(tc.name)
-		if err != tc.err {
+		if !errors.Is(err, tc.err) {
 			t.Errorf("For %q, got %v, expected %v", tc.name, err, tc.err)
 		}
+		t.Logf("%s: %v", tc.name, err)
 	}
 }
 
@@ -124,9 +126,11 @@ func benchmarkWindowsInvalidFilename(b *testing.B, name string) {
 		WindowsInvalidFilename(name)
 	}
 }
+
 func BenchmarkWindowsInvalidFilenameValid(b *testing.B) {
 	benchmarkWindowsInvalidFilename(b, "License.txt.gz")
 }
+
 func BenchmarkWindowsInvalidFilenameNUL(b *testing.B) {
 	benchmarkWindowsInvalidFilename(b, "nul.txt.gz")
 }