From 3d0599fca0cbe77536e6cf6c4aed60749e7e8c35 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Fri, 16 Apr 2021 12:33:04 -0700
Subject: [PATCH] ipn{,/ipnlocal}: in direct file receive mode, don't rename
 partial file

Let caller (macOS) do it so Finder progress bar can be dismissed
without races.

Updates tailscale/corp#1575

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 ipn/backend.go          | 15 +++++++++------
 ipn/ipnlocal/peerapi.go | 31 +++++++++++++++----------------
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/ipn/backend.go b/ipn/backend.go
index 8216d30b3..102dc2c97 100644
--- a/ipn/backend.go
+++ b/ipn/backend.go
@@ -95,12 +95,15 @@ type PartialFile struct {
 	DeclaredSize int64     // or -1 if unknown
 	Received     int64     // bytes copied thus far
 
-	// FinalPath is non-empty when the final has been completely
-	// written and renamed into place. This is then the complete
-	// path to the file post-rename. This is only set in
-	// "direct" file mode where the peerapi isn't being used; see
-	// LocalBackend.SetDirectFileRoot.
-	FinalPath string `json:",omitempty"`
+	// PartialPath is set non-empty in "direct" file mode to the
+	// in-progress '*.partial' file's path when the peerapi isn't
+	// being used; see LocalBackend.SetDirectFileRoot.
+	PartialPath 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
+	// ".partial" suffix.
+	Done bool `json:",omitempty"`
 }
 
 // StateKey is an opaque identifier for a set of LocalBackend state
diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go
index 9d3399291..c9411b9f3 100644
--- a/ipn/ipnlocal/peerapi.go
+++ b/ipn/ipnlocal/peerapi.go
@@ -381,21 +381,22 @@ This is my Tailscale device. Your device is %v.
 }
 
 type incomingFile struct {
-	name    string // "foo.jpg"
-	started time.Time
-	size    int64     // or -1 if unknown; never 0
-	w       io.Writer // underlying writer
-	ph      *peerAPIHandler
+	name        string // "foo.jpg"
+	started     time.Time
+	size        int64     // or -1 if unknown; never 0
+	w           io.Writer // underlying writer
+	ph          *peerAPIHandler
+	partialPath string // non-empty in direct mode
 
 	mu         sync.Mutex
 	copied     int64
+	done       bool
 	lastNotify time.Time
-	finalPath  string // non-empty in direct mode, when file is done
 }
 
-func (f *incomingFile) markAndNotifyDone(finalPath string) {
+func (f *incomingFile) markAndNotifyDone() {
 	f.mu.Lock()
-	f.finalPath = finalPath
+	f.done = true
 	f.mu.Unlock()
 	b := f.ph.ps.b
 	b.sendFileNotify()
@@ -432,7 +433,8 @@ func (f *incomingFile) PartialFile() ipn.PartialFile {
 		Started:      f.started,
 		DeclaredSize: f.size,
 		Received:     f.copied,
-		FinalPath:    f.finalPath,
+		PartialPath:  f.partialPath,
+		Done:         f.done,
 	}
 }
 
@@ -502,6 +504,9 @@ func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) {
 			w:       f,
 			ph:      h,
 		}
+		if h.ps.directFileMode {
+			inFile.partialPath = dstFile
+		}
 		h.ps.b.registerIncomingFile(inFile, true)
 		defer h.ps.b.registerIncomingFile(inFile, false)
 		n, err := io.Copy(inFile, r.Body)
@@ -519,14 +524,8 @@ func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if h.ps.directFileMode {
-		finalPath := strings.TrimSuffix(dstFile, partialSuffix)
-		if err := os.Rename(dstFile, finalPath); err != nil {
-			h.logf("Rename error: %v", err)
-			http.Error(w, "error renaming file", http.StatusInternalServerError)
-			return
-		}
 		if inFile != nil { // non-zero length; TODO: notify even for zero length
-			inFile.markAndNotifyDone(finalPath)
+			inFile.markAndNotifyDone()
 		}
 	}