From 06838e28b2031dc69e3cb17dafd484cf734ad848 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 4 Sep 2026 00:02:33 -0700 Subject: [PATCH] filer: serve "//" paths at the cleaned path instead of redirecting (#11150) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * filer: serve "//" paths at the cleaned path instead of redirecting http.ServeMux redirects a non-canonical path ("//", "..") to its cleaned form, but since Go 1.22 it builds the Location from the already-escaped path, so it is percent-encoded twice (golang/go#79897). A client that follows the redirect re-posts "/负极全景" as "/%25E8%25B4%259F...", and the filer stores a directory literally named "%E8%B4%9F...". Wrap the filer muxes in CleanPathHandler, which rewrites the request to the same cleaned path ServeMux would have redirected to and dispatches directly. The decoded name reaches the handler, the round trip goes away, and clients that do not follow redirects work too. Fixes #11125 * filer: keep RequestURI in step with the cleaned path PostHandler derives storage rules, the bucket and the read-only check from r.RequestURI while writing the entry at r.URL.Path. After CleanPathHandler rewrote only the URL, a "//" or ".." request would be placed by the raw path and written to the cleaned one. Rewrite RequestURI too, as the redirect-following client used to. * filer: match storage rules on the decoded write path PostHandler resolved the storage rule from r.RequestURI, the raw request-target. Clients percent-encode non-ASCII segments on the wire, so a read-only or TTL rule configured on "/data/只读/" never matched a POST to "/data/%E5%8F%AA%E8%AF%BB/" and the write went through. Use r.URL.Path, the decoded path the entry is actually written to, as the header-based destination check already does. The query string no longer reaches the rule lookup, so the "?" trimming in the read-only error is gone. --- weed/command/filer.go | 20 ++-- weed/server/common.go | 45 +++++++++ weed/server/common_test.go | 99 +++++++++++++++++++ weed/server/filer_server_handlers_write.go | 8 +- .../filer_server_storage_rule_ttl_test.go | 35 +++++++ 5 files changed, 197 insertions(+), 10 deletions(-) diff --git a/weed/command/filer.go b/weed/command/filer.go index a50176f0d..66c5a5813 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -400,6 +400,11 @@ func (fo *FilerOptions) startFiler() { glog.Fatalf("Filer startup error: %v", nfs_err) } + // Serve "//" and ".." paths at their cleaned form instead of letting the mux + // redirect: its Location is double-escaped, turning non-ASCII names into + // percent-encoded directory names when a client follows it (#11125). + defaultHandler := weed_server.CleanPathHandler(defaultMux) + // Ensure fs.Shutdown() runs exactly once, whether triggered by a signal hook // or by the main goroutine after Serve() returns (e.g., MiniCluster tests). var shutdownOnce sync.Once @@ -416,14 +421,15 @@ func (fo *FilerOptions) startFiler() { if e != nil { glog.Fatalf("Filer server public listener error on port %d:%v", *fo.publicPort, e) } + publicHandler := weed_server.CleanPathHandler(publicVolumeMux) go func() { - if e := http.Serve(publicListener, publicVolumeMux); e != nil { + if e := http.Serve(publicListener, publicHandler); e != nil { glog.Fatalf("Volume server fail to serve public: %v", e) } }() if localPublicListener != nil { go func() { - if e := http.Serve(localPublicListener, publicVolumeMux); e != nil { + if e := http.Serve(localPublicListener, publicHandler); e != nil { glog.Errorf("Volume server fail to serve public: %v", e) } }() @@ -506,7 +512,7 @@ func (fo *FilerOptions) startFiler() { if err != nil { glog.Fatalf("Failed to listen on %s: %v", localSocket, err) } - socketServer = newHttpServer(defaultMux, nil) + socketServer = newHttpServer(defaultHandler, nil) go socketServer.Serve(filerSocketListener) } @@ -549,14 +555,14 @@ func (fo *FilerOptions) startFiler() { var localTLSServer *http.Server if filerLocalListener != nil { - localTLSServer = newHttpServer(defaultMux, tlsConfig) + localTLSServer = newHttpServer(defaultHandler, tlsConfig) go func() { if err := localTLSServer.ServeTLS(filerLocalListener, "", ""); err != nil { glog.Errorf("Filer Fail to serve: %v", err) } }() } - httpS := newHttpServer(defaultMux, tlsConfig) + httpS := newHttpServer(defaultHandler, tlsConfig) // Register a single shutdown hook that runs the steps in the correct order: // stop accepting new gRPC/HTTP requests, then close the filer database. @@ -600,14 +606,14 @@ func (fo *FilerOptions) startFiler() { } else { var localHTTPServer *http.Server if filerLocalListener != nil { - localHTTPServer = newHttpServer(defaultMux, nil) + localHTTPServer = newHttpServer(defaultHandler, nil) go func() { if err := localHTTPServer.Serve(filerLocalListener); err != nil { glog.Errorf("Filer Fail to serve: %v", err) } }() } - httpS := newHttpServer(defaultMux, nil) + httpS := newHttpServer(defaultHandler, nil) // Register a single shutdown hook that runs the steps in the correct order: // stop accepting new gRPC/HTTP requests, then close the filer database. diff --git a/weed/server/common.go b/weed/server/common.go index c9b87a5a8..8399d927c 100644 --- a/weed/server/common.go +++ b/weed/server/common.go @@ -12,6 +12,8 @@ import ( "mime" "mime/multipart" "net/http" + "net/url" + "path" "path/filepath" "strconv" "strings" @@ -416,6 +418,49 @@ func ProcessRangeRequest(r *http.Request, w http.ResponseWriter, totalSize int64 return nil } +// CleanPathHandler serves a request whose path is not canonical ("//", "." or +// ".." segments) at the cleaned path instead of letting http.ServeMux redirect +// to it. ServeMux builds that redirect from the already percent-encoded path, so +// the Location header is encoded twice (golang/go#79897): a client following it +// re-sends "/负极全景" as "/%25E8%25B4%259F...", and the filer then stores a +// directory literally named "%E8%B4%9F...". Cleaning here mirrors the path +// ServeMux would have redirected to, so the decoded name reaches the handler. +func CleanPathHandler(h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + escaped := r.URL.EscapedPath() + if cleaned := cleanPath(escaped); cleaned != escaped { + if p, err := url.PathUnescape(cleaned); err == nil { + r2 := new(http.Request) + *r2 = *r + r2.URL = new(url.URL) + *r2.URL = *r.URL + r2.URL.Path, r2.URL.RawPath = p, cleaned + // PostHandler picks storage rules and the bucket from RequestURI, so + // keep it in step with the path the entry is written to. + r2.RequestURI = r2.URL.RequestURI() + r = r2 + } + } + h.ServeHTTP(w, r) + }) +} + +// cleanPath is the canonical form http.ServeMux redirects to: path.Clean plus +// the trailing slash, which the filer relies on to tell a directory from a file. +func cleanPath(p string) string { + if p == "" { + return "/" + } + if p[0] != '/' { + p = "/" + p + } + np := path.Clean(p) + if p[len(p)-1] == '/' && np != "/" { + np += "/" + } + return np +} + func requestIDMiddleware(h http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { request_id.Middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/weed/server/common_test.go b/weed/server/common_test.go index ca8bfa86f..79bcad9b9 100644 --- a/weed/server/common_test.go +++ b/weed/server/common_test.go @@ -67,6 +67,105 @@ func TestWriteJsonNoJSONP(t *testing.T) { } } +// A POST to a path with a doubled slash must reach the handler at the cleaned, +// still-decoded path. Without CleanPathHandler, http.ServeMux answers with a +// redirect whose Location percent-encodes the already-escaped path a second +// time (golang/go#79897), and a client following it creates directories +// literally named "%E8%B4%9F..." (seaweedfs#11125). +// +// RequestURI must follow the cleaned path as well: PostHandler derives storage +// rules, bucket and read-only checks from it, so it has to agree with URL.Path. +// wantRequestURI defaults to wantEscaped. +func TestCleanPathHandlerServesCleanedPathWithoutRedirect(t *testing.T) { + tests := []struct { + name string + target string + wantPath string + wantEscaped string + wantRequestURI string + }{ + { + name: "double slash before non-ascii segments", + target: "/Image/2026-09-04//负极全景/OK渲染图/test123_残片正光_拼图_105525641.jpg", + wantPath: "/Image/2026-09-04/负极全景/OK渲染图/test123_残片正光_拼图_105525641.jpg", + wantEscaped: "/Image/2026-09-04/%E8%B4%9F%E6%9E%81%E5%85%A8%E6%99%AF/OK%E6%B8%B2%E6%9F%93%E5%9B%BE/test123_%E6%AE%8B%E7%89%87%E6%AD%A3%E5%85%89_%E6%8B%BC%E5%9B%BE_105525641.jpg", + }, + { + name: "already percent-encoded by the client", + target: "/Image//%E8%B4%9F%E6%9E%81%E5%85%A8%E6%99%AF/a.jpg", + wantPath: "/Image/负极全景/a.jpg", + wantEscaped: "/Image/%E8%B4%9F%E6%9E%81%E5%85%A8%E6%99%AF/a.jpg", + }, + { + name: "trailing slash marks a directory and is kept", + target: "/Image//负极全景/", + wantPath: "/Image/负极全景/", + wantEscaped: "/Image/%E8%B4%9F%E6%9E%81%E5%85%A8%E6%99%AF/", + }, + { + name: "dot segments are resolved", + target: "/Image/./tmp/../负极全景/a.jpg", + wantPath: "/Image/负极全景/a.jpg", + wantEscaped: "/Image/%E8%B4%9F%E6%9E%81%E5%85%A8%E6%99%AF/a.jpg", + }, + { + name: "dot segments crossing a bucket move RequestURI to the written bucket, query kept", + target: "/buckets/a/../b/f.jpg?collection=c&ttl=1d", + wantPath: "/buckets/b/f.jpg", + wantEscaped: "/buckets/b/f.jpg", + wantRequestURI: "/buckets/b/f.jpg?collection=c&ttl=1d", + }, + { + name: "canonical path is passed through untouched", + target: "/Image/负极全景/a.jpg", + wantPath: "/Image/负极全景/a.jpg", + wantEscaped: "/Image/%E8%B4%9F%E6%9E%81%E5%85%A8%E6%99%AF/a.jpg", + wantRequestURI: "/Image/负极全景/a.jpg", + }, + { + name: "encoded slash is not a separator and is preserved", + target: "/Image//a%2F%2Fb/c.jpg", + wantPath: "/Image/a//b/c.jpg", + wantEscaped: "/Image/a%2F%2Fb/c.jpg", + }, + { + name: "root", + target: "//", + wantPath: "/", + wantEscaped: "/", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var gotPath, gotEscaped, gotRequestURI string + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + gotPath, gotEscaped, gotRequestURI = r.URL.Path, r.URL.EscapedPath(), r.RequestURI + }) + + w := httptest.NewRecorder() + CleanPathHandler(mux).ServeHTTP(w, httptest.NewRequest(http.MethodPost, tc.target, strings.NewReader("data"))) + + if w.Code != http.StatusOK { + t.Fatalf("status: got %d (Location %q), want 200 with no redirect", w.Code, w.Header().Get("Location")) + } + if gotPath != tc.wantPath { + t.Errorf("r.URL.Path: got %q want %q", gotPath, tc.wantPath) + } + if gotEscaped != tc.wantEscaped { + t.Errorf("r.URL.EscapedPath(): got %q want %q", gotEscaped, tc.wantEscaped) + } + wantRequestURI := tc.wantRequestURI + if wantRequestURI == "" { + wantRequestURI = tc.wantEscaped + } + if gotRequestURI != wantRequestURI { + t.Errorf("r.RequestURI: got %q want %q", gotRequestURI, wantRequestURI) + } + }) + } +} + func TestWriteJsonPrettyDoesNotReadMultipartBody(t *testing.T) { var form bytes.Buffer mw := multipart.NewWriter(&form) diff --git a/weed/server/filer_server_handlers_write.go b/weed/server/filer_server_handlers_write.go index 1344c67f5..03b18d5bc 100644 --- a/weed/server/filer_server_handlers_write.go +++ b/weed/server/filer_server_handlers_write.go @@ -80,7 +80,10 @@ func (fs *FilerServer) assignNewFileInfo(ctx context.Context, so *operation.Stor func (fs *FilerServer) PostHandler(w http.ResponseWriter, r *http.Request, contentLength int64) { ctx := r.Context() - destination := r.RequestURI + // Match storage rules on the decoded path the entry is written to. The raw + // request-target is percent-encoded on the wire, so a rule on "/只读/" would + // never see "/%E5%8F%AA%E8%AF%BB/". + destination := r.URL.Path headerDestination := r.Header.Get(s3_constants.SeaweedStorageDestinationHeader) if headerDestination != "" { destination = headerDestination @@ -269,8 +272,7 @@ func (fs *FilerServer) detectStorageOption(ctx context.Context, requestURI, qCol // MatchStorageRule leaves LocationPrefix empty when several rules merge; fall back to the request path. prefix := rule.LocationPrefix if prefix == "" { - // requestURI may carry a query string on the HTTP path; keep only the path. - prefix, _, _ = strings.Cut(requestURI, "?") + prefix = requestURI } return nil, fmt.Errorf("%w: %s (e.g. bucket over quota)", ErrReadOnly, prefix) } diff --git a/weed/server/filer_server_storage_rule_ttl_test.go b/weed/server/filer_server_storage_rule_ttl_test.go index 1d1942d85..b5d507cea 100644 --- a/weed/server/filer_server_storage_rule_ttl_test.go +++ b/weed/server/filer_server_storage_rule_ttl_test.go @@ -180,6 +180,41 @@ func TestCopyKeepsRemoteEntryUnexpiring(t *testing.T) { } } +// Clients percent-encode non-ASCII path segments on the wire, so a rule has to +// be matched against the decoded path the entry is written to, not the raw +// request-target: a read-only rule on "/data/只读/" must still refuse a POST to +// "/data/%E5%8F%AA%E8%AF%BB/". +func TestPostHandlerMatchesStorageRuleOnDecodedPath(t *testing.T) { + const readOnlyPrefix = "/data/只读/" + store := newRenameTestStore() + source := newFileEntry("/src.txt", 11) + source.Content = []byte("hello") + store.entries["/src.txt"] = source + store.entries[readOnlyPrefix] = newDirectoryEntry(readOnlyPrefix, 10) + + server := &FilerServer{ + filer: newRenameTestFiler(t, store), + option: &FilerOption{}, + entryLockTable: util.NewLockTable[util.FullPath](), + } + if err := server.filer.FilerConf.AddLocationConf(&filer_pb.FilerConf_PathConf{ + LocationPrefix: readOnlyPrefix, + ReadOnly: true, + }); err != nil { + t.Fatalf("AddLocationConf: %v", err) + } + + req := httptest.NewRequest(http.MethodPost, "/data/%E5%8F%AA%E8%AF%BB/dst.txt?cp.from=/src.txt", http.NoBody) + rec := httptest.NewRecorder() + server.PostHandler(rec, req, 0) + if rec.Code != http.StatusInsufficientStorage { + t.Fatalf("copy into read-only path = %d, want %d; body=%q", rec.Code, http.StatusInsufficientStorage, rec.Body.String()) + } + if _, err := store.FindEntry(context.Background(), readOnlyPrefix+"dst.txt"); err == nil { + t.Fatal("copy landed in the read-only path") + } +} + // A completed TUS upload uploads its chunks under the target path's rule, so the // entry it lands has to expire with them. func TestCompleteTusUploadAppliesRuleTtl(t *testing.T) {