Revert "feature/taildrop: do not use m.opts.Dir for Android (#16316)"

This reverts commit d6116ea418.

feature/taildrop/delete.go

@@ -6,7 +6,9 @@ package taildrop
 import (
 	"container/list"
 	"context"
+	"io/fs"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 	"time"
@@ -26,6 +28,7 @@ const deleteDelay = time.Hour
 type fileDeleter struct {
 	logf  logger.Logf
 	clock tstime.DefaultClock
+	dir   string
 	event func(string) // called for certain events; for testing only
 
 	mu     sync.Mutex
@@ -36,7 +39,6 @@ type fileDeleter struct {
 	group       syncs.WaitGroup
 	shutdownCtx context.Context
 	shutdown    context.CancelFunc
-	fs          FileOps // must be used for all filesystem operations
 }
 
 // deleteFile is a specific file to delete after deleteDelay.
@@ -48,14 +50,15 @@ type deleteFile struct {
 func (d *fileDeleter) Init(m *manager, eventHook func(string)) {
 	d.logf = m.opts.Logf
 	d.clock = m.opts.Clock
+	d.dir = m.opts.Dir
 	d.event = eventHook
-	d.fs = m.opts.fileOps
 
 	d.byName = make(map[string]*list.Element)
 	d.emptySignal = make(chan struct{})
 	d.shutdownCtx, d.shutdown = context.WithCancel(context.Background())
 
 	// From a cold-start, load the list of partial and deleted files.
+	//
 	// Only run this if we have ever received at least one file
 	// to avoid ever touching the taildrop directory on systems (e.g., MacOS)
 	// that pop up a security dialog window upon first access.
@@ -68,45 +71,38 @@ func (d *fileDeleter) Init(m *manager, eventHook func(string)) {
 	d.group.Go(func() {
 		d.event("start full-scan")
 		defer d.event("end full-scan")
-
+		rangeDir(d.dir, func(de fs.DirEntry) bool {
-		if d.fs == nil {
-			d.logf("deleter: nil FileOps")
-		}
-
-		files, err := d.fs.ListFiles()
-		if err != nil {
-			d.logf("deleter: ListDir error: %v", err)
-			return
-		}
-		for _, filename := range files {
 			switch {
 			case d.shutdownCtx.Err() != nil:
-				return // terminate early
+				return false // terminate early
-			case strings.HasSuffix(filename, partialSuffix):
+			case !de.Type().IsRegular():
+				return true
+			case strings.HasSuffix(de.Name(), partialSuffix):
 				// Only enqueue the file for deletion if there is no active put.
-				nameID := strings.TrimSuffix(filename, partialSuffix)
+				nameID := strings.TrimSuffix(de.Name(), partialSuffix)
 				if i := strings.LastIndexByte(nameID, '.'); i > 0 {
 					key := incomingFileKey{clientID(nameID[i+len("."):]), nameID[:i]}
 					m.incomingFiles.LoadFunc(key, func(_ *incomingFile, loaded bool) {
 						if !loaded {
-							d.Insert(filename)
+							d.Insert(de.Name())
 						}
 					})
 				} else {
-					d.Insert(filename)
+					d.Insert(de.Name())
 				}
-			case strings.HasSuffix(filename, deletedSuffix):
+			case strings.HasSuffix(de.Name(), deletedSuffix):
 				// Best-effort immediate deletion of deleted files.
-				name := strings.TrimSuffix(filename, deletedSuffix)
+				name := strings.TrimSuffix(de.Name(), deletedSuffix)
-				if d.fs.Remove(name) == nil {
+				if os.Remove(filepath.Join(d.dir, name)) == nil {
-					if d.fs.Remove(filename) == nil {
+					if os.Remove(filepath.Join(d.dir, de.Name())) == nil {
-						continue
+						break
 					}
 				}
-				// Otherwise enqueue for later deletion.
+				// Otherwise, enqueue the file for later deletion.
-				d.Insert(filename)
+				d.Insert(de.Name())
-			}
 			}
+			return true
+		})
 	})
 }
 
@@ -153,13 +149,13 @@ func (d *fileDeleter) waitAndDelete(wait time.Duration) {
 
 			// Delete the expired file.
 			if name, ok := strings.CutSuffix(file.name, deletedSuffix); ok {
-				if err := d.fs.Remove(name); err != nil && !os.IsNotExist(err) {
+				if err := os.Remove(filepath.Join(d.dir, name)); err != nil && !os.IsNotExist(err) {
 					d.logf("could not delete: %v", redactError(err))
 					failed = append(failed, elem)
 					continue
 				}
 			}
-			if err := d.fs.Remove(file.name); err != nil && !os.IsNotExist(err) {
+			if err := os.Remove(filepath.Join(d.dir, file.name)); err != nil && !os.IsNotExist(err) {
 				d.logf("could not delete: %v", redactError(err))
 				failed = append(failed, elem)
 				continue

feature/taildrop/delete_test.go

@@ -5,6 +5,7 @@ package taildrop
 
 import (
 	"os"
+	"path/filepath"
 	"slices"
 	"testing"
 	"time"
@@ -19,20 +20,11 @@ import (
 
 func TestDeleter(t *testing.T) {
 	dir := t.TempDir()
-	var m manager
+	must.Do(touchFile(filepath.Join(dir, "foo.partial")))
-	var fd fileDeleter
+	must.Do(touchFile(filepath.Join(dir, "bar.partial")))
-	m.opts.Logf = t.Logf
+	must.Do(touchFile(filepath.Join(dir, "fizz")))
-	m.opts.Clock = tstime.DefaultClock{Clock: tstest.NewClock(tstest.ClockOpts{
+	must.Do(touchFile(filepath.Join(dir, "fizz.deleted")))
-		Start: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC),
+	must.Do(touchFile(filepath.Join(dir, "buzz.deleted"))) // lacks a matching "buzz" file
-	})}
-	m.opts.State = must.Get(mem.New(nil, ""))
-	m.opts.fileOps, _ = newFileOps(dir)
-
-	must.Do(m.touchFile("foo.partial"))
-	must.Do(m.touchFile("bar.partial"))
-	must.Do(m.touchFile("fizz"))
-	must.Do(m.touchFile("fizz.deleted"))
-	must.Do(m.touchFile("buzz.deleted")) // lacks a matching "buzz" file
 
 	checkDirectory := func(want ...string) {
 		t.Helper()
@@ -77,10 +69,12 @@ func TestDeleter(t *testing.T) {
 	}
 	eventHook := func(event string) { eventsChan <- event }
 
+	var m manager
+	var fd fileDeleter
 	m.opts.Logf = t.Logf
 	m.opts.Clock = tstime.DefaultClock{Clock: clock}
+	m.opts.Dir = dir
 	m.opts.State = must.Get(mem.New(nil, ""))
-	m.opts.fileOps, _ = newFileOps(dir)
 	must.Do(m.opts.State.WriteState(ipn.TaildropReceivedKey, []byte{1}))
 	fd.Init(&m, eventHook)
 	defer fd.Shutdown()
@@ -106,17 +100,17 @@ func TestDeleter(t *testing.T) {
 	checkEvents("end waitAndDelete")
 	checkDirectory()
 
-	must.Do(m.touchFile("one.partial"))
+	must.Do(touchFile(filepath.Join(dir, "one.partial")))
 	insert("one.partial")
 	checkEvents("start waitAndDelete")
 	advance(deleteDelay / 4)
-	must.Do(m.touchFile("two.partial"))
+	must.Do(touchFile(filepath.Join(dir, "two.partial")))
 	insert("two.partial")
 	advance(deleteDelay / 4)
-	must.Do(m.touchFile("three.partial"))
+	must.Do(touchFile(filepath.Join(dir, "three.partial")))
 	insert("three.partial")
 	advance(deleteDelay / 4)
-	must.Do(m.touchFile("four.partial"))
+	must.Do(touchFile(filepath.Join(dir, "four.partial")))
 	insert("four.partial")
 
 	advance(deleteDelay / 4)
@@ -151,8 +145,8 @@ func TestDeleterInitWithoutTaildrop(t *testing.T) {
 	var m manager
 	var fd fileDeleter
 	m.opts.Logf = t.Logf
+	m.opts.Dir = t.TempDir()
 	m.opts.State = must.Get(mem.New(nil, ""))
-	m.opts.fileOps, _ = newFileOps(t.TempDir())
 	fd.Init(&m, func(event string) { t.Errorf("unexpected event: %v", event) })
 	fd.Shutdown()
 }

feature/taildrop/ext.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"maps"
+	"os"
 	"path/filepath"
 	"runtime"
 	"slices"
@@ -74,7 +75,7 @@ type Extension struct {
 
 	// FileOps abstracts platform-specific file operations needed for file transfers.
 	// This is currently being used for Android to use the Storage Access Framework.
-	fileOps FileOps
+	FileOps FileOps
 
 	nodeBackendForTest ipnext.NodeBackend // if non-nil, pretend we're this node state for tests
 
@@ -88,6 +89,30 @@ type Extension struct {
 	outgoingFiles map[string]*ipn.OutgoingFile
 }
 
+// safDirectoryPrefix is used to determine if the directory is managed via SAF.
+const SafDirectoryPrefix = "content://"
+
+// PutMode controls how Manager.PutFile writes files to storage.
+//
+//	PutModeDirect    – write files directly to a filesystem path (default).
+//	PutModeAndroidSAF – use Android’s Storage Access Framework (SAF), where
+//	                      the OS manages the underlying directory permissions.
+type PutMode int
+
+const (
+	PutModeDirect PutMode = iota
+	PutModeAndroidSAF
+)
+
+// FileOps defines platform-specific file operations.
+type FileOps interface {
+	OpenFileWriter(filename string) (io.WriteCloser, string, error)
+
+	// RenamePartialFile finalizes a partial file.
+	// It returns the new SAF URI as a string and an error.
+	RenamePartialFile(partialUri, targetDirUri, targetName string) (string, error)
+}
+
 func (e *Extension) Name() string {
 	return "taildrop"
 }
@@ -151,34 +176,23 @@ func (e *Extension) onChangeProfile(profile ipn.LoginProfileView, _ ipn.PrefsVie
 		return
 	}
 
-	// Use the provided [FileOps] implementation (typically for SAF access on Android),
+	// If we have a netmap, create a taildrop manager.
-	// or create an [fsFileOps] instance rooted at fileRoot.
+	fileRoot, isDirectFileMode := e.fileRoot(uid, activeLogin)
-	//
+	if fileRoot == "" {
-	// A non-nil [FileOps] also implies that we are in DirectFileMode.
-	fops := e.fileOps
-	isDirectFileMode := fops != nil
-	if fops == nil {
-		var fileRoot string
-		if fileRoot, isDirectFileMode = e.fileRoot(uid, activeLogin); fileRoot == "" {
 		e.logf("no Taildrop directory configured")
-			e.setMgrLocked(nil)
-			return
 	}
-
+	mode := PutModeDirect
-		var err error
+	if e.directFileRoot != "" && strings.HasPrefix(e.directFileRoot, SafDirectoryPrefix) {
-		if fops, err = newFileOps(fileRoot); err != nil {
+		mode = PutModeAndroidSAF
-			e.logf("taildrop: cannot create FileOps: %v", err)
-			e.setMgrLocked(nil)
-			return
 	}
-	}
-
 	e.setMgrLocked(managerOptions{
 		Logf:           e.logf,
 		Clock:          tstime.DefaultClock{Clock: e.sb.Clock()},
 		State:          e.stateStore,
+		Dir:            fileRoot,
 		DirectFileMode: isDirectFileMode,
-		fileOps:        fops,
+		FileOps:        e.FileOps,
+		Mode:           mode,
 		SendFileNotify: e.sendFileNotify,
 	}.New())
 }
@@ -207,7 +221,12 @@ func (e *Extension) fileRoot(uid tailcfg.UserID, activeLogin string) (root strin
 	baseDir := fmt.Sprintf("%s-uid-%d",
 		strings.ReplaceAll(activeLogin, "@", "-"),
 		uid)
-	return filepath.Join(varRoot, "files", baseDir), false
+	dir := filepath.Join(varRoot, "files", baseDir)
+	if err := os.MkdirAll(dir, 0700); err != nil {
+		e.logf("Taildrop disabled; error making directory: %v", err)
+		return "", false
+	}
+	return dir, false
 }
 
 // hasCapFileSharing reports whether the current node has the file sharing

feature/taildrop/fileops.go

@@ -1,41 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-package taildrop
-
-import (
-	"io"
-	"io/fs"
-	"os"
-)
-
-// FileOps abstracts over both local‐FS paths and Android SAF URIs.
-type FileOps interface {
-	// OpenWriter creates or truncates a file named relative to the receiver's root,
-	// seeking to the specified offset. If the file does not exist, it is created with mode perm
-	// on platforms that support it.
-	//
-	// It returns an [io.WriteCloser] and the file's absolute path, or an error.
-	// This call may block. Callers should avoid holding locks when calling OpenWriter.
-	OpenWriter(name string, offset int64, perm os.FileMode) (wc io.WriteCloser, path string, err error)
-
-	// Remove deletes a file or directory relative to the receiver's root.
-	// It returns [io.ErrNotExist] if the file or directory does not exist.
-	Remove(name string) error
-
-	// Rename atomically renames oldPath to a new file named newName,
-	// returning the full new path or an error.
-	Rename(oldPath, newName string) (newPath string, err error)
-
-	// ListFiles returns just the basenames of all regular files
-	// in the root directory.
-	ListFiles() ([]string, error)
-
-	// Stat returns the FileInfo for the given name or an error.
-	Stat(name string) (fs.FileInfo, error)
-
-	// OpenReader opens the given basename for the given name or an error.
-	OpenReader(name string) (io.ReadCloser, error)
-}
-
-var newFileOps func(dir string) (FileOps, error)

feature/taildrop/fileops_fs.go

@@ -1,221 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-//go:build !android
-
-package taildrop
-
-import (
-	"bytes"
-	"crypto/sha256"
-	"errors"
-	"fmt"
-	"io"
-	"io/fs"
-	"os"
-	"path"
-	"path/filepath"
-	"strings"
-	"sync"
-	"unicode/utf8"
-)
-
-var renameMu sync.Mutex
-
-// fsFileOps implements FileOps using the local filesystem rooted at a directory.
-// It is used on non-Android platforms.
-type fsFileOps struct{ rootDir string }
-
-func init() {
-	newFileOps = func(dir string) (FileOps, error) {
-		if dir == "" {
-			return nil, errors.New("rootDir cannot be empty")
-		}
-		if err := os.MkdirAll(dir, 0o700); err != nil {
-			return nil, fmt.Errorf("mkdir %q: %w", dir, err)
-		}
-		return fsFileOps{rootDir: dir}, nil
-	}
-}
-
-func (f fsFileOps) OpenWriter(name string, offset int64, perm os.FileMode) (io.WriteCloser, string, error) {
-	path, err := joinDir(f.rootDir, name)
-	if err != nil {
-		return nil, "", err
-	}
-	if err = os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
-		return nil, "", err
-	}
-	fi, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, perm)
-	if err != nil {
-		return nil, "", err
-	}
-	if offset != 0 {
-		curr, err := fi.Seek(0, io.SeekEnd)
-		if err != nil {
-			fi.Close()
-			return nil, "", err
-		}
-		if offset < 0 || offset > curr {
-			fi.Close()
-			return nil, "", fmt.Errorf("offset %d out of range", offset)
-		}
-		if _, err := fi.Seek(offset, io.SeekStart); err != nil {
-			fi.Close()
-			return nil, "", err
-		}
-		if err := fi.Truncate(offset); err != nil {
-			fi.Close()
-			return nil, "", err
-		}
-	}
-	return fi, path, nil
-}
-
-func (f fsFileOps) Remove(name string) error {
-	path, err := joinDir(f.rootDir, name)
-	if err != nil {
-		return err
-	}
-	return os.Remove(path)
-}
-
-// Rename moves the partial file into its final name.
-// newName must be a base name (not absolute or containing path separators).
-// It will retry up to 10 times, de-dup same-checksum files, etc.
-func (f fsFileOps) Rename(oldPath, newName string) (newPath string, err error) {
-	var dst string
-	if filepath.IsAbs(newName) || strings.ContainsRune(newName, os.PathSeparator) {
-		return "", fmt.Errorf("invalid newName %q: must not be an absolute path or contain path separators", newName)
-	}
-
-	dst = filepath.Join(f.rootDir, newName)
-
-	if err := os.MkdirAll(filepath.Dir(dst), 0o700); err != nil {
-		return "", err
-	}
-
-	st, err := os.Stat(oldPath)
-	if err != nil {
-		return "", err
-	}
-	wantSize := st.Size()
-
-	const maxRetries = 10
-	for i := 0; i < maxRetries; i++ {
-		renameMu.Lock()
-		fi, statErr := os.Stat(dst)
-		// Atomically rename the partial file as the destination file if it doesn't exist.
-		// Otherwise, it returns the length of the current destination file.
-		// The operation is atomic.
-		if os.IsNotExist(statErr) {
-			err = os.Rename(oldPath, dst)
-			renameMu.Unlock()
-			if err != nil {
-				return "", err
-			}
-			return dst, nil
-		}
-		if statErr != nil {
-			renameMu.Unlock()
-			return "", statErr
-		}
-		gotSize := fi.Size()
-		renameMu.Unlock()
-
-		// Avoid the final rename if a destination file has the same contents.
-		//
-		// Note: this is best effort and copying files from iOS from the Media Library
-		// results in processing on the iOS side which means the size and shas of the
-		// same file can be different.
-		if gotSize == wantSize {
-			sumP, err := sha256File(oldPath)
-			if err != nil {
-				return "", err
-			}
-			sumD, err := sha256File(dst)
-			if err != nil {
-				return "", err
-			}
-			if bytes.Equal(sumP[:], sumD[:]) {
-				if err := os.Remove(oldPath); err != nil {
-					return "", err
-				}
-				return dst, nil
-			}
-		}
-
-		// Choose a new destination filename and try again.
-		dst = filepath.Join(filepath.Dir(dst), nextFilename(filepath.Base(dst)))
-	}
-
-	return "", fmt.Errorf("too many retries trying to rename %q to %q", oldPath, newName)
-}
-
-// sha256File computes the SHA‑256 of a file.
-func sha256File(path string) (sum [sha256.Size]byte, _ error) {
-	f, err := os.Open(path)
-	if err != nil {
-		return sum, err
-	}
-	defer f.Close()
-	h := sha256.New()
-	if _, err := io.Copy(h, f); err != nil {
-		return sum, err
-	}
-	copy(sum[:], h.Sum(nil))
-	return sum, nil
-}
-
-func (f fsFileOps) ListFiles() ([]string, error) {
-	entries, err := os.ReadDir(f.rootDir)
-	if err != nil {
-		return nil, err
-	}
-	var names []string
-	for _, e := range entries {
-		if e.Type().IsRegular() {
-			names = append(names, e.Name())
-		}
-	}
-	return names, nil
-}
-
-func (f fsFileOps) Stat(name string) (fs.FileInfo, error) {
-	path, err := joinDir(f.rootDir, name)
-	if err != nil {
-		return nil, err
-	}
-	return os.Stat(path)
-}
-
-func (f fsFileOps) OpenReader(name string) (io.ReadCloser, error) {
-	path, err := joinDir(f.rootDir, name)
-	if err != nil {
-		return nil, err
-	}
-	return os.Open(path)
-}
-
-// joinDir is like [filepath.Join] but returns an error if baseName is too long,
-// is a relative path instead of a basename, or is otherwise invalid or unsafe for incoming files.
-func joinDir(dir, baseName string) (string, error) {
-	if !utf8.ValidString(baseName) ||
-		strings.TrimSpace(baseName) != baseName ||
-		len(baseName) > 255 {
-		return "", ErrInvalidFileName
-	}
-	// TODO: validate unicode normalization form too? Varies by platform.
-	clean := path.Clean(baseName)
-	if clean != baseName || clean == "." || clean == ".." {
-		return "", ErrInvalidFileName
-	}
-	for _, r := range baseName {
-		if !validFilenameRune(r) {
-			return "", ErrInvalidFileName
-		}
-	}
-	if !filepath.IsLocal(baseName) {
-		return "", ErrInvalidFileName
-	}
-	return filepath.Join(dir, baseName), nil
-}

feature/taildrop/paths.go

@@ -21,7 +21,7 @@ func (e *Extension) SetDirectFileRoot(root string) {
 // SetFileOps sets the platform specific file operations. This is used
 // to call Android's Storage Access Framework APIs.
 func (e *Extension) SetFileOps(fileOps FileOps) {
-	e.fileOps = fileOps
+	e.FileOps = fileOps
 }
 
 func (e *Extension) setPlatformDefaultDirectFileRoot() {

feature/taildrop/peerapi_test.go

@@ -24,7 +24,6 @@ import (
 	"tailscale.com/tstest"
 	"tailscale.com/tstime"
 	"tailscale.com/types/logger"
-	"tailscale.com/util/must"
 )
 
 // peerAPIHandler serves the PeerAPI for a source specific client.
@@ -94,16 +93,7 @@ func bodyContains(sub string) check {
 
 func fileHasSize(name string, size int) check {
 	return func(t *testing.T, e *peerAPITestEnv) {
-		fsImpl, ok := e.taildrop.opts.fileOps.(*fsFileOps)
+		root := e.taildrop.Dir()
-		if !ok {
-			t.Skip("fileHasSize only supported on fsFileOps backend")
-			return
-		}
-		root := fsImpl.rootDir
-		if root == "" {
-			t.Errorf("no rootdir; can't check whether %q has size %v", name, size)
-			return
-		}
 		if root == "" {
 			t.Errorf("no rootdir; can't check whether %q has size %v", name, size)
 			return
@@ -119,12 +109,12 @@ func fileHasSize(name string, size int) check {
 
 func fileHasContents(name string, want string) check {
 	return func(t *testing.T, e *peerAPITestEnv) {
-		fsImpl, ok := e.taildrop.opts.fileOps.(*fsFileOps)
+		root := e.taildrop.Dir()
-		if !ok {
+		if root == "" {
-			t.Skip("fileHasContents only supported on fsFileOps backend")
+			t.Errorf("no rootdir; can't check contents of %q", name)
 			return
 		}
-		path := filepath.Join(fsImpl.rootDir, name)
+		path := filepath.Join(root, name)
 		got, err := os.ReadFile(path)
 		if err != nil {
 			t.Errorf("fileHasContents: %v", err)
@@ -182,10 +172,9 @@ func TestHandlePeerAPI(t *testing.T) {
 			reqs:       []*http.Request{httptest.NewRequest("PUT", "/v0/put/foo", nil)},
 			checks: checks(
 				httpStatus(http.StatusForbidden),
-				bodyContains("Taildrop disabled"),
+				bodyContains("Taildrop disabled; no storage directory"),
 			),
 		},
-
 		{
 			name:       "bad_method",
 			isSelf:     true,
@@ -482,18 +471,14 @@ func TestHandlePeerAPI(t *testing.T) {
 				selfNode.CapMap = tailcfg.NodeCapMap{tailcfg.CapabilityDebug: nil}
 			}
 			var rootDir string
-			var fo FileOps
 			if !tt.omitRoot {
-				var err error
+				rootDir = t.TempDir()
-				if fo, err = newFileOps(t.TempDir()); err != nil {
-					t.Fatalf("newFileOps: %v", err)
-				}
 			}
 
 			var e peerAPITestEnv
 			e.taildrop = managerOptions{
 				Logf: e.logBuf.Logf,
-				fileOps: fo,
+				Dir:  rootDir,
 			}.New()
 
 			ext := &fakeExtension{
@@ -505,7 +490,9 @@ func TestHandlePeerAPI(t *testing.T) {
 			e.ph = &peerAPIHandler{
 				isSelf:   tt.isSelf,
 				selfNode: selfNode.View(),
-				peerNode: (&tailcfg.Node{ComputedName: "some-peer-name"}).View(),
+				peerNode: (&tailcfg.Node{
+					ComputedName: "some-peer-name",
+				}).View(),
 			}
 			for _, req := range tt.reqs {
 				e.rr = httptest.NewRecorder()
@@ -540,7 +527,7 @@ func TestFileDeleteRace(t *testing.T) {
 	dir := t.TempDir()
 	taildropMgr := managerOptions{
 		Logf: t.Logf,
-		fileOps: must.Get(newFileOps(dir)),
+		Dir:  dir,
 	}.New()
 
 	ph := &peerAPIHandler{

feature/taildrop/resume.go

@@ -9,6 +9,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"strings"
 )
@@ -50,20 +51,19 @@ func (cs *checksum) UnmarshalText(b []byte) error {
 
 // PartialFiles returns a list of partial files in [Handler.Dir]
 // that were sent (or is actively being sent) by the provided id.
-func (m *manager) PartialFiles(id clientID) ([]string, error) {
+func (m *manager) PartialFiles(id clientID) (ret []string, err error) {
-	if m == nil || m.opts.fileOps == nil {
+	if m == nil || m.opts.Dir == "" {
 		return nil, ErrNoTaildrop
 	}
+
 	suffix := id.partialSuffix()
-	files, err := m.opts.fileOps.ListFiles()
+	if err := rangeDir(m.opts.Dir, func(de fs.DirEntry) bool {
-	if err != nil {
+		if name := de.Name(); strings.HasSuffix(name, suffix) {
-		return nil, redactError(err)
+			ret = append(ret, name)
-	}
-	var ret []string
-	for _, filename := range files {
-		if strings.HasSuffix(filename, suffix) {
-			ret = append(ret, filename)
 		}
+		return true
+	}); err != nil {
+		return ret, redactError(err)
 	}
 	return ret, nil
 }
@@ -73,13 +73,17 @@ func (m *manager) PartialFiles(id clientID) ([]string, error) {
 // It returns (BlockChecksum{}, io.EOF) when the stream is complete.
 // It is the caller's responsibility to call close.
 func (m *manager) HashPartialFile(id clientID, baseName string) (next func() (blockChecksum, error), close func() error, err error) {
-	if m == nil || m.opts.fileOps == nil {
+	if m == nil || m.opts.Dir == "" {
 		return nil, nil, ErrNoTaildrop
 	}
 	noopNext := func() (blockChecksum, error) { return blockChecksum{}, io.EOF }
 	noopClose := func() error { return nil }
 
-	f, err := m.opts.fileOps.OpenReader(baseName + id.partialSuffix())
+	dstFile, err := joinDir(m.opts.Dir, baseName)
+	if err != nil {
+		return nil, nil, err
+	}
+	f, err := os.Open(dstFile + id.partialSuffix())
 	if err != nil {
 		if os.IsNotExist(err) {
 			return noopNext, noopClose, nil

feature/taildrop/resume_test.go

@@ -8,7 +8,6 @@ import (
 	"io"
 	"math/rand"
 	"os"
-	"path/filepath"
 	"testing"
 	"testing/iotest"
 
@@ -20,9 +19,7 @@ func TestResume(t *testing.T) {
 	defer func() { blockSize = oldBlockSize }()
 	blockSize = 256
 
-	dir := t.TempDir()
+	m := managerOptions{Logf: t.Logf, Dir: t.TempDir()}.New()
-
-	m := managerOptions{Logf: t.Logf, fileOps: must.Get(newFileOps(dir))}.New()
 	defer m.Shutdown()
 
 	rn := rand.New(rand.NewSource(0))
@@ -40,7 +37,7 @@ func TestResume(t *testing.T) {
 		must.Do(close()) // Windows wants the file handle to be closed to rename it.
 
 		must.Get(m.PutFile("", "foo", r, offset, -1))
-		got := must.Get(os.ReadFile(filepath.Join(dir, "foo")))
+		got := must.Get(os.ReadFile(must.Get(joinDir(m.opts.Dir, "foo"))))
 		if !bytes.Equal(got, want) {
 			t.Errorf("content mismatches")
 		}
@@ -69,7 +66,7 @@ func TestResume(t *testing.T) {
 				t.Fatalf("too many iterations to complete the test")
 			}
 		}
-		got := must.Get(os.ReadFile(filepath.Join(dir, "bar")))
+		got := must.Get(os.ReadFile(must.Get(joinDir(m.opts.Dir, "bar"))))
 		if !bytes.Equal(got, want) {
 			t.Errorf("content mismatches")
 		}

feature/taildrop/retrieve.go

@@ -9,19 +9,19 @@ import (
 	"io"
 	"io/fs"
 	"os"
+	"path/filepath"
 	"runtime"
 	"sort"
 	"time"
 
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/logtail/backoff"
-	"tailscale.com/util/set"
 )
 
 // HasFilesWaiting reports whether any files are buffered in [Handler.Dir].
 // This always returns false when [Handler.DirectFileMode] is false.
-func (m *manager) HasFilesWaiting() bool {
+func (m *manager) HasFilesWaiting() (has bool) {
-	if m == nil || m.opts.fileOps == nil || m.opts.DirectFileMode {
+	if m == nil || m.opts.Dir == "" || m.opts.DirectFileMode {
 		return false
 	}
 
@@ -30,67 +30,64 @@ func (m *manager) HasFilesWaiting() bool {
 	// has-files-or-not values as the macOS/iOS client might
 	// in the future use+delete the files directly. So only
 	// keep this negative cache.
-	total := m.totalReceived.Load()
+	totalReceived := m.totalReceived.Load()
-	if total == m.emptySince.Load() {
+	if totalReceived == m.emptySince.Load() {
 		return false
 	}
 
-	files, err := m.opts.fileOps.ListFiles()
+	// Check whether there is at least one one waiting file.
-	if err != nil {
+	err := rangeDir(m.opts.Dir, func(de fs.DirEntry) bool {
-		return false
+		name := de.Name()
-	}
+		if isPartialOrDeleted(name) || !de.Type().IsRegular() {
-
-	// Build a set of filenames present in Dir
-	fileSet := set.Of(files...)
-
-	for _, filename := range files {
-		if isPartialOrDeleted(filename) {
-			continue
-		}
-		if fileSet.Contains(filename + deletedSuffix) {
-			continue // already handled
-		}
-		// Found at least one downloadable file
 			return true
 		}
-
+		_, err := os.Stat(filepath.Join(m.opts.Dir, name+deletedSuffix))
-	// No waiting files → update negative‑result cache
+		if os.IsNotExist(err) {
-	m.emptySince.Store(total)
+			has = true
 			return false
 		}
+		return true
+	})
+
+	// If there are no more waiting files, record totalReceived as emptySince
+	// so that we can short-circuit the expensive directory traversal
+	// if no files have been received after the start of this call.
+	if err == nil && !has {
+		m.emptySince.Store(totalReceived)
+	}
+	return has
+}
 
 // WaitingFiles returns the list of files that have been sent by a
 // peer that are waiting in [Handler.Dir].
 // This always returns nil when [Handler.DirectFileMode] is false.
-func (m *manager) WaitingFiles() ([]apitype.WaitingFile, error) {
+func (m *manager) WaitingFiles() (ret []apitype.WaitingFile, err error) {
-	if m == nil || m.opts.fileOps == nil {
+	if m == nil || m.opts.Dir == "" {
 		return nil, ErrNoTaildrop
 	}
 	if m.opts.DirectFileMode {
 		return nil, nil
 	}
-	names, err := m.opts.fileOps.ListFiles()
+	if err := rangeDir(m.opts.Dir, func(de fs.DirEntry) bool {
+		name := de.Name()
+		if isPartialOrDeleted(name) || !de.Type().IsRegular() {
+			return true
+		}
+		_, err := os.Stat(filepath.Join(m.opts.Dir, name+deletedSuffix))
+		if os.IsNotExist(err) {
+			fi, err := de.Info()
 			if err != nil {
-		return nil, redactError(err)
+				return true
-	}
-	var ret []apitype.WaitingFile
-	for _, name := range names {
-		if isPartialOrDeleted(name) {
-			continue
-		}
-		// A corresponding .deleted marker means the file was already handled.
-		if _, err := m.opts.fileOps.Stat(name + deletedSuffix); err == nil {
-			continue
-		}
-		fi, err := m.opts.fileOps.Stat(name)
-		if err != nil {
-			continue
 			}
 			ret = append(ret, apitype.WaitingFile{
-			Name: name,
+				Name: filepath.Base(name),
 				Size: fi.Size(),
 			})
 		}
+		return true
+	}); err != nil {
+		return nil, redactError(err)
+	}
 	sort.Slice(ret, func(i, j int) bool { return ret[i].Name < ret[j].Name })
 	return ret, nil
 }
@@ -98,18 +95,21 @@ func (m *manager) WaitingFiles() ([]apitype.WaitingFile, error) {
 // DeleteFile deletes a file of the given baseName from [Handler.Dir].
 // This method is only allowed when [Handler.DirectFileMode] is false.
 func (m *manager) DeleteFile(baseName string) error {
-	if m == nil || m.opts.fileOps == nil {
+	if m == nil || m.opts.Dir == "" {
 		return ErrNoTaildrop
 	}
 	if m.opts.DirectFileMode {
 		return errors.New("deletes not allowed in direct mode")
 	}
-
+	path, err := joinDir(m.opts.Dir, baseName)
+	if err != nil {
+		return err
+	}
 	var bo *backoff.Backoff
 	logf := m.opts.Logf
 	t0 := m.opts.Clock.Now()
 	for {
-		err := m.opts.fileOps.Remove(baseName)
+		err := os.Remove(path)
 		if err != nil && !os.IsNotExist(err) {
 			err = redactError(err)
 			// Put a retry loop around deletes on Windows.
@@ -129,7 +129,7 @@ func (m *manager) DeleteFile(baseName string) error {
 					bo.BackOff(context.Background(), err)
 					continue
 				}
-				if err := m.touchFile(baseName + deletedSuffix); err != nil {
+				if err := touchFile(path + deletedSuffix); err != nil {
 					logf("peerapi: failed to leave deleted marker: %v", err)
 				}
 				m.deleter.Insert(baseName + deletedSuffix)
@@ -141,31 +141,35 @@ func (m *manager) DeleteFile(baseName string) error {
 	}
 }
 
-func (m *manager) touchFile(name string) error {
+func touchFile(path string) error {
-	wc, _, err := m.opts.fileOps.OpenWriter(name /* offset= */, 0, 0666)
+	f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0666)
 	if err != nil {
 		return redactError(err)
 	}
-	return wc.Close()
+	return f.Close()
 }
 
 // OpenFile opens a file of the given baseName from [Handler.Dir].
 // This method is only allowed when [Handler.DirectFileMode] is false.
 func (m *manager) OpenFile(baseName string) (rc io.ReadCloser, size int64, err error) {
-	if m == nil || m.opts.fileOps == nil {
+	if m == nil || m.opts.Dir == "" {
 		return nil, 0, ErrNoTaildrop
 	}
 	if m.opts.DirectFileMode {
 		return nil, 0, errors.New("opens not allowed in direct mode")
 	}
-	if _, err := m.opts.fileOps.Stat(baseName + deletedSuffix); err == nil {
+	path, err := joinDir(m.opts.Dir, baseName)
-		return nil, 0, redactError(&fs.PathError{Op: "open", Path: baseName, Err: fs.ErrNotExist})
+	if err != nil {
+		return nil, 0, err
 	}
-	f, err := m.opts.fileOps.OpenReader(baseName)
+	if _, err := os.Stat(path + deletedSuffix); err == nil {
+		return nil, 0, redactError(&fs.PathError{Op: "open", Path: path, Err: fs.ErrNotExist})
+	}
+	f, err := os.Open(path)
 	if err != nil {
 		return nil, 0, redactError(err)
 	}
-	fi, err := m.opts.fileOps.Stat(baseName)
+	fi, err := f.Stat()
 	if err != nil {
 		f.Close()
 		return nil, 0, redactError(err)

feature/taildrop/send.go

@@ -4,8 +4,11 @@
 package taildrop
 
 import (
+	"crypto/sha256"
 	"fmt"
 	"io"
+	"os"
+	"path/filepath"
 	"sync"
 	"time"
 
@@ -70,10 +73,9 @@ func (f *incomingFile) Write(p []byte) (n int, err error) {
 // specific partial file. This allows the client to determine whether to resume
 // a partial file. While resuming, PutFile may be called again with a non-zero
 // offset to specify where to resume receiving data at.
-func (m *manager) PutFile(id clientID, baseName string, r io.Reader, offset, length int64) (fileLength int64, err error) {
+func (m *manager) PutFile(id clientID, baseName string, r io.Reader, offset, length int64) (int64, error) {
-
 	switch {
-	case m == nil || m.opts.fileOps == nil:
+	case m == nil || m.opts.Dir == "":
 		return 0, ErrNoTaildrop
 	case !envknob.CanTaildrop():
 		return 0, ErrNoTaildrop
@@ -81,48 +83,48 @@ func (m *manager) PutFile(id clientID, baseName string, r io.Reader, offset, len
 		return 0, ErrNotAccessible
 	}
 
-	if err := validateBaseName(baseName); err != nil {
+	//Compute dstPath & avoid mid‑upload deletion
+	var dstPath string
+	if m.opts.Mode == PutModeDirect {
+		var err error
+		dstPath, err = joinDir(m.opts.Dir, baseName)
+		if err != nil {
 			return 0, err
 		}
+	} else {
+		// In SAF mode, we simply use the baseName as the destination "path"
+		// (the actual directory is managed by SAF).
+		dstPath = baseName
+	}
+	m.deleter.Remove(filepath.Base(dstPath)) // avoid deleting the partial file while receiving
 
-	// and make sure we don't delete it while uploading:
+	// Check whether there is an in-progress transfer for the file.
-	m.deleter.Remove(baseName)
+	partialFileKey := incomingFileKey{id, baseName}
+	inFile, loaded := m.incomingFiles.LoadOrInit(partialFileKey, func() *incomingFile {
+		return &incomingFile{
+			clock:          m.opts.Clock,
+			started:        m.opts.Clock.Now(),
+			size:           length,
+			sendFileNotify: m.opts.SendFileNotify,
+		}
+	})
+	if loaded {
+		return 0, ErrFileExists
+	}
+	defer m.incomingFiles.Delete(partialFileKey)
 
-	// Create (if not already) the partial file with read-write permissions.
+	// Open writer & populate inFile paths
-	partialName := baseName + id.partialSuffix()
+	wc, partialPath, err := m.openWriterAndPaths(id, m.opts.Mode, inFile, baseName, dstPath, offset)
-	wc, partialPath, err := m.opts.fileOps.OpenWriter(partialName, offset, 0o666)
 	if err != nil {
 		return 0, m.redactAndLogError("Create", err)
 	}
 	defer func() {
 		wc.Close()
 		if err != nil {
-			m.deleter.Insert(partialName) // mark partial file for eventual deletion
+			m.deleter.Insert(filepath.Base(partialPath)) // mark partial file for eventual deletion
 		}
 	}()
 
-	// Check whether there is an in-progress transfer for the file.
-	inFileKey := incomingFileKey{id, baseName}
-	inFile, loaded := m.incomingFiles.LoadOrInit(inFileKey, func() *incomingFile {
-		inFile := &incomingFile{
-			clock:          m.opts.Clock,
-			started:        m.opts.Clock.Now(),
-			size:           length,
-			sendFileNotify: m.opts.SendFileNotify,
-		}
-		if m.opts.DirectFileMode {
-			inFile.partialPath = partialPath
-		}
-		return inFile
-	})
-
-	inFile.w = wc
-
-	if loaded {
-		return 0, ErrFileExists
-	}
-	defer m.incomingFiles.Delete(inFileKey)
-
 	// Record that we have started to receive at least one file.
 	// This is used by the deleter upon a cold-start to scan the directory
 	// for any files that need to be deleted.
@@ -146,26 +148,220 @@ func (m *manager) PutFile(id clientID, baseName string, r io.Reader, offset, len
 		return 0, m.redactAndLogError("Close", err)
 	}
 
-	fileLength = offset + copyLength
+	fileLength := offset + copyLength
 
 	inFile.mu.Lock()
 	inFile.done = true
 	inFile.mu.Unlock()
 
-	// 6) Finalize (rename/move) the partial into place via FileOps.Rename
+	// Finalize rename
-	finalPath, err := m.opts.fileOps.Rename(partialPath, baseName)
+	switch m.opts.Mode {
+	case PutModeDirect:
+		var finalDst string
+		finalDst, err = m.finalizeDirect(inFile, partialPath, dstPath, fileLength)
 		if err != nil {
 			return 0, m.redactAndLogError("Rename", err)
 		}
-	inFile.finalPath = finalPath
+		inFile.finalPath = finalDst
+
+	case PutModeAndroidSAF:
+		if err = m.finalizeSAF(partialPath, baseName); err != nil {
+			return 0, m.redactAndLogError("Rename", err)
+		}
+	}
 
 	m.totalReceived.Add(1)
 	m.opts.SendFileNotify()
 	return fileLength, nil
 }
 
+// openWriterAndPaths opens the correct writer, seeks/truncates if needed,
+// and sets inFile.partialPath & inFile.finalPath for later cleanup/rename.
+// The caller is responsible for closing the file on completion.
+func (m *manager) openWriterAndPaths(
+	id clientID,
+	mode PutMode,
+	inFile *incomingFile,
+	baseName string,
+	dstPath string,
+	offset int64,
+) (wc io.WriteCloser, partialPath string, err error) {
+	switch mode {
+
+	case PutModeDirect:
+		partialPath = dstPath + id.partialSuffix()
+		f, err := os.OpenFile(partialPath, os.O_CREATE|os.O_RDWR, 0o666)
+		if err != nil {
+			return nil, "", m.redactAndLogError("Create", err)
+		}
+		if offset != 0 {
+			curr, err := f.Seek(0, io.SeekEnd)
+			if err != nil {
+				f.Close()
+				return nil, "", m.redactAndLogError("Seek", err)
+			}
+			if offset < 0 || offset > curr {
+				f.Close()
+				return nil, "", m.redactAndLogError("Seek", fmt.Errorf("offset %d out of range", offset))
+			}
+			if _, err := f.Seek(offset, io.SeekStart); err != nil {
+				f.Close()
+				return nil, "", m.redactAndLogError("Seek", err)
+			}
+			if err := f.Truncate(offset); err != nil {
+				f.Close()
+				return nil, "", m.redactAndLogError("Truncate", err)
+			}
+		}
+		inFile.w = f
+		wc = f
+		inFile.partialPath = partialPath
+		inFile.finalPath = dstPath
+		return wc, partialPath, nil
+
+	case PutModeAndroidSAF:
+		if m.opts.FileOps == nil {
+			return nil, "", m.redactAndLogError("Create (SAF)", fmt.Errorf("missing FileOps"))
+		}
+		writer, uri, err := m.opts.FileOps.OpenFileWriter(baseName)
+		if err != nil {
+			return nil, "", m.redactAndLogError("Create (SAF)", fmt.Errorf("failed to open file for writing via SAF"))
+		}
+		if writer == nil || uri == "" {
+			return nil, "", fmt.Errorf("invalid SAF writer or URI")
+		}
+		// SAF mode does not support resuming, so enforce offset == 0.
+		if offset != 0 {
+			writer.Close()
+			return nil, "", m.redactAndLogError("Seek", fmt.Errorf("resuming is not supported in SAF mode"))
+		}
+		inFile.w = writer
+		wc = writer
+		partialPath = uri
+		inFile.partialPath = uri
+		inFile.finalPath = baseName
+		return wc, partialPath, nil
+
+	default:
+		return nil, "", fmt.Errorf("unsupported PutMode: %v", mode)
+	}
+}
+
+// finalizeDirect atomically renames or dedups the partial file, retrying
+// under new names up to 10 times. It returns the final path that succeeded.
+func (m *manager) finalizeDirect(
+	inFile *incomingFile,
+	partialPath string,
+	initialDst string,
+	fileLength int64,
+) (string, error) {
+	var (
+		once       sync.Once
+		cachedSum  [sha256.Size]byte
+		cacheErr   error
+		computeSum = func() ([sha256.Size]byte, error) {
+			once.Do(func() { cachedSum, cacheErr = sha256File(partialPath) })
+			return cachedSum, cacheErr
+		}
+	)
+
+	dstPath := initialDst
+	const maxRetries = 10
+	for i := 0; i < maxRetries; i++ {
+		// Atomically rename the partial file as the destination file if it doesn't exist.
+		// Otherwise, it returns the length of the current destination file.
+		// The operation is atomic.
+		lengthOnDisk, err := func() (int64, error) {
+			m.renameMu.Lock()
+			defer m.renameMu.Unlock()
+			fi, statErr := os.Stat(dstPath)
+			if os.IsNotExist(statErr) {
+				// dst missing → rename partial into place
+				return -1, os.Rename(partialPath, dstPath)
+			}
+			if statErr != nil {
+				return -1, statErr
+			}
+			return fi.Size(), nil
+		}()
+		if err != nil {
+			return "", err
+		}
+		if lengthOnDisk < 0 {
+			// successfully moved
+			inFile.finalPath = dstPath
+			return dstPath, nil
+		}
+
+		// Avoid the final rename if a destination file has the same contents.
+		//
+		// Note: this is best effort and copying files from iOS from the Media Library
+		// results in processing on the iOS side which means the size and shas of the
+		// same file can be different.
+		if lengthOnDisk == fileLength {
+			partSum, err := computeSum()
+			if err != nil {
+				return "", err
+			}
+			dstSum, err := sha256File(dstPath)
+			if err != nil {
+				return "", err
+			}
+			if partSum == dstSum {
+				// same content → drop the partial
+				if err := os.Remove(partialPath); err != nil {
+					return "", err
+				}
+				inFile.finalPath = dstPath
+				return dstPath, nil
+			}
+		}
+
+		// Choose a new destination filename and try again.
+		dstPath = nextFilename(dstPath)
+	}
+
+	return "", fmt.Errorf("too many retries trying to rename a partial file %q", initialDst)
+}
+
+// finalizeSAF retries RenamePartialFile up to 10 times, generating a new
+// name on each failure until the SAF URI changes.
+func (m *manager) finalizeSAF(
+	partialPath, finalName string,
+) error {
+	if m.opts.FileOps == nil {
+		return fmt.Errorf("missing FileOps for SAF finalize")
+	}
+	const maxTries = 10
+	name := finalName
+	for i := 0; i < maxTries; i++ {
+		newURI, err := m.opts.FileOps.RenamePartialFile(partialPath, m.opts.Dir, name)
+		if err != nil {
+			return err
+		}
+		if newURI != "" && newURI != name {
+			return nil
+		}
+		name = nextFilename(name)
+	}
+	return fmt.Errorf("failed to finalize SAF file after %d retries", maxTries)
+}
+
 func (m *manager) redactAndLogError(stage string, err error) error {
 	err = redactError(err)
 	m.opts.Logf("put %s error: %v", stage, err)
 	return err
 }
+
+func sha256File(file string) (out [sha256.Size]byte, err error) {
+	h := sha256.New()
+	f, err := os.Open(file)
+	if err != nil {
+		return out, err
+	}
+	defer f.Close()
+	if _, err := io.Copy(h, f); err != nil {
+		return out, err
+	}
+	return [sha256.Size]byte(h.Sum(nil)), nil
+}

feature/taildrop/send_test.go

@@ -4,64 +4,123 @@
 package taildrop
 
 import (
+	"bytes"
+	"fmt"
+	"io"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"tailscale.com/tstime"
-	"tailscale.com/util/must"
 )
 
+// nopWriteCloser is a no-op io.WriteCloser wrapping a bytes.Buffer.
+type nopWriteCloser struct{ *bytes.Buffer }
+
+func (nwc nopWriteCloser) Close() error { return nil }
+
+// mockFileOps implements just enough of the FileOps interface for SAF tests.
+type mockFileOps struct {
+	writes   *bytes.Buffer
+	renameOK bool
+}
+
+func (m *mockFileOps) OpenFileWriter(name string) (io.WriteCloser, string, error) {
+	m.writes = new(bytes.Buffer)
+	return nopWriteCloser{m.writes}, "uri://" + name + ".partial", nil
+}
+
+func (m *mockFileOps) RenamePartialFile(partialPath, dir, finalName string) (string, error) {
+	if !m.renameOK {
+		m.renameOK = true
+		return "uri://" + finalName, nil
+	}
+	return "", io.ErrUnexpectedEOF
+}
+
 func TestPutFile(t *testing.T) {
 	const content = "hello, world"
 
 	tests := []struct {
 		name     string
-		directFileMode bool
+		mode     PutMode
+		setup    func(t *testing.T) (*manager, string, *mockFileOps)
+		wantFile string
 	}{
-		{"DirectFileMode", true},
+		{
-		{"NonDirectFileMode", false},
+			name: "PutModeDirect",
-	}
+			mode: PutModeDirect,
-
+			setup: func(t *testing.T) (*manager, string, *mockFileOps) {
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
 				dir := t.TempDir()
-			mgr := managerOptions{
+				opts := managerOptions{
 					Logf:           t.Logf,
 					Clock:          tstime.DefaultClock{},
 					State:          nil,
-				fileOps:        must.Get(newFileOps(dir)),
+					Dir:            dir,
-				DirectFileMode: tt.directFileMode,
+					Mode:           PutModeDirect,
+					DirectFileMode: true,
 					SendFileNotify: func() {},
-			}.New()
+				}
+				mgr := opts.New()
+				return mgr, dir, nil
+			},
+			wantFile: "file.txt",
+		},
+		{
+			name: "PutModeAndroidSAF",
+			mode: PutModeAndroidSAF,
+			setup: func(t *testing.T) (*manager, string, *mockFileOps) {
+				// SAF still needs a non-empty Dir to pass the guard.
+				dir := t.TempDir()
+				mops := &mockFileOps{}
+				opts := managerOptions{
+					Logf:           t.Logf,
+					Clock:          tstime.DefaultClock{},
+					State:          nil,
+					Dir:            dir,
+					Mode:           PutModeAndroidSAF,
+					FileOps:        mops,
+					DirectFileMode: true,
+					SendFileNotify: func() {},
+				}
+				mgr := opts.New()
+				return mgr, dir, mops
+			},
+			wantFile: "file.txt",
+		},
+	}
 
-			id := clientID("0")
+	for _, tc := range tests {
-			n, err := mgr.PutFile(id, "file.txt", strings.NewReader(content), 0, int64(len(content)))
+		t.Run(tc.name, func(t *testing.T) {
+			mgr, dir, mops := tc.setup(t)
+			id := clientID(fmt.Sprint(0))
+			reader := bytes.NewReader([]byte(content))
+
+			n, err := mgr.PutFile(id, "file.txt", reader, 0, int64(len(content)))
 			if err != nil {
-				t.Fatalf("PutFile error: %v", err)
+				t.Fatalf("PutFile(%s) error: %v", tc.name, err)
 			}
 			if n != int64(len(content)) {
 				t.Errorf("wrote %d bytes; want %d", n, len(content))
 			}
 
-			path := filepath.Join(dir, "file.txt")
+			switch tc.mode {
-
+			case PutModeDirect:
-			got, err := os.ReadFile(path)
+				path := filepath.Join(dir, tc.wantFile)
+				data, err := os.ReadFile(path)
 				if err != nil {
-				t.Fatalf("ReadFile %q: %v", path, err)
+					t.Fatalf("ReadFile error: %v", err)
 				}
-			if string(got) != content {
+				if got := string(data); got != content {
-				t.Errorf("file contents = %q; want %q", string(got), content)
+					t.Errorf("file contents = %q; want %q", got, content)
 				}
 
-			entries, err := os.ReadDir(dir)
+			case PutModeAndroidSAF:
-			if err != nil {
+				if mops.writes == nil {
-				t.Fatal(err)
+					t.Fatal("SAF writer was never created")
 				}
-			for _, entry := range entries {
+				if got := mops.writes.String(); got != content {
-				if strings.Contains(entry.Name(), ".partial") {
+					t.Errorf("SAF writes = %q; want %q", got, content)
-					t.Errorf("unexpected partial file left behind: %s", entry.Name())
 				}
 			}
 		})

feature/taildrop/taildrop.go

@@ -12,6 +12,8 @@ package taildrop
 import (
 	"errors"
 	"hash/adler32"
+	"io"
+	"io/fs"
 	"os"
 	"path"
 	"path/filepath"
@@ -19,6 +21,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"unicode"
 	"unicode/utf8"
@@ -69,6 +72,11 @@ type managerOptions struct {
 	Clock tstime.DefaultClock // may be nil
 	State ipn.StateStore      // may be nil
 
+	// Dir is the directory to store received files.
+	// This main either be the final location for the files
+	// or just a temporary staging directory (see DirectFileMode).
+	Dir string
+
 	// DirectFileMode reports whether we are writing files
 	// directly to a download directory, rather than writing them to
 	// a temporary staging directory.
@@ -83,10 +91,9 @@ type managerOptions struct {
 	// copy them out, and then delete them.
 	DirectFileMode bool
 
-	// FileOps abstracts platform-specific file operations needed for file transfers.
+	FileOps FileOps
-	// Android's implementation uses the Storage Access Framework, and other platforms
+
-	// use fsFileOps.
+	Mode PutMode
-	fileOps FileOps
 
 	// SendFileNotify is called periodically while a file is actively
 	// receiving the contents for the file. There is a final call
@@ -104,6 +111,9 @@ type manager struct {
 	// deleter managers asynchronous deletion of files.
 	deleter fileDeleter
 
+	// renameMu is used to protect os.Rename calls so that they are atomic.
+	renameMu sync.Mutex
+
 	// totalReceived counts the cumulative total of received files.
 	totalReceived atomic.Int64
 	// emptySince specifies that there were no waiting files
@@ -127,6 +137,11 @@ func (opts managerOptions) New() *manager {
 	return m
 }
 
+// Dir returns the directory.
+func (m *manager) Dir() string {
+	return m.opts.Dir
+}
+
 // Shutdown shuts down the Manager.
 // It blocks until all spawned goroutines have stopped running.
 func (m *manager) Shutdown() {
@@ -157,30 +172,58 @@ func isPartialOrDeleted(s string) bool {
 	return strings.HasSuffix(s, deletedSuffix) || strings.HasSuffix(s, partialSuffix)
 }
 
-func validateBaseName(name string) error {
+func joinDir(dir, baseName string) (fullPath string, err error) {
-	if !utf8.ValidString(name) ||
+	if !utf8.ValidString(baseName) {
-		strings.TrimSpace(name) != name ||
+		return "", ErrInvalidFileName
-		len(name) > 255 {
+	}
-		return ErrInvalidFileName
+	if strings.TrimSpace(baseName) != baseName {
+		return "", ErrInvalidFileName
+	}
+	if len(baseName) > 255 {
+		return "", ErrInvalidFileName
 	}
 	// TODO: validate unicode normalization form too? Varies by platform.
-	clean := path.Clean(name)
+	clean := path.Clean(baseName)
-	if clean != name || clean == "." || clean == ".." {
+	if clean != baseName ||
-		return ErrInvalidFileName
+		clean == "." || clean == ".." ||
+		isPartialOrDeleted(clean) {
+		return "", ErrInvalidFileName
 	}
-	if isPartialOrDeleted(name) {
+	for _, r := range baseName {
-		return ErrInvalidFileName
-	}
-	for _, r := range name {
 		if !validFilenameRune(r) {
-			return ErrInvalidFileName
+			return "", ErrInvalidFileName
 		}
 	}
-	if !filepath.IsLocal(name) {
+	if !filepath.IsLocal(baseName) {
-		return ErrInvalidFileName
+		return "", ErrInvalidFileName
 	}
+	return filepath.Join(dir, baseName), nil
+}
+
+// rangeDir iterates over the contents of a directory, calling fn for each entry.
+// It continues iterating while fn returns true.
+// It reports the number of entries seen.
+func rangeDir(dir string, fn func(fs.DirEntry) bool) error {
+	f, err := os.Open(dir)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+	for {
+		des, err := f.ReadDir(10)
+		for _, de := range des {
+			if !fn(de) {
 				return nil
 			}
+		}
+		if err != nil {
+			if err == io.EOF {
+				return nil
+			}
+			return err
+		}
+	}
+}
 
 // IncomingFiles returns a list of active incoming files.
 func (m *manager) IncomingFiles() []ipn.PartialFile {

feature/taildrop/taildrop_test.go

@@ -4,10 +4,40 @@
 package taildrop
 
 import (
+	"path/filepath"
 	"strings"
 	"testing"
 )
 
+func TestJoinDir(t *testing.T) {
+	dir := t.TempDir()
+	tests := []struct {
+		in     string
+		want   string // just relative to m.Dir
+		wantOk bool
+	}{
+		{"", "", false},
+		{"foo", "foo", true},
+		{"./foo", "", false},
+		{"../foo", "", false},
+		{"foo/bar", "", false},
+		{"😋", "😋", true},
+		{"\xde\xad\xbe\xef", "", false},
+		{"foo.partial", "", false},
+		{"foo.deleted", "", false},
+		{strings.Repeat("a", 1024), "", false},
+		{"foo:bar", "", false},
+	}
+	for _, tt := range tests {
+		got, gotErr := joinDir(dir, tt.in)
+		got, _ = filepath.Rel(dir, got)
+		gotOk := gotErr == nil
+		if got != tt.want || gotOk != tt.wantOk {
+			t.Errorf("joinDir(%q) = (%v, %v), want (%v, %v)", tt.in, got, gotOk, tt.want, tt.wantOk)
+		}
+	}
+}
+
 func TestNextFilename(t *testing.T) {
 	tests := []struct {
 		in    string
@@ -37,29 +67,3 @@ func TestNextFilename(t *testing.T) {
 		}
 	}
 }
-
-func TestValidateBaseName(t *testing.T) {
-	tests := []struct {
-		in     string
-		wantOk bool
-	}{
-		{"", false},
-		{"foo", true},
-		{"./foo", false},
-		{"../foo", false},
-		{"foo/bar", false},
-		{"😋", true},
-		{"\xde\xad\xbe\xef", false},
-		{"foo.partial", false},
-		{"foo.deleted", false},
-		{strings.Repeat("a", 1024), false},
-		{"foo:bar", false},
-	}
-	for _, tt := range tests {
-		err := validateBaseName(tt.in)
-		gotOk := err == nil
-		if gotOk != tt.wantOk {
-			t.Errorf("validateBaseName(%q) = %v, wantOk = %v", tt.in, err, tt.wantOk)
-		}
-	}
-}