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
This commit is contained in:
Nick Craig-Wood 2020-11-27 11:49:37 +00:00
parent a9585efd64
commit 584523672c
3 changed files with 81 additions and 0 deletions

View File

@ -30,6 +30,7 @@ import (
"regexp" "regexp"
"strings" "strings"
"time" "time"
"unicode/utf8"
"github.com/dropbox/dropbox-sdk-go-unofficial/dropbox" "github.com/dropbox/dropbox-sdk-go-unofficial/dropbox"
"github.com/dropbox/dropbox-sdk-go-unofficial/dropbox/auth" "github.com/dropbox/dropbox-sdk-go-unofficial/dropbox/auth"
@ -86,6 +87,8 @@ const (
// by default. // by default.
defaultChunkSize = 48 * fs.MebiByte defaultChunkSize = 48 * fs.MebiByte
maxChunkSize = 150 * 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 ( var (
@ -839,6 +842,10 @@ func (f *Fs) Mkdir(ctx context.Context, dir string) error {
arg2 := files.CreateFolderArg{ arg2 := files.CreateFolderArg{
Path: f.opt.Enc.FromStandardPath(root), 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.pacer.Call(func() (bool, error) {
_, err = f.srv.CreateFolderV2(&arg2) _, err = f.srv.CreateFolderV2(&arg2)
return shouldRetry(err) return shouldRetry(err)
@ -1417,6 +1424,31 @@ func (o *Object) uploadChunked(in0 io.Reader, commitInfo *files.CommitInfo, size
return entry, nil 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 // Update the already existing object
// //
// Copy the reader into the object updating modTime and size // 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" commitInfo.Mode.Tag = "overwrite"
// The Dropbox API only accepts timestamps in UTC with second precision. // The Dropbox API only accepts timestamps in UTC with second precision.
commitInfo.ClientModified = src.ModTime(ctx).UTC().Round(time.Second) 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() size := src.Size()
var err error var err error

View File

@ -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)
}
}

View File

@ -71,6 +71,7 @@ var (
ErrorCantShareDirectories = errors.New("this backend can't share directories with link") ErrorCantShareDirectories = errors.New("this backend can't share directories with link")
ErrorNotImplemented = errors.New("optional feature not implemented") ErrorNotImplemented = errors.New("optional feature not implemented")
ErrorCommandNotFound = errors.New("command not found") ErrorCommandNotFound = errors.New("command not found")
ErrorFileNameTooLong = errors.New("file name too long")
) )
// RegInfo provides information about a filesystem // RegInfo provides information about a filesystem