From 5f9555afadd18987336ea7a4058b29d1fb879d70 Mon Sep 17 00:00:00 2001
From: Denis Kirillov <d.kirillov@yadro.com>
Date: Thu, 12 Oct 2023 11:10:43 +0300
Subject: [PATCH] [#237] Fix list object versions marker param

According to https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectVersions.html
we have to use `key-marker`

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
---
 CHANGELOG.md                    |  1 +
 api/handler/delete_test.go      |  9 ++-----
 api/handler/object_list.go      |  2 +-
 api/handler/object_list_test.go | 48 +++++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c575d92..daef632 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@ This document outlines major changes between releases.
 - Fix parsing signed headers in presigned urls (#182)
 - Fix url escaping (#188)
 - Use correct keys in `list-multipart-uploads` response (#185)
+- Fix parsing `key-marker` for object list versions (#237)
 
 ### Added
 - Add `trace_id` value into log record when tracing is enabled (#142)
diff --git a/api/handler/delete_test.go b/api/handler/delete_test.go
index e2f49f0..bae3005 100644
--- a/api/handler/delete_test.go
+++ b/api/handler/delete_test.go
@@ -456,13 +456,8 @@ func headObjectBase(hc *handlerContext, bktName, objName, version string) *httpt
 	return w
 }
 
-func listVersions(t *testing.T, tc *handlerContext, bktName string) *ListObjectsVersionsResponse {
-	w, r := prepareTestRequest(tc, bktName, "", nil)
-	tc.Handler().ListBucketObjectVersionsHandler(w, r)
-	assertStatus(t, w, http.StatusOK)
-	res := &ListObjectsVersionsResponse{}
-	parseTestResponse(t, w, res)
-	return res
+func listVersions(_ *testing.T, tc *handlerContext, bktName string) *ListObjectsVersionsResponse {
+	return listObjectsVersions(tc, bktName, "", "", "", "", -1)
 }
 
 func getVersion(resp *ListObjectsVersionsResponse, objName string) []*ObjectVersionResponse {
diff --git a/api/handler/object_list.go b/api/handler/object_list.go
index 4f3cad1..aa3db7a 100644
--- a/api/handler/object_list.go
+++ b/api/handler/object_list.go
@@ -253,7 +253,7 @@ func parseListObjectVersionsRequest(reqInfo *middleware.ReqInfo) (*layer.ListObj
 	}
 
 	res.Prefix = queryValues.Get("prefix")
-	res.KeyMarker = queryValues.Get("marker")
+	res.KeyMarker = queryValues.Get("key-marker")
 	res.Delimiter = queryValues.Get("delimiter")
 	res.Encode = queryValues.Get("encoding-type")
 	res.VersionIDMarker = queryValues.Get("version-id-marker")
diff --git a/api/handler/object_list_test.go b/api/handler/object_list_test.go
index 3db3baf..5b6b859 100644
--- a/api/handler/object_list_test.go
+++ b/api/handler/object_list_test.go
@@ -3,6 +3,7 @@ package handler
 import (
 	"net/http"
 	"net/url"
+	"sort"
 	"strconv"
 	"testing"
 
@@ -56,6 +57,36 @@ func TestListObjectNullVersions(t *testing.T) {
 	require.Equal(t, data.UnversionedObjectVersionID, result.Version[1].VersionID)
 }
 
+func TestListObjectsPaging(t *testing.T) {
+	hc := prepareHandlerContext(t)
+
+	bktName := "bucket-versioning-enabled"
+	createTestBucket(hc, bktName)
+
+	n := 10
+
+	var objects []string
+	for i := 0; i < n; i++ {
+		objects = append(objects, "objects"+strconv.Itoa(i))
+		putObjectContent(hc, bktName, objects[i], "content")
+	}
+	sort.Strings(objects)
+
+	result := &ListObjectsVersionsResponse{IsTruncated: true}
+	for result.IsTruncated {
+		result = listObjectsVersions(hc, bktName, "", "", result.NextKeyMarker, result.NextVersionIDMarker, n/3)
+
+		for i, version := range result.Version {
+			if objects[i] != version.Key {
+				t.Errorf("expected: '%s', got: '%s'", objects[i], version.Key)
+			}
+		}
+		objects = objects[len(result.Version):]
+	}
+
+	require.Empty(t, objects)
+}
+
 func TestS3CompatibilityBucketListV2BothContinuationTokenStartAfter(t *testing.T) {
 	tc := prepareHandlerContext(t)
 
@@ -215,3 +246,20 @@ func listObjectsV1(hc *handlerContext, bktName, prefix, delimiter, marker string
 	parseTestResponse(hc.t, w, res)
 	return res
 }
+
+func listObjectsVersions(hc *handlerContext, bktName, prefix, delimiter, keyMarker, versionIDMarker string, maxKeys int) *ListObjectsVersionsResponse {
+	query := prepareCommonListObjectsQuery(prefix, delimiter, maxKeys)
+	if len(keyMarker) != 0 {
+		query.Add("key-marker", keyMarker)
+	}
+	if len(versionIDMarker) != 0 {
+		query.Add("version-id-marker", versionIDMarker)
+	}
+
+	w, r := prepareTestFullRequest(hc, bktName, "", query, nil)
+	hc.Handler().ListBucketObjectVersionsHandler(w, r)
+	assertStatus(hc.t, w, http.StatusOK)
+	res := &ListObjectsVersionsResponse{}
+	parseTestResponse(hc.t, w, res)
+	return res
+}
-- 
2.45.2