From 63c4fef27aa1c9ae1e36e2642499f9199c11f720 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 12 Mar 2025 17:29:53 +0000 Subject: [PATCH] fs: fix corruption of SizeSuffix with "B" suffix in config (eg --min-size) Before this change, the config system round tripped fs.SizeSuffix values through strings like this, corrupting them in the process. "2B" -> 2 -> "2" -> 2048 This caused `--min-size 2B` to be interpreted as `--min-size 2k`. This fix makes sure SizeSuffix values have a "B" suffix when turned into a string where necessary, so it becomes "2B" -> 2 -> "2B" -> 2 In rclone v2 we should probably declare unsuffixed SizeSuffix values are in bytes not kBytes (done for rsync compatibility) but this would be a backwards incompatible change which we don't want for v1. Fixes #8437 Fixes #8212 Fixes #5169 --- fs/fs_test.go | 28 ++++++++++++++++++++++++++++ fs/registry.go | 18 +++++++++++++++--- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/fs/fs_test.go b/fs/fs_test.go index e0099e96a..45b16229c 100644 --- a/fs/fs_test.go +++ b/fs/fs_test.go @@ -261,6 +261,34 @@ func TestOptionString(t *testing.T) { assert.Equal(t, "true", caseInsensitiveOption.String()) } +func TestOptionStringStringArray(t *testing.T) { + opt := Option{ + Name: "string_array", + Default: []string(nil), + } + assert.Equal(t, "", opt.String()) + opt.Default = []string{} + assert.Equal(t, "", opt.String()) + opt.Default = []string{"a", "b"} + assert.Equal(t, "a,b", opt.String()) + opt.Default = []string{"hello, world!", "goodbye, world!"} + assert.Equal(t, `"hello, world!","goodbye, world!"`, opt.String()) +} + +func TestOptionStringSizeSuffix(t *testing.T) { + opt := Option{ + Name: "size_suffix", + Default: SizeSuffix(0), + } + assert.Equal(t, "0", opt.String()) + opt.Default = SizeSuffix(-1) + assert.Equal(t, "off", opt.String()) + opt.Default = SizeSuffix(100) + assert.Equal(t, "100B", opt.String()) + opt.Default = SizeSuffix(1024) + assert.Equal(t, "1Ki", opt.String()) +} + func TestOptionSet(t *testing.T) { o := caseInsensitiveOption assert.Equal(t, true, o.Value) diff --git a/fs/registry.go b/fs/registry.go index 628a18afe..2df893394 100644 --- a/fs/registry.go +++ b/fs/registry.go @@ -259,15 +259,27 @@ func (o *Option) IsDefault() bool { // String turns Option into a string func (o *Option) String() string { v := o.GetValue() - if stringArray, isStringArray := v.([]string); isStringArray { + switch x := v.(type) { + case []string: // Treat empty string array as empty string // This is to make the default value of the option help nice - if len(stringArray) == 0 { + if len(x) == 0 { return "" } // Encode string arrays as CSV // The default Go encoding can't be decoded uniquely - return CommaSepList(stringArray).String() + return CommaSepList(x).String() + case SizeSuffix: + str := x.String() + // Suffix bare numbers with "B" unless they are 0 + // + // This makes sure that fs.SizeSuffix roundtrips through string + if len(str) > 0 && str != "0" { + if lastDigit := str[len(str)-1]; lastDigit >= '0' && lastDigit <= '9' { + str += "B" + } + } + return str } return fmt.Sprint(v) }