test: harden multipart upload compatibility

chfs-ng/internal/api/server.go

@@ -531,6 +531,9 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) {
 		writeJSONError(w, http.StatusForbidden, "has no privilege")
 		return
 	}
+	// Preflight the complete multipart batch before creating anything.
+	// This prevents duplicate/bad entries from leaving a partial upload.
+	seen := make(map[string]struct{})
 	var saved []string
 	for _, hs := range r.MultipartForm.File {
 		for _, h := range hs {
@@ -540,6 +543,11 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) {
 				return
 			}
 			target := path.Join(dir, name)
+			if _, ok := seen[target]; ok {
+				writeJSONError(w, http.StatusBadRequest, "invalid parameter")
+				return
+			}
+			seen[target] = struct{}{}
 			if _, e := s.Store.Stat(target); e == nil {
 				writeJSONError(w, http.StatusBadRequest, "invalid parameter")
 				return
@@ -547,6 +555,12 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) {
 				http.Error(w, e.Error(), 500)
 				return
 			}
+			// Writing happens only after every part has passed validation.
+		}
+	}
+	for _, hs := range r.MultipartForm.File {
+		for _, h := range hs {
+			target := path.Join(dir, filepath.ToSlash(h.Filename))
 			if e := savePart(s.Store, target, h); e != nil {
 				http.Error(w, e.Error(), 500)
 				return
@@ -590,7 +604,9 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), 400)
 		return
 	}
-	var saved []string
+	// Preflight the complete multipart batch before creating anything.
+	// This prevents duplicate/bad entries from leaving a partial upload.
+	seen := make(map[string]struct{})
 	for _, hs := range r.MultipartForm.File {
 		for _, h := range hs {
 			name := filepath.ToSlash(h.Filename)
@@ -600,6 +616,11 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) {
 				return
 			}
 			target := path.Join(dir, name)
+			if _, ok := seen[target]; ok {
+				writeJSONError(w, http.StatusBadRequest, "invalid parameter")
+				return
+			}
+			seen[target] = struct{}{}
 			if _, e := s.Store.Stat(target); e == nil {
 				writeJSONError(w, http.StatusBadRequest, "invalid parameter")
 				return
@@ -607,11 +628,16 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) {
 				http.Error(w, e.Error(), 500)
 				return
 			}
+		}
+	}
+	for _, hs := range r.MultipartForm.File {
+		for _, h := range hs {
+			name := strings.TrimPrefix(filepath.ToSlash(h.Filename), "/")
+			target := path.Join(dir, name)
 			if err := savePart(s.Store, target, h); err != nil {
 				http.Error(w, err.Error(), 500)
 				return
 			}
-			saved = append(saved, path.Join(dir, name))
 		}
 	}
 	writeCreated(w)

chfs-ng/internal/api/upload_compat_test.go

