fix(admin): allow control chars in file paths when browsing filer (#9043)

* fix(admin): allow control chars in file paths when browsing filer

The admin UI rejected any path containing \x00, \r, or \n as "path contains
invalid characters". These bytes are legal in S3 object keys, so objects
created through the S3 API (or replicated via filer.sync) could exist on the
filer but be unreachable from the admin UI — browse, download, and upload
all failed with "Invalid file path".

Drop the control-character rejection and instead URL-escape the path when
constructing filer request URLs, so that such bytes cannot inject into the
HTTP request target. Path traversal protection via path.Clean is unchanged.

* test(admin): strengthen file path tests with byte-preserving checks

Assert full expected output for validateAndCleanFilePath so silent stripping
of control characters would fail the test, and cover \r and \x00 escaping in
filerFileURL in addition to \n and space.

weed/admin/handlers/file_browser_handlers.go

@@ -9,6 +9,7 @@ import (
 	"mime/multipart"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"path"
 	"path/filepath"
@@ -398,7 +399,7 @@ func (h *FileBrowserHandlers) uploadFileToFiler(filePath string, fileHeader *mul
 
 	// Create the upload URL - the httpClient will normalize to the correct scheme (http/https)
 	// based on the https.client configuration in security.toml
-	uploadURL := fmt.Sprintf("%s%s", filerHttpAddress, cleanFilePath)
+	uploadURL := filerFileURL(filerHttpAddress, cleanFilePath)
 
 	// Normalize the URL scheme based on TLS configuration
 	uploadURL, err = h.httpClient.NormalizeHttpScheme(uploadURL)
@@ -503,14 +504,16 @@ func (h *FileBrowserHandlers) validateAndCleanFilePath(filePath string) (string,
 		return "", fmt.Errorf("path traversal not allowed")
 	}
 
-	// Additional validation: ensure path doesn't contain dangerous characters
-	if strings.ContainsAny(cleanPath, "\x00\r\n") {
-		return "", fmt.Errorf("path contains invalid characters")
-	}
-
 	return cleanPath, nil
 }
 
+// filerFileURL joins the filer HTTP address with a validated file path, URL-escaping
+// the path so that control characters and other bytes that are legal in S3 object keys
+// cannot inject into the HTTP request target.
+func filerFileURL(filerHttpAddress, cleanFilePath string) string {
+	return filerHttpAddress + (&url.URL{Path: cleanFilePath}).EscapedPath()
+}
+
 // fetchFileContent fetches file content from the filer and returns the content or an error.
 func (h *FileBrowserHandlers) fetchFileContent(filePath string, timeout time.Duration) (string, error) {
 	filerAddress := h.adminServer.GetFilerAddress()
@@ -529,7 +532,7 @@ func (h *FileBrowserHandlers) fetchFileContent(filePath string, timeout time.Dur
 	}
 
 	// Create the file URL with proper scheme based on TLS configuration
-	fileURL := fmt.Sprintf("%s%s", filerHttpAddress, cleanFilePath)
+	fileURL := filerFileURL(filerHttpAddress, cleanFilePath)
 	fileURL, err = h.httpClient.NormalizeHttpScheme(fileURL)
 	if err != nil {
 		return "", fmt.Errorf("failed to construct file URL: %w", err)
@@ -597,7 +600,7 @@ func (h *FileBrowserHandlers) DownloadFile(w http.ResponseWriter, r *http.Reques
 	}
 
 	// Create the download URL with proper scheme based on TLS configuration
-	downloadURL := fmt.Sprintf("%s%s", filerHttpAddress, cleanFilePath)
+	downloadURL := filerFileURL(filerHttpAddress, cleanFilePath)
 	downloadURL, err = h.httpClient.NormalizeHttpScheme(downloadURL)
 	if err != nil {
 		writeJSONError(w, http.StatusInternalServerError, "Failed to construct download URL: "+err.Error())
@@ -1043,7 +1046,7 @@ func (h *FileBrowserHandlers) isLikelyTextFile(filePath string, maxCheckSize int
 	}
 
 	// Create the file URL with proper scheme based on TLS configuration
-	fileURL := fmt.Sprintf("%s%s", filerHttpAddress, cleanFilePath)
+	fileURL := filerFileURL(filerHttpAddress, cleanFilePath)
 	fileURL, err = h.httpClient.NormalizeHttpScheme(fileURL)
 	if err != nil {
 		glog.Errorf("Failed to normalize URL scheme: %v", err)

weed/admin/handlers/file_browser_handlers_test.go

@@ -0,0 +1,62 @@
+package handlers
+
+import (
+	"testing"
+)
+
+func TestValidateAndCleanFilePath_AllowsControlChars(t *testing.T) {
+	h := &FileBrowserHandlers{}
+
+	// S3 object keys may legally contain any UTF-8 bytes, including control
+	// characters like \n, \r, and \x00. The admin UI must be able to browse
+	// and manage such entries rather than silently stripping or rejecting them.
+	cases := []struct {
+		in   string
+		want string
+	}{
+		{"/buckets/profilebuilder/3testGB.zip\n ", "/buckets/profilebuilder/3testGB.zip\n "},
+		{"/foo\rbar", "/foo\rbar"},
+		{"/foo\x00bar", "/foo\x00bar"},
+		{"/normal/path.txt", "/normal/path.txt"},
+		// Missing leading slash should be added back.
+		{"relative/path.txt", "/relative/path.txt"},
+		// Duplicate slashes should be collapsed by path.Clean.
+		{"/a//b", "/a/b"},
+	}
+	for _, tc := range cases {
+		got, err := h.validateAndCleanFilePath(tc.in)
+		if err != nil {
+			t.Errorf("validateAndCleanFilePath(%q) unexpected error: %v", tc.in, err)
+			continue
+		}
+		if got != tc.want {
+			t.Errorf("validateAndCleanFilePath(%q) = %q, want %q", tc.in, got, tc.want)
+		}
+	}
+}
+
+func TestValidateAndCleanFilePath_RejectsEmpty(t *testing.T) {
+	h := &FileBrowserHandlers{}
+	if _, err := h.validateAndCleanFilePath(""); err == nil {
+		t.Errorf("expected empty path rejection")
+	}
+}
+
+func TestFilerFileURL_EscapesControlChars(t *testing.T) {
+	cases := []struct {
+		addr string
+		path string
+		want string
+	}{
+		{"http://127.0.0.1:8888", "/buckets/profilebuilder/3testGB.zip\n ", "http://127.0.0.1:8888/buckets/profilebuilder/3testGB.zip%0A%20"},
+		{"http://127.0.0.1:8888", "/buckets/profilebuilder/file\rname", "http://127.0.0.1:8888/buckets/profilebuilder/file%0Dname"},
+		{"http://127.0.0.1:8888", "/buckets/profilebuilder/file\x00name", "http://127.0.0.1:8888/buckets/profilebuilder/file%00name"},
+		// Plain path round-trips unchanged.
+		{"http://h:1", "/a/b.txt", "http://h:1/a/b.txt"},
+	}
+	for _, tc := range cases {
+		if got := filerFileURL(tc.addr, tc.path); got != tc.want {
+			t.Errorf("filerFileURL(%q, %q) = %q, want %q", tc.addr, tc.path, got, tc.want)
+		}
+	}
+}