From 584523672c034d89fb315d959d1988e0b736b94a Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Fri, 27 Nov 2020 11:49:37 +0000
Subject: [PATCH] dropbox: test file name length before upload to fix upload
 loop

Before this change rclone would upload the whole of multipart files
before receiving a message from dropbox that the path was too long.

This change hard codes the 255 rune limit and checks that before
uploading any files.

Fixes #4805
---
 backend/dropbox/dropbox.go               | 36 +++++++++++++++++++
 backend/dropbox/dropbox_internal_test.go | 44 ++++++++++++++++++++++++
 fs/fs.go                                 |  1 +
 3 files changed, 81 insertions(+)
 create mode 100644 backend/dropbox/dropbox_internal_test.go

diff --git a/backend/dropbox/dropbox.go b/backend/dropbox/dropbox.go
index 180cd9551..350b2d4e0 100755
--- a/backend/dropbox/dropbox.go
+++ b/backend/dropbox/dropbox.go
@@ -30,6 +30,7 @@ import (
 	"regexp"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"github.com/dropbox/dropbox-sdk-go-unofficial/dropbox"
 	"github.com/dropbox/dropbox-sdk-go-unofficial/dropbox/auth"
@@ -86,6 +87,8 @@ const (
 	// by default.
 	defaultChunkSize = 48 * fs.MebiByte
 	maxChunkSize     = 150 * fs.MebiByte
+	// Max length of filename parts: https://help.dropbox.com/installs-integrations/sync-uploads/files-not-syncing
+	maxFileNameLength = 255
 )
 
 var (
@@ -839,6 +842,10 @@ func (f *Fs) Mkdir(ctx context.Context, dir string) error {
 	arg2 := files.CreateFolderArg{
 		Path: f.opt.Enc.FromStandardPath(root),
 	}
+	// Don't attempt to create filenames that are too long
+	if cErr := checkPathLength(arg2.Path); cErr != nil {
+		return cErr
+	}
 	err = f.pacer.Call(func() (bool, error) {
 		_, err = f.srv.CreateFolderV2(&arg2)
 		return shouldRetry(err)
@@ -1417,6 +1424,31 @@ func (o *Object) uploadChunked(in0 io.Reader, commitInfo *files.CommitInfo, size
 	return entry, nil
 }
 
+// checks all the parts of name to see they are below
+// maxFileNameLength runes.
+//
+// This checks the length as runes which isn't quite right as dropbox
+// seems to encode some symbols (eg ☺) as two "characters". This seems
+// like utf-16 except that ☺ doesn't need two characters in utf-16.
+//
+// Using runes instead of what dropbox is using will work for most
+// cases, and when it goes wrong we will upload something we should
+// have detected as too long which is the least damaging way to fail.
+func checkPathLength(name string) (err error) {
+	for next := ""; len(name) > 0; name = next {
+		if slash := strings.IndexRune(name, '/'); slash >= 0 {
+			name, next = name[:slash], name[slash+1:]
+		} else {
+			next = ""
+		}
+		length := utf8.RuneCountInString(name)
+		if length > maxFileNameLength {
+			return fserrors.NoRetryError(fs.ErrorFileNameTooLong)
+		}
+	}
+	return nil
+}
+
 // Update the already existing object
 //
 // Copy the reader into the object updating modTime and size
@@ -1434,6 +1466,10 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op
 	commitInfo.Mode.Tag = "overwrite"
 	// The Dropbox API only accepts timestamps in UTC with second precision.
 	commitInfo.ClientModified = src.ModTime(ctx).UTC().Round(time.Second)
+	// Don't attempt to create filenames that are too long
+	if cErr := checkPathLength(commitInfo.Path); cErr != nil {
+		return cErr
+	}
 
 	size := src.Size()
 	var err error
diff --git a/backend/dropbox/dropbox_internal_test.go b/backend/dropbox/dropbox_internal_test.go
new file mode 100644
index 000000000..75bbf4798
--- /dev/null
+++ b/backend/dropbox/dropbox_internal_test.go
@@ -0,0 +1,44 @@
+package dropbox
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestInternalCheckPathLength(t *testing.T) {
+	rep := func(n int, r rune) (out string) {
+		rs := make([]rune, n)
+		for i := range rs {
+			rs[i] = r
+		}
+		return string(rs)
+	}
+	for _, test := range []struct {
+		in string
+		ok bool
+	}{
+		{in: "", ok: true},
+		{in: rep(maxFileNameLength, 'a'), ok: true},
+		{in: rep(maxFileNameLength+1, 'a'), ok: false},
+		{in: rep(maxFileNameLength, '£'), ok: true},
+		{in: rep(maxFileNameLength+1, '£'), ok: false},
+		{in: rep(maxFileNameLength, '☺'), ok: true},
+		{in: rep(maxFileNameLength+1, '☺'), ok: false},
+		{in: rep(maxFileNameLength, '你'), ok: true},
+		{in: rep(maxFileNameLength+1, '你'), ok: false},
+		{in: "/ok/ok", ok: true},
+		{in: "/ok/" + rep(maxFileNameLength, 'a') + "/ok", ok: true},
+		{in: "/ok/" + rep(maxFileNameLength+1, 'a') + "/ok", ok: false},
+		{in: "/ok/" + rep(maxFileNameLength, '£') + "/ok", ok: true},
+		{in: "/ok/" + rep(maxFileNameLength+1, '£') + "/ok", ok: false},
+		{in: "/ok/" + rep(maxFileNameLength, '☺') + "/ok", ok: true},
+		{in: "/ok/" + rep(maxFileNameLength+1, '☺') + "/ok", ok: false},
+		{in: "/ok/" + rep(maxFileNameLength, '你') + "/ok", ok: true},
+		{in: "/ok/" + rep(maxFileNameLength+1, '你') + "/ok", ok: false},
+	} {
+
+		err := checkPathLength(test.in)
+		assert.Equal(t, test.ok, err == nil, test.in)
+	}
+}
diff --git a/fs/fs.go b/fs/fs.go
index c267b20b9..5ec968936 100644
--- a/fs/fs.go
+++ b/fs/fs.go
@@ -71,6 +71,7 @@ var (
 	ErrorCantShareDirectories        = errors.New("this backend can't share directories with link")
 	ErrorNotImplemented              = errors.New("optional feature not implemented")
 	ErrorCommandNotFound             = errors.New("command not found")
+	ErrorFileNameTooLong             = errors.New("file name too long")
 )
 
 // RegInfo provides information about a filesystem