taildrop: remove breaking abstraction layers for apple (#10728)

Removes the avoidFinalRename logic and all associated code as it is no longer required by the Apple clients.
Enables resume logic to be usable for Apple clients.

Fixes tailscale/corp#14772

Signed-off-by: Rhea Ghosh <rhea@tailscale.com>

cmd/tailscaled/taildrop.go

@@ -27,7 +27,6 @@ func configureTaildrop(logf logger.Logf, lb *ipnlocal.LocalBackend) {
 		} else {
 			logf("%s Taildrop: using %v", dg, path)
 			lb.SetDirectFileRoot(path)
-			lb.SetDirectFileDoFinalRename(true)
 		}
 	}
 

ipn/backend.go

@@ -175,6 +175,7 @@ type PartialFile struct {
 	// in-progress '*.partial' file's path when the peerapi isn't
 	// being used; see LocalBackend.SetDirectFileRoot.
 	PartialPath string `json:",omitempty"`
+	FinalPath   string `json:",omitempty"`
 
 	// Done is set in "direct" mode when the partial file has been
 	// closed and is ready for the caller to rename away the

ipn/ipnlocal/local.go

@@ -265,9 +265,8 @@ type LocalBackend struct {
 	// It's also used on several NAS platforms (Synology, TrueNAS, etc)
 	// but in that case DoFinalRename is also set true, which moves the
 	// *.partial file to its final name on completion.
-	directFileRoot          string
-	directFileDoFinalRename bool // false on macOS, true on several NAS platforms
-	componentLogUntil       map[string]componentLogState
+	directFileRoot    string
+	componentLogUntil map[string]componentLogState
 	// c2nUpdateStatus is the status of c2n-triggered client update.
 	c2nUpdateStatus     updateStatus
 	currentUser         ipnauth.WindowsToken
@@ -540,17 +539,6 @@ func (b *LocalBackend) SetDirectFileRoot(dir string) {
 	b.directFileRoot = dir
 }
 
-// SetDirectFileDoFinalRename sets whether the peerapi file server should rename
-// a received "name.partial" file to "name" when the download is complete.
-//
-// This only applies when SetDirectFileRoot is non-empty.
-// The default is false.
-func (b *LocalBackend) SetDirectFileDoFinalRename(v bool) {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-	b.directFileDoFinalRename = v
-}
-
 // ReloadConfig reloads the backend's config from disk.
 //
 // It returns (false, nil) if not running in declarative mode, (true, nil) on
@@ -3877,13 +3865,12 @@ func (b *LocalBackend) initPeerAPIListener() {
 	ps := &peerAPIServer{
 		b: b,
 		taildrop: taildrop.ManagerOptions{
-			Logf:             b.logf,
-			Clock:            tstime.DefaultClock{Clock: b.clock},
-			State:            b.store,
-			Dir:              fileRoot,
-			DirectFileMode:   b.directFileRoot != "",
-			AvoidFinalRename: !b.directFileDoFinalRename,
-			SendFileNotify:   b.sendFileNotify,
+			Logf:           b.logf,
+			Clock:          tstime.DefaultClock{Clock: b.clock},
+			State:          b.store,
+			Dir:            fileRoot,
+			DirectFileMode: b.directFileRoot != "",
+			SendFileNotify: b.sendFileNotify,
 		}.New(),
 	}
 	if dm, ok := b.sys.DNSManager.GetOK(); ok {

taildrop/resume.go

@@ -64,9 +64,6 @@ func (m *Manager) PartialFiles(id ClientID) (ret []string, err error) {
 	if m == nil || m.opts.Dir == "" {
 		return nil, ErrNoTaildrop
 	}
-	if m.opts.DirectFileMode && m.opts.AvoidFinalRename {
-		return nil, nil // resuming is not supported for users that peek at our file structure
-	}
 
 	suffix := id.partialSuffix()
 	if err := rangeDir(m.opts.Dir, func(de fs.DirEntry) bool {
@@ -90,9 +87,6 @@ func (m *Manager) HashPartialFile(id ClientID, baseName string) (next func() (Bl
 	}
 	noopNext := func() (BlockChecksum, error) { return BlockChecksum{}, io.EOF }
 	noopClose := func() error { return nil }
-	if m.opts.DirectFileMode && m.opts.AvoidFinalRename {
-		return noopNext, noopClose, nil // resuming is not supported for users that peek at our file structure
-	}
 
 	dstFile, err := joinDir(m.opts.Dir, baseName)
 	if err != nil {

taildrop/send.go

@@ -31,6 +31,7 @@ type incomingFile struct {
 	w              io.Writer // underlying writer
 	sendFileNotify func()    // called when done
 	partialPath    string    // non-empty in direct mode
+	finalPath      string    // not used in direct mode
 
 	mu         sync.Mutex
 	copied     int64
@@ -92,13 +93,6 @@ func (m *Manager) PutFile(id ClientID, baseName string, r io.Reader, offset, len
 		return err
 	}
 
-	avoidPartialRename := m.opts.DirectFileMode && m.opts.AvoidFinalRename
-	if avoidPartialRename {
-		// Users using AvoidFinalRename are depending on the exact filename
-		// of the partial files. So avoid injecting the id into it.
-		id = ""
-	}
-
 	// Check whether there is an in-progress transfer for the file.
 	partialPath := dstPath + id.partialSuffix()
 	inFileKey := incomingFileKey{id, baseName}
@@ -111,6 +105,7 @@ func (m *Manager) PutFile(id ClientID, baseName string, r io.Reader, offset, len
 		}
 		if m.opts.DirectFileMode {
 			inFile.partialPath = partialPath
+			inFile.finalPath = dstPath
 		}
 		return inFile
 	})
@@ -128,10 +123,6 @@ func (m *Manager) PutFile(id ClientID, baseName string, r io.Reader, offset, len
 	defer func() {
 		f.Close() // best-effort to cleanup dangling file handles
 		if err != nil {
-			if avoidPartialRename {
-				os.Remove(partialPath) // best-effort
-				return
-			}
 			m.deleter.Insert(filepath.Base(partialPath)) // mark partial file for eventual deletion
 		}
 	}()
@@ -179,16 +170,9 @@ func (m *Manager) PutFile(id ClientID, baseName string, r io.Reader, offset, len
 	}
 	fileLength := offset + copyLength
 
-	// Return early for avoidPartialRename since users of AvoidFinalRename
-	// are depending on the exact naming of partial files.
-	if avoidPartialRename {
-		inFile.mu.Lock()
-		inFile.done = true
-		inFile.mu.Unlock()
-		m.totalReceived.Add(1)
-		m.opts.SendFileNotify()
-		return fileLength, nil
-	}
+	inFile.mu.Lock()
+	inFile.done = true
+	inFile.mu.Unlock()
 
 	// File has been successfully received, rename the partial file
 	// to the final destination filename. If a file of that name already exists,
@@ -221,6 +205,10 @@ func (m *Manager) PutFile(id ClientID, baseName string, r io.Reader, offset, len
 		}
 
 		// 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 dstLength == fileLength {
 			partialSum, err := computePartialSum()
 			if err != nil {
@@ -240,6 +228,7 @@ func (m *Manager) PutFile(id ClientID, baseName string, r io.Reader, offset, len
 
 		// Choose a new destination filename and try again.
 		dstPath = NextFilename(dstPath)
+		inFile.finalPath = dstPath
 	}
 	if maxRetries <= 0 {
 		return 0, errors.New("too many retries trying to rename partial file")

taildrop/taildrop.go

@@ -90,15 +90,6 @@ type ManagerOptions struct {
 	// copy them out, and then delete them.
 	DirectFileMode bool
 
-	// AvoidFinalRename specifies whether in DirectFileMode
-	// we should avoid renaming "foo.jpg.partial" to "foo.jpg" after reception.
-	//
-	// TODO(joetsai,rhea): Delete this. This is currently depended upon
-	// in the Apple platforms since it violates the abstraction layer
-	// and directly assumes how taildrop represents partial files.
-	// Right now, file resumption does not work on Apple.
-	AvoidFinalRename bool
-
 	// 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.
@@ -244,6 +235,7 @@ func (m *Manager) IncomingFiles() []ipn.PartialFile {
 			DeclaredSize: f.size,
 			Received:     f.copied,
 			PartialPath:  f.partialPath,
+			FinalPath:    f.finalPath,
 			Done:         f.done,
 		})
 		return true