From 3567a47258c7c29aac3870add592975564122f22 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Fri, 28 Apr 2023 11:58:49 +0100
Subject: [PATCH] fs: make ConfigString properly reverse suffixed file systems

Before this change we renamed file systems with overridden config with
{suffix}.

However this meant that ConfigString produced a value which wouldn't
re-create the file system.

This uses an internal hash to keep note of what config goes which
which {suffix} in order to remake the config properly.
---
 fs/newfs.go      | 35 ++++++++++++++++++++++++++++++-----
 fs/newfs_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 5 deletions(-)
 create mode 100644 fs/newfs_test.go

diff --git a/fs/newfs.go b/fs/newfs.go
index cc921bb65..52375e86e 100644
--- a/fs/newfs.go
+++ b/fs/newfs.go
@@ -9,11 +9,18 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 
 	"github.com/rclone/rclone/fs/config/configmap"
 	"github.com/rclone/rclone/fs/fspath"
 )
 
+// Store the hashes of the overridden config
+var (
+	overriddenConfigMu sync.Mutex
+	overriddenConfig   = make(map[string]string)
+)
+
 // NewFs makes a new Fs object from the path
 //
 // The path is of the form remote:path
@@ -37,18 +44,25 @@ func NewFs(ctx context.Context, path string) (Fs, error) {
 		extraConfig := overridden.String()
 		//Debugf(nil, "detected overridden config %q", extraConfig)
 		md5sumBinary := md5.Sum([]byte(extraConfig))
-		suffix := base64.RawURLEncoding.EncodeToString(md5sumBinary[:])
+		configHash := base64.RawURLEncoding.EncodeToString(md5sumBinary[:])
 		// 5 characters length is 5*6 = 30 bits of base64
-		const maxLength = 5
-		if len(suffix) > maxLength {
-			suffix = suffix[:maxLength]
+		overriddenConfigMu.Lock()
+		var suffix string
+		for maxLength := 5; ; maxLength++ {
+			suffix = "{" + configHash[:maxLength] + "}"
+			existingExtraConfig, ok := overriddenConfig[suffix]
+			if !ok || existingExtraConfig == extraConfig {
+				break
+			}
 		}
-		suffix = "{" + suffix + "}"
 		Debugf(configName, "detected overridden config - adding %q suffix to name", suffix)
 		// Add the suffix to the config name
 		//
 		// These need to work as filesystem names as the VFS cache will use them
 		configName += suffix
+		// Store the config suffixes for reversing in ConfigString
+		overriddenConfig[suffix] = extraConfig
+		overriddenConfigMu.Unlock()
 	}
 	f, err := fsInfo.NewFs(ctx, configName, fsPath, config)
 	if f != nil && (err == nil || err == ErrorIsFile) {
@@ -105,6 +119,17 @@ func ParseRemote(path string) (fsInfo *RegInfo, configName, fsPath string, conne
 // to configure the Fs as passed to fs.NewFs
 func ConfigString(f Fs) string {
 	name := f.Name()
+	if open := strings.IndexRune(name, '{'); open >= 0 && strings.HasSuffix(name, "}") {
+		suffix := name[open:]
+		overriddenConfigMu.Lock()
+		config, ok := overriddenConfig[suffix]
+		overriddenConfigMu.Unlock()
+		if ok {
+			name = name[:open] + "," + config
+		} else {
+			Errorf(f, "Failed to find config for suffix %q", suffix)
+		}
+	}
 	root := f.Root()
 	if name == "local" && f.Features().IsLocal {
 		return root
diff --git a/fs/newfs_test.go b/fs/newfs_test.go
new file mode 100644
index 000000000..3f86cd2d5
--- /dev/null
+++ b/fs/newfs_test.go
@@ -0,0 +1,43 @@
+package fs_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fstest/mockfs"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewFs(t *testing.T) {
+	ctx := context.Background()
+
+	// Register mockfs temporarily
+	oldRegistry := fs.Registry
+	mockfs.Register()
+	defer func() {
+		fs.Registry = oldRegistry
+	}()
+
+	f1, err := fs.NewFs(ctx, ":mockfs:/tmp")
+	require.NoError(t, err)
+	assert.Equal(t, ":mockfs", f1.Name())
+	assert.Equal(t, "/tmp", f1.Root())
+
+	assert.Equal(t, ":mockfs:/tmp", fs.ConfigString(f1))
+
+	f2, err := fs.NewFs(ctx, ":mockfs,potato:/tmp")
+	require.NoError(t, err)
+	assert.Equal(t, ":mockfs{S_NHG}", f2.Name())
+	assert.Equal(t, "/tmp", f2.Root())
+
+	assert.Equal(t, ":mockfs,potato='true':/tmp", fs.ConfigString(f2))
+
+	f3, err := fs.NewFs(ctx, ":mockfs,potato='true':/tmp")
+	require.NoError(t, err)
+	assert.Equal(t, ":mockfs{S_NHG}", f3.Name())
+	assert.Equal(t, "/tmp", f3.Root())
+
+	assert.Equal(t, ":mockfs,potato='true':/tmp", fs.ConfigString(f3))
+}