From 542677d8074fcac91f1caffdaf9d5576da4012b6 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Tue, 21 Mar 2023 12:44:45 +0000
Subject: [PATCH] s3: fix --s3-versions on individual objects

Before this fix attempting to access an s3 versioned object by name in
a subdirectory of root would not find the object.

This fixes the problem and introduced an integraton test.

See: https://forum.rclone.org/t/s3-versions-cant-retrieve-old-version/36900
---
 backend/s3/s3.go               |  7 ++++---
 backend/s3/s3_internal_test.go | 21 +++++++++++++++++++--
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/backend/s3/s3.go b/backend/s3/s3.go
index 4d4994302..f8dc5cd37 100644
--- a/backend/s3/s3.go
+++ b/backend/s3/s3.go
@@ -3119,6 +3119,7 @@ func (f *Fs) getMetaDataListing(ctx context.Context, wantRemote string) (info *s
 	err = f.list(ctx, listOpt{
 		bucket:       bucket,
 		directory:    bucketPath,
+		prefix:       f.rootDirectory,
 		recurse:      true,
 		withVersions: f.opt.Versions,
 		findFile:     true,
@@ -3524,10 +3525,10 @@ type listOpt struct {
 // list lists the objects into the function supplied with the opt
 // supplied.
 func (f *Fs) list(ctx context.Context, opt listOpt, fn listFn) error {
+	if opt.prefix != "" {
+		opt.prefix += "/"
+	}
 	if !opt.findFile {
-		if opt.prefix != "" {
-			opt.prefix += "/"
-		}
 		if opt.directory != "" {
 			opt.directory += "/"
 		}
diff --git a/backend/s3/s3_internal_test.go b/backend/s3/s3_internal_test.go
index 9d22ca69b..ac67ccc81 100644
--- a/backend/s3/s3_internal_test.go
+++ b/backend/s3/s3_internal_test.go
@@ -6,12 +6,15 @@ import (
 	"context"
 	"crypto/md5"
 	"fmt"
+	"path"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/cache"
 	"github.com/rclone/rclone/fs/hash"
 	"github.com/rclone/rclone/fstest"
 	"github.com/rclone/rclone/fstest/fstests"
@@ -250,7 +253,8 @@ func (f *Fs) InternalTestVersions(t *testing.T) {
 	time.Sleep(2 * time.Second)
 
 	// Create an object
-	const fileName = "test-versions.txt"
+	const dirName = "versions"
+	const fileName = dirName + "/" + "test-versions.txt"
 	contents := random.String(100)
 	item := fstest.NewItem(fileName, contents, fstest.Time("2001-05-06T04:05:06.499999999Z"))
 	obj := fstests.PutTestContents(ctx, t, f, &item, contents, true)
@@ -280,11 +284,12 @@ func (f *Fs) InternalTestVersions(t *testing.T) {
 		}()
 
 		// Read the contents
-		entries, err := f.List(ctx, "")
+		entries, err := f.List(ctx, dirName)
 		require.NoError(t, err)
 		tests := 0
 		var fileNameVersion string
 		for _, entry := range entries {
+			t.Log(entry)
 			remote := entry.Remote()
 			if remote == fileName {
 				t.Run("ReadCurrent", func(t *testing.T) {
@@ -309,6 +314,18 @@ func (f *Fs) InternalTestVersions(t *testing.T) {
 			require.NotNil(t, o)
 			assert.Equal(t, int64(100), o.Size(), o.Remote())
 		})
+
+		// Check we can make a NewFs from that object with a version suffix
+		t.Run("NewFs", func(t *testing.T) {
+			newPath := path.Join(fs.ConfigString(f), fileNameVersion)
+			// Make sure --s3-versions is set in the config of the new remote
+			confPath := strings.Replace(newPath, ":", ",versions:", 1)
+			fNew, err := cache.Get(ctx, confPath)
+			// This should return pointing to a file
+			assert.Equal(t, fs.ErrorIsFile, err)
+			// With the directory the directory above
+			assert.Equal(t, dirName, path.Base(fs.ConfigString(fNew)))
+		})
 	})
 
 	t.Run("VersionAt", func(t *testing.T) {