fix: reject start-after in ListObjects V1
listobjects v1 now returns InvalidArgument when the start-after query parameter is present, matching s3, which only accepts it with list-type=2. previously the parameter was silently ignored. Fixes #2004
This commit is contained in:
1 parent
09e1b65724
commit
b3bf48bdcb
6 files changed
+67
-3
No files matched your search
@@ -21,7 +21,9 @@ import (
|
|||||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||||
"github.com/gofiber/fiber/v3"
|
"github.com/gofiber/fiber/v3"
|
||||||
"github.com/versity/versitygw/auth"
|
"github.com/versity/versitygw/auth"
|
||||||
|
"github.com/versity/versitygw/debuglogger"
|
||||||
"github.com/versity/versitygw/s3api/utils"
|
"github.com/versity/versitygw/s3api/utils"
|
||||||
|
"github.com/versity/versitygw/s3err"
|
||||||
"github.com/versity/versitygw/s3response"
|
"github.com/versity/versitygw/s3response"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -631,6 +633,17 @@ func (c S3ApiController) ListObjects(ctx fiber.Ctx) (*Response, error) {
|
|||||||
}, err
|
}, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// start-after belongs to list-type=2 only; S3 rejects it here instead of
|
||||||
|
// ignoring it
|
||||||
|
if ctx.Request().URI().QueryArgs().Has("start-after") {
|
||||||
|
debuglogger.Logf("start-after is not supported in ListObjects V1")
|
||||||
|
return &Response{
|
||||||
|
MetaOpts: &MetaOptions{
|
||||||
|
BucketOwner: parsedAcl.Owner,
|
||||||
|
},
|
||||||
|
}, s3err.GetInvalidArgumentErr(s3err.InvalidArgStartAfter, "")
|
||||||
|
}
|
||||||
|
|
||||||
maxkeys, err := utils.ParseMaxLimiter(maxkeysStr, utils.LimiterTypeMaxKeys)
|
maxkeys, err := utils.ParseMaxLimiter(maxkeysStr, utils.LimiterTypeMaxKeys)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return &Response{
|
return &Response{
|
||||||
|
|||||||
@@ -1212,6 +1212,24 @@ func TestS3ApiController_ListObjects(t *testing.T) {
|
|||||||
err: s3err.GetInvalidArgMaxLimiter(string(utils.LimiterTypeMaxKeys), "bla"),
|
err: s3err.GetInvalidArgMaxLimiter(string(utils.LimiterTypeMaxKeys), "bla"),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "start-after is not supported",
|
||||||
|
input: testInput{
|
||||||
|
locals: defaultLocals,
|
||||||
|
beRes: listResult,
|
||||||
|
queries: map[string]string{
|
||||||
|
"start-after": "my-key",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
output: testOutput{
|
||||||
|
response: &Response{
|
||||||
|
MetaOpts: &MetaOptions{
|
||||||
|
BucketOwner: "root",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgStartAfter, ""),
|
||||||
|
},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "backend returns error",
|
name: "backend returns error",
|
||||||
input: testInput{
|
input: testInput{
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ type InvalidArgErrorCode int
|
|||||||
const (
|
const (
|
||||||
InvalidArgMaxBuckets InvalidArgErrorCode = iota
|
InvalidArgMaxBuckets InvalidArgErrorCode = iota
|
||||||
InvalidArgNegativeMaxKeys
|
InvalidArgNegativeMaxKeys
|
||||||
|
InvalidArgStartAfter
|
||||||
InvalidArgObjectAttributes
|
InvalidArgObjectAttributes
|
||||||
InvalidArgPartNumber
|
InvalidArgPartNumber
|
||||||
InvalidArgCompleteMpPartNumber
|
InvalidArgCompleteMpPartNumber
|
||||||
@@ -90,6 +91,10 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
|
|||||||
Description: "max-keys cannot be negative",
|
Description: "max-keys cannot be negative",
|
||||||
ArgumentName: "maxKeys",
|
ArgumentName: "maxKeys",
|
||||||
},
|
},
|
||||||
|
InvalidArgStartAfter: {
|
||||||
|
Description: "startAfter only supported in REST.GET.BUCKET with list-type=2",
|
||||||
|
ArgumentName: "start-after",
|
||||||
|
},
|
||||||
InvalidArgObjectAttributes: {
|
InvalidArgObjectAttributes: {
|
||||||
Description: "Invalid attribute name specified.",
|
Description: "Invalid attribute name specified.",
|
||||||
ArgumentName: "x-amz-object-attributes",
|
ArgumentName: "x-amz-object-attributes",
|
||||||
|
|||||||
@@ -17,6 +17,8 @@ package integration
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"time"
|
||||||
|
|
||||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||||
@@ -338,6 +340,33 @@ func ListObjects_marker_not_from_obj_list(s *S3Conf) error {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ListObjects_start_after_not_supported sends start-after without
|
||||||
|
// list-type=2. S3 rejects it with InvalidArgument instead of ignoring it.
|
||||||
|
func ListObjects_start_after_not_supported(s *S3Conf) error {
|
||||||
|
testName := "ListObjects_start_after_not_supported"
|
||||||
|
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||||
|
_, err := putObjects(s3client, []string{"bar", "foo"}, bucket)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// ListObjectsInput has no StartAfter field, so send the query directly
|
||||||
|
req, err := createSignedReq(http.MethodGet, s.endpoint,
|
||||||
|
fmt.Sprintf("%v?start-after=bar", bucket),
|
||||||
|
s.awsID, s.awsSecret, "s3", s.awsRegion, "", nil, time.Now(), nil)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
resp, err := s.httpClient.Do(req)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
return checkHTTPResponseApiErr(resp, s3err.GetInvalidArgumentErr(s3err.InvalidArgStartAfter, ""))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func ListObjects_with_checksum(s *S3Conf) error {
|
func ListObjects_with_checksum(s *S3Conf) error {
|
||||||
testName := "ListObjects_with_checksum"
|
testName := "ListObjects_with_checksum"
|
||||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||||
|
|||||||
@@ -315,6 +315,7 @@ func TestListObjects(ts *TestState) {
|
|||||||
ts.Run(ListObjects_delimiter)
|
ts.Run(ListObjects_delimiter)
|
||||||
ts.Run(ListObjects_max_keys_none)
|
ts.Run(ListObjects_max_keys_none)
|
||||||
ts.Run(ListObjects_marker_not_from_obj_list)
|
ts.Run(ListObjects_marker_not_from_obj_list)
|
||||||
|
ts.Run(ListObjects_start_after_not_supported)
|
||||||
ts.Run(ListObjects_list_all_objs)
|
ts.Run(ListObjects_list_all_objs)
|
||||||
ts.Run(ListObjects_nested_dir_file_objs)
|
ts.Run(ListObjects_nested_dir_file_objs)
|
||||||
ts.Run(ListObjects_check_owner)
|
ts.Run(ListObjects_check_owner)
|
||||||
@@ -3117,6 +3118,7 @@ func GetIntTests() IntTests {
|
|||||||
"ListObjects_delimiter": ListObjects_delimiter,
|
"ListObjects_delimiter": ListObjects_delimiter,
|
||||||
"ListObjects_max_keys_none": ListObjects_max_keys_none,
|
"ListObjects_max_keys_none": ListObjects_max_keys_none,
|
||||||
"ListObjects_marker_not_from_obj_list": ListObjects_marker_not_from_obj_list,
|
"ListObjects_marker_not_from_obj_list": ListObjects_marker_not_from_obj_list,
|
||||||
|
"ListObjects_start_after_not_supported": ListObjects_start_after_not_supported,
|
||||||
"ListObjects_list_all_objs": ListObjects_list_all_objs,
|
"ListObjects_list_all_objs": ListObjects_list_all_objs,
|
||||||
"ListObjects_nested_dir_file_objs": ListObjects_nested_dir_file_objs,
|
"ListObjects_nested_dir_file_objs": ListObjects_nested_dir_file_objs,
|
||||||
"ListObjects_check_owner": ListObjects_check_owner,
|
"ListObjects_check_owner": ListObjects_check_owner,
|
||||||
|
|||||||
@@ -260,9 +260,6 @@ source ./tests/drivers/objects_and_versions.sh
|
|||||||
|
|
||||||
# tags: curl,ListObjects,start-after,invalid-query
|
# tags: curl,ListObjects,start-after,invalid-query
|
||||||
@test "ListObjectsV1 - start-after - error" {
|
@test "ListObjectsV1 - start-after - error" {
|
||||||
if [ "$DIRECT" != "true" ]; then
|
|
||||||
skip "https://github.com/versity/versitygw/issues/2004"
|
|
||||||
fi
|
|
||||||
run setup_bucket_and_files_v3 "$BUCKET_ONE_NAME" 2
|
run setup_bucket_and_files_v3 "$BUCKET_ONE_NAME" 2
|
||||||
assert_success
|
assert_success
|
||||||
read -r bucket_name file_one file_two <<< "$output"
|
read -r bucket_name file_one file_two <<< "$output"
|
||||||
|
|||||||
Reference in new issue
Block a user