Sfoglia il codice sorgente

Fix FTP root listing and WebDAV lock persistence

FTP: serve the root directory and README.txt from a read-only in-memory filesystem instead of writing placeholder folders into a per-user tmp buffer. This removes the tmpFolder parameter from NewFTPHandler and the driver, and rejects writes to the root. Storages are rebuilt on each root access, so newly mounted or closed ones appear immediately. Tests added.

WebDAV: share the lock system per user and root across requests. The handler is rebuilt per request, so macOS Finder's LOCK followed by a PUT with the lock token in an If header failed with 412 and left a 0 byte file. Tests cover the shared lock system and a LOCK-then-PUT flow across requests.
Toby Chui 18 ore fa
parent
commit
9cb6b3fbbf

+ 1 - 1
src/mod/fileservers/servers/ftpserv/ftpserv.go

@@ -101,7 +101,7 @@ func (m *Manager) StartFtpServer() error {
 		passiveModeIP = externalIP
 	}
 
-	h, err := ftp.NewFTPHandler(m.option.UserManager, m.option.Hostname, serverPort, m.option.TmpFolder, passiveModeIP)
+	h, err := ftp.NewFTPHandler(m.option.UserManager, m.option.Hostname, serverPort, passiveModeIP)
 	if err != nil {
 		return err
 	}

+ 40 - 25
src/mod/storage/ftp/aofs.go

