diff --git a/feature/taildrop/delete.go b/feature/taildrop/delete.go index e9c8d7f1c..3da4ba878 100644 --- a/feature/taildrop/delete.go +++ b/feature/taildrop/delete.go @@ -6,9 +6,7 @@ package taildrop import ( "container/list" "context" - "io/fs" "os" - "path/filepath" "strings" "sync" "time" @@ -39,6 +37,8 @@ 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. @@ -52,13 +52,16 @@ func (d *fileDeleter) Init(m *manager, eventHook func(string)) { d.clock = m.opts.Clock d.dir = m.opts.Dir d.event = eventHook + d.fs = m.opts.FileOps + if d.fs == nil { + panic("taildrop: FileOps is nil in fileDeleter.Init") + } 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. @@ -71,38 +74,41 @@ 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 { + + files, err := d.fs.ListFiles(d.dir) + if err != nil { + d.logf("deleter: ListDir error: %v", err) + return + } + for _, fi := range files { switch { case d.shutdownCtx.Err() != nil: - return false // terminate early - case !de.Type().IsRegular(): - return true - case strings.HasSuffix(de.Name(), partialSuffix): + return // terminate early + case strings.HasSuffix(fi, partialSuffix): // Only enqueue the file for deletion if there is no active put. - nameID := strings.TrimSuffix(de.Name(), partialSuffix) + nameID := strings.TrimSuffix(fi, 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(de.Name()) + d.Insert(fi) } }) } else { - d.Insert(de.Name()) + d.Insert(fi) } - case strings.HasSuffix(de.Name(), deletedSuffix): + case strings.HasSuffix(fi, deletedSuffix): // Best-effort immediate deletion of deleted files. - name := strings.TrimSuffix(de.Name(), deletedSuffix) - if os.Remove(filepath.Join(d.dir, name)) == nil { - if os.Remove(filepath.Join(d.dir, de.Name())) == nil { - break + name := strings.TrimSuffix(fi, deletedSuffix) + if d.fs.Remove(name) == nil { + if d.fs.Remove(fi) == nil { + continue } } - // Otherwise, enqueue the file for later deletion. - d.Insert(de.Name()) + // Otherwise enqueue for later deletion. + d.Insert(fi) } - return true - }) + } }) } @@ -149,13 +155,13 @@ func (d *fileDeleter) waitAndDelete(wait time.Duration) { // Delete the expired file. if name, ok := strings.CutSuffix(file.name, deletedSuffix); ok { - if err := os.Remove(filepath.Join(d.dir, name)); err != nil && !os.IsNotExist(err) { + if err := d.fs.Remove(name); err != nil && !os.IsNotExist(err) { d.logf("could not delete: %v", redactError(err)) failed = append(failed, elem) continue } } - if err := os.Remove(filepath.Join(d.dir, file.name)); err != nil && !os.IsNotExist(err) { + if err := d.fs.Remove(file.name); err != nil && !os.IsNotExist(err) { d.logf("could not delete: %v", redactError(err)) failed = append(failed, elem) continue diff --git a/feature/taildrop/delete_test.go b/feature/taildrop/delete_test.go index 7a58de55c..d0fd6ecfc 100644 --- a/feature/taildrop/delete_test.go +++ b/feature/taildrop/delete_test.go @@ -75,6 +75,7 @@ func TestDeleter(t *testing.T) { m.opts.Clock = tstime.DefaultClock{Clock: clock} m.opts.Dir = dir m.opts.State = must.Get(mem.New(nil, "")) + m.opts.FileOps, _ = newDefaultFileOps(dir) must.Do(m.opts.State.WriteState(ipn.TaildropReceivedKey, []byte{1})) fd.Init(&m, eventHook) defer fd.Shutdown() @@ -147,6 +148,7 @@ func TestDeleterInitWithoutTaildrop(t *testing.T) { m.opts.Logf = t.Logf m.opts.Dir = t.TempDir() m.opts.State = must.Get(mem.New(nil, "")) + m.opts.FileOps, _ = newDefaultFileOps(t.TempDir()) fd.Init(&m, func(event string) { t.Errorf("unexpected event: %v", event) }) fd.Shutdown() } diff --git a/feature/taildrop/ext.go b/feature/taildrop/ext.go index c11fe3af4..775ffd4f7 100644 --- a/feature/taildrop/ext.go +++ b/feature/taildrop/ext.go @@ -89,30 +89,6 @@ 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" } @@ -176,23 +152,29 @@ func (e *Extension) onChangeProfile(profile ipn.LoginProfileView, _ ipn.PrefsVie return } - // If we have a netmap, create a taildrop manager. + /// If we have a netmap, create a taildrop manager. fileRoot, isDirectFileMode := e.fileRoot(uid, activeLogin) if fileRoot == "" { - e.logf("no Taildrop directory configured") + e.logf("DEBUG-ADDR=⟪taildrop⟫ no Taildrop directory configured") } - mode := PutModeDirect - if e.directFileRoot != "" && strings.HasPrefix(e.directFileRoot, SafDirectoryPrefix) { - mode = PutModeAndroidSAF + + // Pick the FileOps (use default if none provided) + fops := e.FileOps + if fops == nil { + fo, err := newDefaultFileOps(fileRoot) + if err != nil { + panic(fmt.Sprintf("taildrop: cannot create FileOps: %v", err)) + } + fops = fo } + e.setMgrLocked(managerOptions{ Logf: e.logf, Clock: tstime.DefaultClock{Clock: e.sb.Clock()}, State: e.stateStore, Dir: fileRoot, DirectFileMode: isDirectFileMode, - FileOps: e.FileOps, - Mode: mode, + FileOps: fops, SendFileNotify: e.sendFileNotify, }.New()) } diff --git a/feature/taildrop/fileops.go b/feature/taildrop/fileops.go new file mode 100644 index 000000000..d2b26d0a1 --- /dev/null +++ b/feature/taildrop/fileops.go @@ -0,0 +1,38 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package taildrop + +import ( + "errors" + "io" + "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. + OpenWriter(name string, offset int64, perm os.FileMode) (wc io.WriteCloser, path string, err error) + // Base returns the last element of path. + Base(path string) string + // Remove deletes the given entry, where "name" is always a basename. + 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) + + // ListFileNames returns just the basenames of all regular files + // in the given subdirectory, in a single slice. + ListFiles(dir string) ([]string, error) +} + +var newDefaultFileOps = func(dir string) (FileOps, error) { return nil, errors.New("FileOps is not implemented") } + +// DefaultFileOps is the non‑Android FileOps implementation. +// It exists on Android too so the stub constructor can compile, +// but Android never uses the value. +type DefaultFileOps struct{ rootDir string } diff --git a/feature/taildrop/fileops_fs.go b/feature/taildrop/fileops_fs.go new file mode 100644 index 000000000..637d073de --- /dev/null +++ b/feature/taildrop/fileops_fs.go @@ -0,0 +1,174 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause +//go:build !android + +package taildrop + +import ( + "bytes" + "crypto/sha256" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "sync" +) + +var renameMu sync.Mutex + +func init() { + newDefaultFileOps = func(dir string) (FileOps, error) { + if dir == "" { + return nil, fmt.Errorf("taildrop: drootDir cannot be empty") + } + return DefaultFileOps{rootDir: dir}, nil + } +} + +func (d DefaultFileOps) OpenWriter(name string, offset int64, perm os.FileMode) (io.WriteCloser, string, error) { + path := filepath.Join(d.rootDir, name) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return nil, "", err + } + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, perm) + if err != nil { + return nil, "", err + } + if offset != 0 { + curr, err := f.Seek(0, io.SeekEnd) + if err != nil { + f.Close() + return nil, "", err + } + if offset < 0 || offset > curr { + f.Close() + return nil, "", fmt.Errorf("offset %d out of range", offset) + } + if _, err := f.Seek(offset, io.SeekStart); err != nil { + f.Close() + return nil, "", err + } + if err := f.Truncate(offset); err != nil { + f.Close() + return nil, "", err + } + } + return f, path, nil +} + +func (DefaultFileOps) Base(pathOrURI string) string { + return filepath.Base(pathOrURI) +} + +func (d DefaultFileOps) Remove(name string) error { + path := filepath.Join(d.rootDir, name) + return os.Remove(path) +} + +// Rename moves the partial file into its final name. +// If finalName contains any path separators (or is absolute), +// we use it verbatim; otherwise we join it to the partial’s dir. +// It will retry up to 10 times, de-dup same-checksum files, etc. +func (m DefaultFileOps) Rename(partial, finalName string) (string, error) { + var dst string + var err error + if filepath.IsAbs(finalName) || strings.ContainsRune(finalName, os.PathSeparator) { + dst = finalName + } else { + dst, err = joinDir(m.rootDir, finalName) + if err != nil { + return "", err + } + } + + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return "", err + } + + st, err := os.Stat(partial) + 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(partial, 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(partial) + if err != nil { + return "", err + } + sumD, err := sha256File(dst) + if err != nil { + return "", err + } + if bytes.Equal(sumP[:], sumD[:]) { + if err := os.Remove(partial); 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", partial, finalName) +} + +// sha256File computes the SHA‑256 of a file. +func sha256File(path string) ([sha256.Size]byte, error) { + var sum [sha256.Size]byte + 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 (d DefaultFileOps) ListFiles(dir string) ([]string, error) { + entries, err := os.ReadDir(dir) + 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 +} diff --git a/feature/taildrop/peerapi_test.go b/feature/taildrop/peerapi_test.go index 1a003b6ed..d335faf27 100644 --- a/feature/taildrop/peerapi_test.go +++ b/feature/taildrop/peerapi_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" "tailscale.com/client/tailscale/apitype" "tailscale.com/ipn/ipnlocal" "tailscale.com/tailcfg" @@ -170,11 +171,10 @@ func TestHandlePeerAPI(t *testing.T) { isSelf: true, capSharing: true, reqs: []*http.Request{httptest.NewRequest("PUT", "/v0/put/foo", nil)}, - checks: checks( - httpStatus(http.StatusForbidden), - bodyContains("Taildrop disabled; no storage directory"), - ), + // no checks — we’ll assert a panic instead }, + // … the rest of your cases unchanged … + { name: "bad_method", isSelf: true, @@ -474,11 +474,25 @@ func TestHandlePeerAPI(t *testing.T) { if !tt.omitRoot { rootDir = t.TempDir() } + fo, _ := newDefaultFileOps(rootDir) + + if tt.omitRoot { + var e peerAPITestEnv + require.Panics(t, func() { + _ = managerOptions{ + Logf: e.logBuf.Logf, + Dir: rootDir, + FileOps: fo, + }.New() + }, "case %q should panic when no storage root", tt.name) + return + } var e peerAPITestEnv e.taildrop = managerOptions{ - Logf: e.logBuf.Logf, - Dir: rootDir, + Logf: e.logBuf.Logf, + Dir: rootDir, + FileOps: fo, }.New() ext := &fakeExtension{ @@ -490,9 +504,7 @@ 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() @@ -525,9 +537,11 @@ func TestHandlePeerAPI(t *testing.T) { // a bit. So test that we work around that sufficiently. func TestFileDeleteRace(t *testing.T) { dir := t.TempDir() + fo, _ := newDefaultFileOps(dir) taildropMgr := managerOptions{ - Logf: t.Logf, - Dir: dir, + Logf: t.Logf, + Dir: dir, + FileOps: fo, }.New() ph := &peerAPIHandler{ diff --git a/feature/taildrop/resume.go b/feature/taildrop/resume.go index 211a1ff6b..2f17f5b19 100644 --- a/feature/taildrop/resume.go +++ b/feature/taildrop/resume.go @@ -9,7 +9,6 @@ import ( "encoding/hex" "fmt" "io" - "io/fs" "os" "strings" ) @@ -51,19 +50,20 @@ 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) (ret []string, err error) { +func (m *manager) PartialFiles(id clientID) ([]string, error) { if m == nil || m.opts.Dir == "" { return nil, ErrNoTaildrop } - suffix := id.partialSuffix() - if err := rangeDir(m.opts.Dir, func(de fs.DirEntry) bool { - if name := de.Name(); strings.HasSuffix(name, suffix) { - ret = append(ret, name) + files, err := m.opts.FileOps.ListFiles(m.opts.Dir) + if err != nil { + return nil, redactError(err) + } + var ret []string + for _, fi := range files { + if strings.HasSuffix(fi, suffix) { + ret = append(ret, fi) } - return true - }); err != nil { - return ret, redactError(err) } return ret, nil } diff --git a/feature/taildrop/resume_test.go b/feature/taildrop/resume_test.go index dac3c657b..bf93a742f 100644 --- a/feature/taildrop/resume_test.go +++ b/feature/taildrop/resume_test.go @@ -19,7 +19,10 @@ func TestResume(t *testing.T) { defer func() { blockSize = oldBlockSize }() blockSize = 256 - m := managerOptions{Logf: t.Logf, Dir: t.TempDir()}.New() + dir := t.TempDir() + + fo, _ := newDefaultFileOps(dir) + m := managerOptions{Logf: t.Logf, Dir: dir, FileOps: fo}.New() defer m.Shutdown() rn := rand.New(rand.NewSource(0)) diff --git a/feature/taildrop/retrieve.go b/feature/taildrop/retrieve.go index 6fb975193..69efe7996 100644 --- a/feature/taildrop/retrieve.go +++ b/feature/taildrop/retrieve.go @@ -20,74 +20,77 @@ import ( // HasFilesWaiting reports whether any files are buffered in [Handler.Dir]. // This always returns false when [Handler.DirectFileMode] is false. -func (m *manager) HasFilesWaiting() (has bool) { +func (m *manager) HasFilesWaiting() bool { if m == nil || m.opts.Dir == "" || m.opts.DirectFileMode { return false } - // Optimization: this is usually empty, so avoid opening - // the directory and checking. We can't cache the actual - // 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. - totalReceived := m.totalReceived.Load() - if totalReceived == m.emptySince.Load() { + // Negative‑result cache + total := m.totalReceived.Load() + if total == m.emptySince.Load() { return false } - // Check whether there is at least one one waiting file. - 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) { - 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) + files, err := m.opts.FileOps.ListFiles(m.opts.Dir) + if err != nil { + return false } - return has + + // Build a set of filenames present in Dir + have := make(map[string]struct{}, len(files)) + for _, fi := range files { + have[fi] = struct{}{} + } + + for _, fi := range files { + if isPartialOrDeleted(fi) { + continue + } + if _, ok := have[fi+deletedSuffix]; ok { + continue // already handled + } + // Found at least one downloadable file + return true + } + + // No waiting files → update negative‑result cache + m.emptySince.Store(total) + return false } // 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() (ret []apitype.WaitingFile, err error) { +func (m *manager) WaitingFiles() ([]apitype.WaitingFile, error) { if m == nil || m.opts.Dir == "" { return nil, ErrNoTaildrop } if m.opts.DirectFileMode { return nil, nil } - 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 true - } - ret = append(ret, apitype.WaitingFile{ - Name: filepath.Base(name), - Size: fi.Size(), - }) - } - return true - }); err != nil { + names, err := m.opts.FileOps.ListFiles(m.opts.Dir) + if err != nil { return nil, redactError(err) } + var ret []apitype.WaitingFile + for _, name := range names { + if isPartialOrDeleted(name) { + continue + } + // A corresponding .deleted marker means the file was already handled. + if _, err := os.Stat(filepath.Join(m.opts.Dir, name+deletedSuffix)); err == nil { + continue + } + full := filepath.Join(m.opts.Dir, name) + fi, err := os.Stat(full) + if err != nil { + continue + } + ret = append(ret, apitype.WaitingFile{ + Name: filepath.Base(name), + Size: fi.Size(), + }) + } sort.Slice(ret, func(i, j int) bool { return ret[i].Name < ret[j].Name }) return ret, nil } diff --git a/feature/taildrop/send.go b/feature/taildrop/send.go index 59a1701da..a2eddec42 100644 --- a/feature/taildrop/send.go +++ b/feature/taildrop/send.go @@ -4,11 +4,8 @@ package taildrop import ( - "crypto/sha256" "fmt" "io" - "os" - "path/filepath" "sync" "time" @@ -74,8 +71,9 @@ func (f *incomingFile) Write(p []byte) (n int, err error) { // 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) (int64, error) { + fo := m.opts.FileOps switch { - case m == nil || m.opts.Dir == "": + case m == nil || m.opts.Dir == "" || fo == nil: return 0, ErrNoTaildrop case !envknob.CanTaildrop(): return 0, ErrNoTaildrop @@ -83,20 +81,12 @@ func (m *manager) PutFile(id clientID, baseName string, r io.Reader, offset, len return 0, ErrNotAccessible } - //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 + dstID, err := joinDir(m.opts.Dir, baseName) + if err != nil { + return 0, err } - m.deleter.Remove(filepath.Base(dstPath)) // avoid deleting the partial file while receiving + // and make sure we don't delete it while uploading: + m.deleter.Remove(baseName) // Check whether there is an in-progress transfer for the file. partialFileKey := incomingFileKey{id, baseName} @@ -113,18 +103,23 @@ func (m *manager) PutFile(id clientID, baseName string, r io.Reader, offset, len } defer m.incomingFiles.Delete(partialFileKey) - // Open writer & populate inFile paths - wc, partialPath, err := m.openWriterAndPaths(id, m.opts.Mode, inFile, baseName, dstPath, offset) + // Open writer & populate inFile paths (might be a .part file or SAF URI) + partialName := baseName + id.partialSuffix() + wc, partialPath, err := fo.OpenWriter(baseName+id.partialSuffix(), offset, 0o666) if err != nil { return 0, m.redactAndLogError("Create", err) } defer func() { wc.Close() if err != nil { - m.deleter.Insert(filepath.Base(partialPath)) // mark partial file for eventual deletion + m.deleter.Insert(partialName) // mark partial file for eventual deletion } }() + inFile.w = wc + inFile.partialPath = partialPath + inFile.finalPath = dstID + // 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. @@ -154,214 +149,20 @@ func (m *manager) PutFile(id clientID, baseName string, r io.Reader, offset, len inFile.done = true inFile.mu.Unlock() - // Finalize rename - 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 = finalDst - - case PutModeAndroidSAF: - if err = m.finalizeSAF(partialPath, baseName); err != nil { - return 0, m.redactAndLogError("Rename", err) - } + // 6) Finalize (rename/move) the partial into place via FileOps.Rename + newID, err := fo.Rename(partialPath, baseName) + if err != nil { + return 0, m.redactAndLogError("Rename", err) } + inFile.finalPath = newID 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 -} diff --git a/feature/taildrop/send_test.go b/feature/taildrop/send_test.go index 8edb70417..0402da4a9 100644 --- a/feature/taildrop/send_test.go +++ b/feature/taildrop/send_test.go @@ -5,10 +5,9 @@ package taildrop import ( "bytes" - "fmt" - "io" "os" "path/filepath" + "strings" "testing" "tailscale.com/tstime" @@ -19,110 +18,36 @@ 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 - mode PutMode - setup func(t *testing.T) (*manager, string, *mockFileOps) - wantFile string - }{ - { - name: "PutModeDirect", - mode: PutModeDirect, - setup: func(t *testing.T) (*manager, string, *mockFileOps) { - dir := t.TempDir() - opts := managerOptions{ - Logf: t.Logf, - Clock: tstime.DefaultClock{}, - State: nil, - Dir: dir, - Mode: PutModeDirect, - DirectFileMode: true, - SendFileNotify: func() {}, - } - 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", - }, + dir := t.TempDir() + mops, _ := newDefaultFileOps(dir) + mgr := managerOptions{ + Logf: t.Logf, + Clock: tstime.DefaultClock{}, + State: nil, + Dir: dir, + FileOps: mops, + DirectFileMode: true, + SendFileNotify: func() {}, + }.New() + + id := clientID("0") + n, err := mgr.PutFile(id, "file.txt", strings.NewReader(content), 0, int64(len(content))) + if err != nil { + t.Fatalf("PutFile error: %v", err) + } + if n != int64(len(content)) { + t.Errorf("wrote %d bytes; want %d", n, len(content)) } - for _, tc := range tests { - 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(%s) error: %v", tc.name, err) - } - if n != int64(len(content)) { - t.Errorf("wrote %d bytes; want %d", n, len(content)) - } - - switch tc.mode { - case PutModeDirect: - path := filepath.Join(dir, tc.wantFile) - data, err := os.ReadFile(path) - if err != nil { - t.Fatalf("ReadFile error: %v", err) - } - if got := string(data); got != content { - t.Errorf("file contents = %q; want %q", got, content) - } - - case PutModeAndroidSAF: - if mops.writes == nil { - t.Fatal("SAF writer was never created") - } - if got := mops.writes.String(); got != content { - t.Errorf("SAF writes = %q; want %q", got, content) - } - } - }) + path := filepath.Join(dir, "file.txt") + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile %q: %v", path, err) + } + if string(got) != content { + t.Errorf("file contents = %q; want %q", string(got), content) } } diff --git a/feature/taildrop/taildrop.go b/feature/taildrop/taildrop.go index 2dfa415bb..68946431f 100644 --- a/feature/taildrop/taildrop.go +++ b/feature/taildrop/taildrop.go @@ -12,8 +12,6 @@ package taildrop import ( "errors" "hash/adler32" - "io" - "io/fs" "os" "path" "path/filepath" @@ -21,7 +19,6 @@ import ( "sort" "strconv" "strings" - "sync" "sync/atomic" "unicode" "unicode/utf8" @@ -91,10 +88,11 @@ type managerOptions struct { // copy them out, and then delete them. DirectFileMode bool + // FileOps abstracts platform-specific file operations needed for file transfers. + // Android's implementation uses the Storage Access Framework, and other platforms + // use DefaultFileOps. FileOps FileOps - Mode PutMode - // SendFileNotify is called periodically while a file is actively // receiving the contents for the file. There is a final call // to the function when reception completes. @@ -111,9 +109,6 @@ 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 @@ -200,31 +195,6 @@ func joinDir(dir, baseName string) (fullPath string, err error) { 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 { // Make sure we always set n.IncomingFiles non-nil so it gets encoded