From 7c828ffe092edd0b02f34042f391fb1fc526a433 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Thu, 28 Mar 2024 12:12:48 +0000
Subject: [PATCH] operations: fix very long file names when using copy with
 --partial

Before this change we were using the wrong variable to read the
filename length from. This meant that very long filenames were not
being truncated as intended.

This problem was spotted by Wang Zhiwei on the forum in a code review.

See: https://forum.rclone.org/t/why-use-c-remoteforcopy-instead-of-c-remote-to-check-length-in-copy-operation/45099
---
 fs/operations/copy.go      |  4 ++--
 fs/operations/copy_test.go | 43 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/fs/operations/copy.go b/fs/operations/copy.go
index e04b8a9e4..45fb3f64a 100644
--- a/fs/operations/copy.go
+++ b/fs/operations/copy.go
@@ -98,9 +98,9 @@ func (c *copy) checkPartial() (remoteForCopy string, inplace bool, err error) {
 	// Avoid making the leaf name longer if it's already lengthy to avoid
 	// trouble with file name length limits.
 	suffix := "." + random.String(8) + c.ci.PartialSuffix
-	base := path.Base(c.remoteForCopy)
+	base := path.Base(remoteForCopy)
 	if len(base) > 100 {
-		remoteForCopy = TruncateString(c.remoteForCopy, len(c.remoteForCopy)-len(suffix)) + suffix
+		remoteForCopy = TruncateString(remoteForCopy, len(remoteForCopy)-len(suffix)) + suffix
 	} else {
 		remoteForCopy += suffix
 	}
diff --git a/fs/operations/copy_test.go b/fs/operations/copy_test.go
index f309b0060..73aa5e05a 100644
--- a/fs/operations/copy_test.go
+++ b/fs/operations/copy_test.go
@@ -5,6 +5,9 @@ import (
 	"crypto/rand"
 	"errors"
 	"fmt"
+	"os"
+	"path"
+	"sort"
 	"strings"
 	"testing"
 
@@ -121,6 +124,46 @@ func TestCopyFile(t *testing.T) {
 	r.CheckRemoteItems(t, file2)
 }
 
+// Find the longest file name for writing to local
+func maxLengthFileName(t *testing.T, r *fstest.Run) string {
+	require.NoError(t, r.Flocal.Mkdir(context.Background(), "")) // create the root
+	const maxLen = 16 * 1024
+	name := strings.Repeat("A", maxLen)
+	i := sort.Search(len(name), func(i int) (fail bool) {
+		filePath := path.Join(r.LocalName, name[:i])
+		err := os.WriteFile(filePath, []byte{0}, 0777)
+		if err != nil {
+			return true
+		}
+		err = os.Remove(filePath)
+		if err != nil {
+			t.Logf("Failed to remove test file: %v", err)
+		}
+		return false
+	})
+	return name[:i-1]
+}
+
+// Check we can copy a file of maximum name length
+func TestCopyLongFile(t *testing.T) {
+	ctx := context.Background()
+	r := fstest.NewRun(t)
+	if !r.Fremote.Features().IsLocal {
+		t.Skip("Test only runs on local")
+	}
+
+	// Find the maximum length of file we can write
+	name := maxLengthFileName(t, r)
+	t.Logf("Max length of file name is %d", len(name))
+	file1 := r.WriteFile(name, "file1 contents", t1)
+	r.CheckLocalItems(t, file1)
+
+	err := operations.CopyFile(ctx, r.Fremote, r.Flocal, file1.Path, file1.Path)
+	require.NoError(t, err)
+	r.CheckLocalItems(t, file1)
+	r.CheckRemoteItems(t, file1)
+}
+
 func TestCopyFileBackupDir(t *testing.T) {
 	ctx := context.Background()
 	ctx, ci := fs.AddConfig(ctx)