16790: Avoid returning empty NextMarker tag in list responses.
authorTom Clegg <tom@tomclegg.ca>
Tue, 1 Sep 2020 14:16:47 +0000 (10:16 -0400)
committerTom Clegg <tom@tomclegg.ca>
Tue, 1 Sep 2020 14:16:47 +0000 (10:16 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

services/keep-web/s3.go
services/keep-web/s3_test.go

index 4e8028ae6e8de3c429679c7ff25fc87ee58f3cf2..52cfede46642120dce520868d2d38758fb97cc12 100644 (file)
@@ -358,6 +358,11 @@ func (h *handler) s3list(w http.ResponseWriter, r *http.Request, fs arvados.Cust
                // CommonPrefixes is nil, which confuses some clients.
                // Fix by using this nested struct instead.
                CommonPrefixes []commonPrefix
+               // Similarly, we need omitempty here, because an empty
+               // tag confuses some clients (e.g.,
+               // github.com/aws/aws-sdk-net never terminates its
+               // paging loop).
+               NextMarker string `xml:"NextMarker,omitempty"`
        }
        resp := listResp{
                ListResp: s3.ListResp{
index c6d53238e81645928acd4b76aec520bea6674c4c..1ff19a98bcbb66d8dd3c65fbf5dd68a70ab5a6ac 100644 (file)
@@ -411,6 +411,26 @@ func (s *IntegrationSuite) TestS3ListNoCommonPrefixes(c *check.C) {
        c.Check(string(buf), check.Not(check.Matches), `(?ms).*CommonPrefixes.*`)
 }
 
+// If there is no delimiter in the request, or the results are not
+// truncated, the NextMarker XML tag should not appear in the response
+// body.
+func (s *IntegrationSuite) TestS3ListNoNextMarker(c *check.C) {
+       stage := s.s3setup(c)
+       defer stage.teardown(c)
+
+       for _, query := range []string{"prefix=e&delimiter=/", ""} {
+               req, err := http.NewRequest("GET", stage.collbucket.URL("/"), nil)
+               c.Assert(err, check.IsNil)
+               req.Header.Set("Authorization", "AWS "+arvadostest.ActiveTokenV2+":none")
+               req.URL.RawQuery = query
+               resp, err := http.DefaultClient.Do(req)
+               c.Assert(err, check.IsNil)
+               buf, err := ioutil.ReadAll(resp.Body)
+               c.Assert(err, check.IsNil)
+               c.Check(string(buf), check.Not(check.Matches), `(?ms).*NextMarker.*`)
+       }
+}
+
 func (s *IntegrationSuite) TestS3CollectionList(c *check.C) {
        stage := s.s3setup(c)
        defer stage.teardown(c)