@@ -0,0 +1,156 @@
+package api
+
+import (
+	"bytes"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"chfs-ng/internal/auth"
+	"chfs-ng/internal/storage"
+)
+
+func uploadCompatServer(t *testing.T) (*Server, string) {
+	t.Helper()
+	root := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(root, "dir"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	return &Server{Store: storage.Local{Root: root}, Auth: &auth.Manager{Accounts: map[string]*auth.Account{}, Guest: &auth.Account{Mask: auth.Read | auth.Write | auth.Delete}}, RemoveMode: 3}, root
+}
+
+func makeUploadRequest(t *testing.T, method, target string, fieldName, fieldValue, fileName, content string) (*httptest.ResponseRecorder, *http.Request) {
+	t.Helper()
+	var b bytes.Buffer
+	mw := multipart.NewWriter(&b)
+	if fieldName != "" {
+		if err := mw.WriteField(fieldName, fieldValue); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if fileName != "" {
+		fw, err := mw.CreateFormFile("file", fileName)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, err := fw.Write([]byte(content)); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := mw.Close(); err != nil {
+		t.Fatal(err)
+	}
+	req := httptest.NewRequest(method, target, &b)
+	req.Header.Set("Content-Type", mw.FormDataContentType())
+	return httptest.NewRecorder(), req
+}
+
+func TestUploadNormalizesClientFilenamesInsideTargetFolder(t *testing.T) {
+	s, root := uploadCompatServer(t)
+	for _, tc := range []struct{ input, output string }{{"../escape1.txt", "escape1.txt"}, {"/absolute3.txt", "absolute3.txt"}} {
+		rr, req := makeUploadRequest(t, http.MethodPost, "/chfs/upload?folder=dir", "", "", tc.input, "x")
+		s.Handler().ServeHTTP(rr, req)
+		if rr.Code != http.StatusCreated {
+			t.Errorf("upload %q: status=%d", tc.input, rr.Code)
+		}
+		if _, err := os.Stat(filepath.Join(root, tc.output)); !os.IsNotExist(err) {
+			t.Fatalf("filename escaped target folder: %v", err)
+		}
+		if _, err := os.Stat(filepath.Join(root, "dir", tc.output)); err != nil {
+			t.Fatalf("normalized upload %s missing: %v", tc.output, err)
+		}
+	}
+}
+
+func TestUpload2UsesQueryFolderAndRejectsTraversal(t *testing.T) {
+	s, root := uploadCompatServer(t)
+	rr, req := makeUploadRequest(t, http.MethodPost, "/chfs/upload2?folder=dir", "", "", "nested.txt", "hello")
+	s.Handler().ServeHTTP(rr, req)
+	if rr.Code != http.StatusCreated {
+		t.Fatalf("upload2 status=%d body=%s", rr.Code, rr.Body.String())
+	}
+	got, err := os.ReadFile(filepath.Join(root, "dir", "nested.txt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(got) != "hello" {
+		t.Fatalf("content=%q", got)
+	}
+	for _, folder := range []string{"../", "..%2F", "..\\\\"} {
+		rr, req = makeUploadRequest(t, http.MethodPost, "/chfs/upload2?folder="+folder, "", "", "bad.txt", "x")
+		s.Handler().ServeHTTP(rr, req)
+		if rr.Code != http.StatusBadRequest {
+			t.Errorf("upload2 folder %q: status=%d", folder, rr.Code)
+		}
+	}
+}
+func TestUploadRejectsDuplicateWithoutOverwriting(t *testing.T) {
+	s, root := uploadCompatServer(t)
+	target := filepath.Join(root, "dir", "same.txt")
+	if err := os.WriteFile(target, []byte("original"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	rr, req := makeUploadRequest(t, http.MethodPost, "/chfs/upload", "folder", "dir", "same.txt", "replacement")
+	s.Handler().ServeHTTP(rr, req)
+	if rr.Code != http.StatusBadRequest {
+		t.Fatalf("status=%d", rr.Code)
+	}
+	got, err := os.ReadFile(target)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(got) != "original" {
+		t.Fatalf("duplicate upload overwrote file: %q", got)
+	}
+}
+
+func TestUpload2RejectsDuplicateWithoutOverwriting(t *testing.T) {
+	s, root := uploadCompatServer(t)
+	target := filepath.Join(root, "dir", "same.txt")
+	if err := os.WriteFile(target, []byte("original"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	rr, req := makeUploadRequest(t, http.MethodPost, "/chfs/upload2?folder=dir", "", "", "same.txt", "replacement")
+	s.Handler().ServeHTTP(rr, req)
+	if rr.Code != http.StatusBadRequest {
+		t.Fatalf("status=%d", rr.Code)
+	}
+	got, err := os.ReadFile(target)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(got) != "original" {
+		t.Fatalf("duplicate upload2 overwrote file: %q", got)
+	}
+}
+
+func TestUpload2BatchWritesAllFiles(t *testing.T) {
+	s, root := uploadCompatServer(t)
+	var b bytes.Buffer
+	mw := multipart.NewWriter(&b)
+	for _, name := range []string{"first.txt", "second.txt"} {
+		fw, err := mw.CreateFormFile("file", name)
+		if err != nil {
+			t.Fatal(err)
+		}
+		_, _ = fw.Write([]byte(name))
+	}
+	if err := mw.Close(); err != nil {
+		t.Fatal(err)
+	}
+	req := httptest.NewRequest(http.MethodPost, "/chfs/upload2?folder=dir", &b)
+	req.Header.Set("Content-Type", mw.FormDataContentType())
+	rr := httptest.NewRecorder()
+	s.Handler().ServeHTTP(rr, req)
+	if rr.Code != http.StatusCreated {
+		t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String())
+	}
+	for _, name := range []string{"first.txt", "second.txt"} {
+		if _, err := os.Stat(filepath.Join(root, "dir", name)); err != nil {
+			t.Fatalf("%s missing: %v", name, err)
+		}
+	}
+}