@@ -21,8 +21,34 @@ var (
 )
 
 type aofs struct {
-	userinfo  *user.User
-	tmpFolder string
+	userinfo *user.User
+}
+
+// isVirtualRootPath reports whether name is served by the in-memory FTP root
+// (the root listing itself and its README.txt) rather than a real storage.
+func isVirtualRootPath(name string) bool {
+	path := filepath.ToSlash(filepath.Clean(name))
+	return path == "/" || path == "/README.txt"
+}
+
+// rootFs builds the FTP root as a read-only in-memory file system: one folder
+// per storage the user can reach (buffered remote storages are not supported
+// by the FTP driver) plus a README.txt. It is rebuilt on every root access so
+// newly mounted or closed storages show up immediately.
+func (a aofs) rootFs() afero.Fs {
+	memfs := afero.NewMemMapFs()
+	for _, fsh := range a.userinfo.GetAllFileSystemHandler() {
+		if !fsh.RequireBuffer {
+			memfs.Mkdir("/"+fsh.UUID, 0555)
+		}
+	}
+
+	readmeContent, err := os.ReadFile(filepath.Join("system", "ftp", "README.txt"))
+	if err != nil {
+		readmeContent = []byte("DO NOT UPLOAD FILES INTO THE ROOT DIRECTORY")
+	}
+	afero.WriteFile(memfs, "/README.txt", readmeContent, 0444)
+	return afero.NewReadOnlyFs(memfs)
 }
 
 func (a aofs) Create(name string) (afero.File, error) {
@@ -71,6 +97,9 @@ func (a aofs) MkdirAll(path string, perm os.FileMode) error {
 
 func (a aofs) Open(name string) (afero.File, error) {
 	//fmt.Println("FTP OPEN")
+	if isVirtualRootPath(name) {
+		return a.rootFs().Open(name)
+	}
 	fsh, rewritePath, err := a.pathRewrite(name)
 	if err != nil {
 		return nil, err
@@ -84,6 +113,9 @@ func (a aofs) Open(name string) (afero.File, error) {
 
 func (a aofs) Stat(name string) (os.FileInfo, error) {
 	//fmt.Println("FTP STAT")
+	if isVirtualRootPath(name) {
+		return a.rootFs().Stat(name)
+	}
 	fsh, rewritePath, err := a.pathRewrite(name)
 	if err != nil {
 		return nil, err
@@ -96,6 +128,9 @@ func (a aofs) Stat(name string) (os.FileInfo, error) {
 
 func (a aofs) OpenFile(name string, flag int, perm os.FileMode) (afero.File, error) {
 	//fmt.Println("FTP OPEN FILE")
+	if isVirtualRootPath(name) {
+		return a.rootFs().OpenFile(name, flag, perm)
+	}
 	fsh, rewritePath, err := a.pathRewrite(name)
 	if err != nil {
 		return nil, err
@@ -212,29 +247,9 @@ func (a aofs) Chtimes(name string, atime time.Time, mtime time.Time) error {
 func (a aofs) pathRewrite(path string) (*filesystem.FileSystemHandler, string, error) {
 	path = filepath.ToSlash(filepath.Clean(path))
 	//log.Println("Original path: ", path)
-	if path == "/" {
-		//Roots. Show ftpbuf root
-		fshs := a.userinfo.GetAllFileSystemHandler()
-		for _, fsh := range fshs {
-			//Create a folder representation for this virtual directory
-
-			if !fsh.RequireBuffer {
-				os.MkdirAll(filepath.Join(a.tmpFolder, fsh.UUID), 0755)
-			}
-		}
-
-		readmeContent, err := os.ReadFile("./system/ftp/README.txt")
-		if err != nil {
-			readmeContent = []byte("DO NOT UPLOAD FILES INTO THE ROOT DIRECTORY")
-		}
-		os.WriteFile(filepath.Join(a.tmpFolder, "README.txt"), readmeContent, 0755)
-
-		//Return the tmpFolder root
-		tmpfs, _ := a.userinfo.GetFileSystemHandlerFromVirtualPath("tmp:/")
-		return tmpfs, a.tmpFolder, nil
-	} else if path == "/README.txt" {
-		tmpfs, _ := a.userinfo.GetFileSystemHandlerFromVirtualPath("tmp:/")
-		return tmpfs, a.tmpFolder + "README.txt", nil
+	if isVirtualRootPath(path) {
+		//The root is an in-memory read-only listing, see rootFs
+		return nil, "", errors.New("FTP root directory is read only")
 	} else if len(path) > 0 {
 		//Rewrite the path for any alternative filepath
 		//Get the uuid of the filepath

+ 114 - 0
src/mod/storage/ftp/aofs_test.go

@@ -0,0 +1,114 @@
+package ftp
+
+import (
+	"os"
+	"sort"
+	"testing"
+
+	"imuslab.com/arozos/mod/filesystem"
+	"imuslab.com/arozos/mod/storage"
+	"imuslab.com/arozos/mod/user"
+)
+
+// newTestAofs returns an aofs whose user can reach a local storage, a
+// buffered remote storage and a closed storage.
+func newTestAofs() aofs {
+	return aofs{
+		userinfo: &user.User{
+			Username: "alice",
+			HomeDirectories: &storage.StoragePool{
+				Storages: []*filesystem.FileSystemHandler{
+					{UUID: "user"},
+					{UUID: "remote", RequireBuffer: true},
+					{UUID: "closed", Closed: true},
+				},
+			},
+		},
+	}
+}
+
+func TestIsVirtualRootPath(t *testing.T) {
+	cases := []struct {
+		path string
+		want bool
+	}{
+		{"/", true},
+		{"/README.txt", true},
+		{"/user/../README.txt", true},
+		{"/user", false},
+		{"/user/README.txt", false},
+		{"/readme.txt", false},
+	}
+	for _, c := range cases {
+		if got := isVirtualRootPath(c.path); got != c.want {
+			t.Errorf("isVirtualRootPath(%q) = %v, want %v", c.path, got, c.want)
+		}
+	}
+}
+
+func TestRootListing(t *testing.T) {
+	a := newTestAofs()
+
+	fi, err := a.Stat("/")
+	if err != nil {
+		t.Fatalf("Stat root: %v", err)
+	}
+	if !fi.IsDir() {
+		t.Error("expected root to be a directory")
+	}
+
+	f, err := a.Open("/")
+	if err != nil {
+		t.Fatalf("Open root: %v", err)
+	}
+	defer f.Close()
+	names, err := f.Readdirnames(-1)
+	if err != nil {
+		t.Fatalf("Readdirnames: %v", err)
+	}
+	sort.Strings(names)
+	want := []string{"README.txt", "user"}
+	if len(names) != len(want) {
+		t.Fatalf("root entries = %v, want %v", names, want)
+	}
+	for i := range want {
+		if names[i] != want[i] {
+			t.Errorf("root entries = %v, want %v", names, want)
+			break
+		}
+	}
+}
+
+func TestRootReadmeReadable(t *testing.T) {
+	a := newTestAofs()
+	f, err := a.Open("/README.txt")
+	if err != nil {
+		t.Fatalf("Open README: %v", err)
+	}
+	defer f.Close()
+	buf := make([]byte, 64)
+	n, _ := f.Read(buf)
+	if n == 0 {
+		t.Error("expected README.txt to have content")
+	}
+}
+
+func TestRootIsReadOnly(t *testing.T) {
+	a := newTestAofs()
+
+	if _, err := a.OpenFile("/README.txt", os.O_WRONLY|os.O_TRUNC, 0644); err == nil {
+		t.Error("expected writing README.txt to fail")
+	}
+	if _, err := a.Create("/README.txt"); err == nil {
+		t.Error("expected creating README.txt to fail")
+	}
+	if err := a.Remove("/README.txt"); err == nil {
+		t.Error("expected removing README.txt to fail")
+	}
+	if err := a.Mkdir("/", 0755); err == nil {
+		t.Error("expected mkdir on root to fail")
+	}
+	if err := a.Rename("/README.txt", "/user/README.txt"); err == nil {
+		t.Error("expected renaming README.txt to fail")
+	}
+}

+ 1 - 7
src/mod/storage/ftp/drivers.go

@@ -4,7 +4,6 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
-	"os"
 	"time"
 
 	ftp "github.com/fclairamb/ftpserverlib"
@@ -76,18 +75,13 @@ func (m mainDriver) AuthUser(cc ftp.ClientContext, user string, pass string) (ft
 			return nil, errors.New("User " + userinfo.Username + " has no permission to access FTP endpoint")
 		}
 
-		//Create tmp buffer for this user
-		tmpFolder := m.tmpFolder + "users/" + userinfo.Username + "/ftpbuf/"
-		os.MkdirAll(tmpFolder, 0755)
-
 		//Record username into connected user list
 		m.connectedUserList.Store(cc.ID(), userinfo.Username)
 		//log the signin request
 		m.userHandler.GetAuthAgent().Logger.LogAuthByRequestInfo(userinfo.Username, cc.RemoteAddr().String(), time.Now().Unix(), true, "ftp")
 		//Return the aofs object
 		return aofs{
-			userinfo:  userinfo,
-			tmpFolder: tmpFolder,
+			userinfo: userinfo,
 		}, nil
 	} else {
 		//log the signin request

+ 1 - 3
src/mod/storage/ftp/ftp.go

@@ -25,12 +25,11 @@ type Handler struct {
 type mainDriver struct {
 	setting           ftp.Settings
 	userHandler       *user.UserHandler
-	tmpFolder         string
 	connectedUserList *sync.Map
 }
 
 // NewFTPHandler creates a new handler for FTP Server as a wrapper to the ftpserverlib
-func NewFTPHandler(userHandler *user.UserHandler, ServerName string, Port int, tmpFolder string, PassiveModeIP string) (*Handler, error) {
+func NewFTPHandler(userHandler *user.UserHandler, ServerName string, Port int, PassiveModeIP string) (*Handler, error) {
 	//Create table for ftp if it doesn't exists
 	db := userHandler.GetDatabase()
 	db.NewTable("ftp")
@@ -46,7 +45,6 @@ func NewFTPHandler(userHandler *user.UserHandler, ServerName string, Port int, t
 			},
 		},
 		userHandler:       userHandler,
-		tmpFolder:         tmpFolder,
 		connectedUserList: &sync.Map{},
 	})
 	return &Handler{

+ 12 - 2
src/mod/storage/webdav/webdav.go

@@ -32,7 +32,7 @@ import (
 type Server struct {
 	hostname    string            //The hostname of this devices
 	userHandler *user.UserHandler //The central userHandler
-	filesystems sync.Map          //The syncmap for storing opened file server
+	filesystems sync.Map          //Lock system per user and root, shared across requests
 	prefix      string            //The prefix to strip away from filepath
 	tlsMode     bool              //Bypass tls windows mode if enabled
 	Enabled     bool              //If the server is enabled. Set this to false for disable this service
@@ -344,13 +344,23 @@ func (s *Server) serveReadOnlyWebDav(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+// getLockSystem returns the lock system shared by every request of a user on
+// one root. The handler is rebuilt per request, but locks must outlive it:
+// macOS Finder LOCKs a file and then PUTs the content with the lock token in
+// an If header, which fails with 412 against a fresh lock system and leaves
+// a 0 byte file behind.
+func (s *Server) getLockSystem(username string, prefix string) webdav.LockSystem {
+	ls, _ := s.filesystems.LoadOrStore(username+"\x00"+prefix, webdav.NewMemLS())
+	return ls.(webdav.LockSystem)
+}
+
 func (s *Server) getFsFromRealRoot(fsh *filesystem.FileSystemHandler, username string, prefix string) *webdav.Handler {
 	//Create a webdav adapter from the fsh
 	fshadapter := NewFshWebDAVAdapter(fsh, username)
 	fs := &webdav.Handler{
 		Prefix:     prefix,
 		FileSystem: fshadapter,
-		LockSystem: webdav.NewMemLS(),
+		LockSystem: s.getLockSystem(username, prefix),
 	}
 
 	//Create event listener for the path request

+ 57 - 0
src/mod/storage/webdav/webdav_test.go

@@ -12,6 +12,7 @@ import (
 
 	"imuslab.com/arozos/mod/auth"
 	db "imuslab.com/arozos/mod/database"
+	"imuslab.com/arozos/mod/network/webdav"
 	"imuslab.com/arozos/mod/permission"
 	"imuslab.com/arozos/mod/share/shareEntry"
 	"imuslab.com/arozos/mod/storage"
@@ -597,3 +598,59 @@ func TestHandleConnectionList_Empty(t *testing.T) {
 		t.Errorf("expected JSON array in response, got %q", body)
 	}
 }
+
+func TestGetLockSystem_SharedPerUserAndRoot(t *testing.T) {
+	s := newTestServer()
+	a := s.getLockSystem("alice", "/webdav/user")
+	if a != s.getLockSystem("alice", "/webdav/user") {
+		t.Error("expected the same lock system for the same user and root")
+	}
+	if a == s.getLockSystem("bob", "/webdav/user") {
+		t.Error("expected different users to get different lock systems")
+	}
+	if a == s.getLockSystem("alice", "/webdav/tmp") {
+		t.Error("expected different roots to get different lock systems")
+	}
+}
+
+// Finder LOCKs a file, then PUTs its content with the lock token in an If
+// header, each in its own request. The PUT must succeed and write the body.
+func TestLockThenPut_AcrossRequests(t *testing.T) {
+	s := newTestServer()
+	dir := t.TempDir()
+	newHandler := func() *webdav.Handler {
+		return &webdav.Handler{
+			Prefix:     "/webdav/user",
+			FileSystem: webdav.Dir(dir),
+			LockSystem: s.getLockSystem("alice", "/webdav/user"),
+		}
+	}
+
+	lockBody := `<?xml version="1.0" encoding="utf-8"?><D:lockinfo xmlns:D="DAV:"><D:lockscope><D:exclusive/></D:lockscope><D:locktype><D:write/></D:locktype><D:owner>finder</D:owner></D:lockinfo>`
+	req := httptest.NewRequest("LOCK", "/webdav/user/1.jpg", strings.NewReader(lockBody))
+	req.Header.Set("User-Agent", "WebDAVFS/3.0.0 (03008000) Darwin/25.6.0")
+	w := httptest.NewRecorder()
+	newHandler().ServeHTTP(w, req)
+	token := w.Header().Get("Lock-Token")
+	if w.Code != http.StatusOK && w.Code != http.StatusCreated || token == "" {
+		t.Fatalf("LOCK: got status %d, token %q", w.Code, token)
+	}
+
+	content := "jpeg bytes"
+	req = httptest.NewRequest(http.MethodPut, "/webdav/user/1.jpg", strings.NewReader(content))
+	req.Header.Set("User-Agent", "WebDAVFS/3.0.0 (03008000) Darwin/25.6.0")
+	req.Header.Set("If", "("+token+")")
+	w = httptest.NewRecorder()
+	newHandler().ServeHTTP(w, req)
+	if w.Code != http.StatusCreated {
+		t.Fatalf("PUT with lock token: expected 201, got %d", w.Code)
+	}
+
+	got, err := os.ReadFile(filepath.Join(dir, "1.jpg"))
+	if err != nil {
+		t.Fatalf("read uploaded file: %v", err)
+	}
+	if string(got) != content {
+		t.Errorf("uploaded file content = %q, want %q", got, content)
+	}
+}