From b300f77c2de6ed35e755dcd746a704c2f7e46e6c Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@tomclegg.ca>
Date: Tue, 28 Jul 2020 16:14:29 -0400
Subject: [PATCH] 16535: Use 1000 as default and maximum value for max-keys.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>
---
 services/keep-web/s3.go      |  6 ++++--
 services/keep-web/s3_test.go | 24 ++++++++++++++++++------
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 8bda2eac50..536a77eed2 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -21,6 +21,8 @@ import (
 	"github.com/AdRoll/goamz/s3"
 )
 
+const s3MaxKeys = 1000
+
 // serveS3 handles r and returns true if r is a request from an S3
 // client, otherwise it returns false.
 func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
@@ -193,10 +195,10 @@ func (h *handler) s3list(w http.ResponseWriter, r *http.Request, fs arvados.Cust
 	params.bucket = strings.SplitN(r.URL.Path[1:], "/", 2)[0]
 	params.delimiter = r.FormValue("delimiter")
 	params.marker = r.FormValue("marker")
-	if mk, _ := strconv.ParseInt(r.FormValue("max-keys"), 10, 64); mk > 0 {
+	if mk, _ := strconv.ParseInt(r.FormValue("max-keys"), 10, 64); mk > 0 && mk < s3MaxKeys {
 		params.maxKeys = int(mk)
 	} else {
-		params.maxKeys = 100
+		params.maxKeys = s3MaxKeys
 	}
 	params.prefix = r.FormValue("prefix")
 
diff --git a/services/keep-web/s3_test.go b/services/keep-web/s3_test.go
index d88e3b5edd..349be7777f 100644
--- a/services/keep-web/s3_test.go
+++ b/services/keep-web/s3_test.go
@@ -270,6 +270,10 @@ func (s *IntegrationSuite) TestS3CollectionList(c *check.C) {
 	s.testS3List(c, stage.collbucket, "dir0/", 71, filesPerDir)
 }
 func (s *IntegrationSuite) testS3List(c *check.C, bucket *s3.Bucket, prefix string, pageSize, expectFiles int) {
+	expectPageSize := pageSize
+	if expectPageSize > 1000 {
+		expectPageSize = 1000
+	}
 	gotKeys := map[string]s3.Key{}
 	nextMarker := ""
 	pages := 0
@@ -278,8 +282,8 @@ func (s *IntegrationSuite) testS3List(c *check.C, bucket *s3.Bucket, prefix stri
 		if !c.Check(err, check.IsNil) {
 			break
 		}
-		c.Check(len(resp.Contents) <= pageSize, check.Equals, true)
-		if pages++; !c.Check(pages <= (expectFiles/pageSize)+1, check.Equals, true) {
+		c.Check(len(resp.Contents) <= expectPageSize, check.Equals, true)
+		if pages++; !c.Check(pages <= (expectFiles/expectPageSize)+1, check.Equals, true) {
 			break
 		}
 		for _, key := range resp.Contents {
@@ -306,11 +310,19 @@ func (s *IntegrationSuite) TestS3CollectionListRollup(c *check.C) {
 	stage.writeBigDirs(c, dirs, filesPerDir)
 	err := stage.collbucket.PutReader("dingbats", &bytes.Buffer{}, 0, "application/octet-stream", s3.Private, s3.Options{})
 	c.Assert(err, check.IsNil)
-	resp, err := stage.collbucket.List("", "", "", 20000)
-	c.Check(err, check.IsNil)
 	var allfiles []string
-	for _, key := range resp.Contents {
-		allfiles = append(allfiles, key.Key)
+	for marker := ""; ; {
+		resp, err := stage.collbucket.List("", "", marker, 20000)
+		c.Check(err, check.IsNil)
+		for _, key := range resp.Contents {
+			if len(allfiles) == 0 || allfiles[len(allfiles)-1] != key.Key {
+				allfiles = append(allfiles, key.Key)
+			}
+		}
+		marker = resp.NextMarker
+		if marker == "" {
+			break
+		}
 	}
 	c.Check(allfiles, check.HasLen, dirs*filesPerDir+3)
 
-- 
2.30.2