Merge pull request #2534 from versity/ben/encoding-type-url

fix: honor encoding-type=url in ListObjects and ListObjectsV2
This commit is contained in:
Ben McClelland
2026-10-16 14:38:45 -07:00
committed by GitHub
10 changed files with 489 additions and 9 deletions
+24
View File
@@ -568,6 +568,14 @@ func (c S3ApiController) ListObjectsV2(ctx fiber.Ctx) (*Response, error) {
},
}, err
}
encodingType, err := utils.ParseEncodingType(ctx)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
res, err := c.be.ListObjectsV2(ctx.RequestCtx(),
&s3.ListObjectsV2Input{
@@ -587,6 +595,10 @@ func (c S3ApiController) ListObjectsV2(ctx fiber.Ctx) (*Response, error) {
}, err
}
if encodingType != "" {
res = utils.URLEncodeListObjectsV2Result(res, encodingType)
}
return &Response{
Headers: map[string]*string{
"x-amz-bucket-region": &region,
@@ -658,6 +670,14 @@ func (c S3ApiController) ListObjects(ctx fiber.Ctx) (*Response, error) {
},
}, err
}
encodingType, err := utils.ParseEncodingType(ctx)
if err != nil {
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, err
}
res, err := c.be.ListObjects(ctx.RequestCtx(),
&s3.ListObjectsInput{
@@ -689,6 +709,10 @@ func (c S3ApiController) ListObjects(ctx fiber.Ctx) (*Response, error) {
res.Marker = &marker
}
if encodingType != "" {
res = utils.URLEncodeListObjectsResult(res, encodingType)
}
return &Response{
Headers: map[string]*string{
"x-amz-bucket-region": &region,
+214
View File
@@ -1060,11 +1060,118 @@ func TestS3ApiController_ListObjectsV2(t *testing.T) {
{Key: utils.GetStringPtr("my-key")},
},
}
specialKeysV2Result := s3response.ListObjectsV2Result{
Name: utils.GetStringPtr("name"),
Prefix: utils.GetStringPtr("dir/a+"),
StartAfter: utils.GetStringPtr("dir/a+ 0"),
ContinuationToken: utils.GetStringPtr("dir/a+ 1"),
NextContinuationToken: utils.GetStringPtr("dir/a+ b.txt"),
Delimiter: utils.GetStringPtr("/"),
Contents: []s3response.Object{
{Key: utils.GetStringPtr("dir/a+ b.txt")},
},
CommonPrefixes: []types.CommonPrefix{
{Prefix: utils.GetStringPtr("dir/a+ c/")},
},
}
urlEncodedV2Result := s3response.ListObjectsV2Result{
Name: utils.GetStringPtr("name"),
Prefix: utils.GetStringPtr("dir/a%2B"),
StartAfter: utils.GetStringPtr("dir/a%2B+0"),
ContinuationToken: utils.GetStringPtr("dir/a+ 1"),
NextContinuationToken: utils.GetStringPtr("dir/a+ b.txt"),
Delimiter: utils.GetStringPtr("/"),
Contents: []s3response.Object{
{Key: utils.GetStringPtr("dir/a%2B+b.txt")},
},
CommonPrefixes: []types.CommonPrefix{
{Prefix: utils.GetStringPtr("dir/a%2B+c/")},
},
EncodingType: types.EncodingTypeUrl,
}
urlEncodedV2ResultMixedCase := urlEncodedV2Result
urlEncodedV2ResultMixedCase.EncodingType = "uRl"
tests := []struct {
name string
input testInput
output testOutput
}{
{
name: "invalid encoding type",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"encoding-type": "gibberish",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEncodingType, "gibberish"),
},
},
{
name: "empty encoding type",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"encoding-type": "",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEncodingType, ""),
},
},
{
name: "url encoding type in any letter case",
input: testInput{
locals: defaultLocals,
beRes: specialKeysV2Result,
queries: map[string]string{
"encoding-type": "uRl",
},
},
output: testOutput{
response: &Response{
Data: urlEncodedV2ResultMixedCase,
Headers: map[string]*string{
"x-amz-bucket-region": utils.GetStringPtr(defaultRegion),
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
},
},
{
name: "url encoding type",
input: testInput{
locals: defaultLocals,
beRes: specialKeysV2Result,
queries: map[string]string{
"encoding-type": "url",
},
},
output: testOutput{
response: &Response{
Data: urlEncodedV2Result,
Headers: map[string]*string{
"x-amz-bucket-region": utils.GetStringPtr(defaultRegion),
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
},
},
{
name: "verify access fails",
input: testInput{
@@ -1179,11 +1286,118 @@ func TestS3ApiController_ListObjects(t *testing.T) {
// S3 returns NextMarker only when the request has a delimiter
listResultNextMarker := listResultEchoed
listResultNextMarker.NextMarker = utils.GetStringPtr("my-key")
specialKeysResult := s3response.ListObjectsResult{
Name: utils.GetStringPtr("name"),
Prefix: utils.GetStringPtr("dir/a+"),
Marker: utils.GetStringPtr("dir/a+ 0"),
NextMarker: utils.GetStringPtr("dir/a+ b.txt"),
Delimiter: utils.GetStringPtr("/"),
Contents: []s3response.Object{
{Key: utils.GetStringPtr("dir/a+ b.txt")},
},
CommonPrefixes: []types.CommonPrefix{
{Prefix: utils.GetStringPtr("dir/a+ c/")},
},
}
urlEncodedResult := s3response.ListObjectsResult{
Name: utils.GetStringPtr("name"),
Prefix: utils.GetStringPtr("dir/a%2B"),
Marker: utils.GetStringPtr("dir/a%2B+0"),
NextMarker: utils.GetStringPtr("dir/a%2B+b.txt"),
Delimiter: utils.GetStringPtr("/"),
Contents: []s3response.Object{
{Key: utils.GetStringPtr("dir/a%2B+b.txt")},
},
CommonPrefixes: []types.CommonPrefix{
{Prefix: utils.GetStringPtr("dir/a%2B+c/")},
},
EncodingType: types.EncodingTypeUrl,
}
urlEncodedResultMixedCase := urlEncodedResult
urlEncodedResultMixedCase.EncodingType = "uRl"
tests := []struct {
name string
input testInput
output testOutput
}{
{
name: "invalid encoding type",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"encoding-type": "gibberish",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEncodingType, "gibberish"),
},
},
{
name: "empty encoding type",
input: testInput{
locals: defaultLocals,
queries: map[string]string{
"encoding-type": "",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEncodingType, ""),
},
},
{
name: "url encoding type in any letter case",
input: testInput{
locals: defaultLocals,
beRes: specialKeysResult,
queries: map[string]string{
"delimiter": "/",
"encoding-type": "uRl",
},
},
output: testOutput{
response: &Response{
Data: urlEncodedResultMixedCase,
Headers: map[string]*string{
"x-amz-bucket-region": utils.GetStringPtr(defaultRegion),
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
},
},
{
name: "url encoding type",
input: testInput{
locals: defaultLocals,
beRes: specialKeysResult,
queries: map[string]string{
"delimiter": "/",
"encoding-type": "url",
},
},
output: testOutput{
response: &Response{
Data: urlEncodedResult,
Headers: map[string]*string{
"x-amz-bucket-region": utils.GetStringPtr(defaultRegion),
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
},
},
{
name: "verify access fails",
input: testInput{
+86
View File
@@ -1225,3 +1225,89 @@ func ParseContentEncoding(ctx fiber.Ctx) string {
return strings.Join(kept, ",")
}
// ParseEncodingType validates the encoding-type query parameter of the object
// listing operations. S3 accepts "url" in any letter case and echoes the value
// as sent. Any other value, including an empty one, is rejected.
func ParseEncodingType(ctx fiber.Ctx) (types.EncodingType, error) {
if !ctx.Request().URI().QueryArgs().Has("encoding-type") {
return "", nil
}
value := ctx.Query("encoding-type")
if !strings.EqualFold(value, string(types.EncodingTypeUrl)) {
debuglogger.Logf("invalid encoding-type: %q", value)
return "", s3err.GetInvalidArgumentErr(s3err.InvalidArgEncodingType, value)
}
return types.EncodingType(value), nil
}
// s3KeyNameEscaper adjusts url.QueryEscape output to the encoding S3 uses for
// encoding-type=url: "/" and "*" stay literal and "~" is escaped. Every "%" in
// the QueryEscape output starts a full escape, so "%2A" can only come from "*".
var s3KeyNameEscaper = strings.NewReplacer("%2F", "/", "%2A", "*", "~", "%7E")
// urlEncodeKeyName encodes a key name the way S3 does for encoding-type=url:
// as a query component, so a space becomes "+" and "+" becomes "%2B", with "/"
// and "*" left as is and "~" escaped as "%7E".
func urlEncodeKeyName(name *string) *string {
if name == nil {
return nil
}
encoded := s3KeyNameEscaper.Replace(url.QueryEscape(*name))
return &encoded
}
// urlEncodeCommonPrefixes returns a copy of prefixes with each prefix encoded.
func urlEncodeCommonPrefixes(prefixes []types.CommonPrefix) []types.CommonPrefix {
if prefixes == nil {
return nil
}
encoded := make([]types.CommonPrefix, len(prefixes))
for i, cp := range prefixes {
cp.Prefix = urlEncodeKeyName(cp.Prefix)
encoded[i] = cp
}
return encoded
}
// urlEncodeObjectKeys returns a copy of objects with each key encoded.
func urlEncodeObjectKeys(objects []s3response.Object) []s3response.Object {
if objects == nil {
return nil
}
encoded := make([]s3response.Object, len(objects))
for i, obj := range objects {
obj.Key = urlEncodeKeyName(obj.Key)
encoded[i] = obj
}
return encoded
}
// URLEncodeListObjectsResult returns res as S3 answers a ListObjects request
// with encoding-type=url: EncodingType set to the value as sent and the
// Delimiter, Marker, Prefix, NextMarker, Key and CommonPrefixes values encoded.
// The backend's values are left untouched.
func URLEncodeListObjectsResult(res s3response.ListObjectsResult, encodingType types.EncodingType) s3response.ListObjectsResult {
res.EncodingType = encodingType
res.Delimiter = urlEncodeKeyName(res.Delimiter)
res.Marker = urlEncodeKeyName(res.Marker)
res.Prefix = urlEncodeKeyName(res.Prefix)
res.NextMarker = urlEncodeKeyName(res.NextMarker)
res.Contents = urlEncodeObjectKeys(res.Contents)
res.CommonPrefixes = urlEncodeCommonPrefixes(res.CommonPrefixes)
return res
}
// URLEncodeListObjectsV2Result returns res as S3 answers a ListObjectsV2
// request with encoding-type=url: EncodingType set to the value as sent and the
// Delimiter, Prefix, StartAfter, Key and CommonPrefixes values encoded. The
// continuation tokens are opaque and stay as they are.
func URLEncodeListObjectsV2Result(res s3response.ListObjectsV2Result, encodingType types.EncodingType) s3response.ListObjectsV2Result {
res.EncodingType = encodingType
res.Delimiter = urlEncodeKeyName(res.Delimiter)
res.Prefix = urlEncodeKeyName(res.Prefix)
res.StartAfter = urlEncodeKeyName(res.StartAfter)
res.Contents = urlEncodeObjectKeys(res.Contents)
res.CommonPrefixes = urlEncodeCommonPrefixes(res.CommonPrefixes)
return res
}
+69
View File
@@ -1906,3 +1906,72 @@ func TestFormatRetainUntilDate(t *testing.T) {
})
}
}
func TestParseEncodingType(t *testing.T) {
tests := []struct {
name string
query string
want types.EncodingType
bad bool
value string
}{
{name: "absent", query: ""},
{name: "url", query: "?encoding-type=url", want: "url"},
{name: "upper case, echoed as sent", query: "?encoding-type=URL", want: "URL"},
{name: "mixed case, echoed as sent", query: "?encoding-type=uRl", want: "uRl"},
{name: "empty value", query: "?encoding-type=", bad: true, value: ""},
{name: "bare parameter", query: "?encoding-type", bad: true, value: ""},
{name: "other method", query: "?encoding-type=base64", bad: true, value: "base64"},
{name: "gibberish", query: "?encoding-type=jdfkllaj", bad: true, value: "jdfkllaj"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := fiberCtxFromURL(t, http.MethodGet, "http://localhost/bucket"+tt.query, http.Header{})
encodingType, err := ParseEncodingType(ctx)
if tt.bad {
assert.Equal(t, s3err.GetInvalidArgumentErr(s3err.InvalidArgEncodingType, tt.value), err)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.want, encodingType)
})
}
}
func TestURLEncodeKeyName(t *testing.T) {
tests := []struct {
name, want string
}{
{"a+ b.txt", "a%2B+b.txt"},
{"dir/sub dir/", "dir/sub+dir/"},
{"100%", "100%25"},
{"a%2Fb", "a%252Fb"},
{"ctl\x01char", "ctl%01char"},
{"é", "%C3%A9"},
{"plain-key_1.txt", "plain-key_1.txt"},
// S3 leaves "*" literal and escapes "~", unlike url.QueryEscape
{"a*b~c", "a*b%7Ec"},
{" !\"#$%&'()*+,-.:;<=>?@[\\]^_`{|}~", "+%21%22%23%24%25%26%27%28%29*%2B%2C-.%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%5E_%60%7B%7C%7D%7E"},
}
for _, tt := range tests {
got := urlEncodeKeyName(&tt.name)
assert.Equal(t, tt.want, *got, tt.name)
decoded, err := url.QueryUnescape(*got)
assert.NoError(t, err)
assert.Equal(t, tt.name, decoded)
}
assert.Nil(t, urlEncodeKeyName(nil))
}
func TestURLEncodeListObjectsResultKeepsBackendValues(t *testing.T) {
key := "a b"
res := s3response.ListObjectsResult{
Contents: []s3response.Object{{Key: &key}},
}
encoded := URLEncodeListObjectsResult(res, types.EncodingTypeUrl)
assert.Equal(t, "a+b", *encoded.Contents[0].Key)
assert.Equal(t, "a b", *res.Contents[0].Key)
assert.Equal(t, "a b", key)
}
+5
View File
@@ -78,6 +78,7 @@ const (
InvalidArgUploadIdWithUploads
InvalidArgBucketRegionEndpoint
InvalidArgEmptyETag
InvalidArgEncodingType
)
var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
@@ -153,6 +154,10 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
Description: "aws-chunked encoding is not supported when x-amz-content-sha256 UNSIGNED-PAYLOAD is supplied.",
ArgumentName: "Content-Encoding",
},
InvalidArgEncodingType: {
Description: "Invalid Encoding Method specified in Request",
ArgumentName: "encoding-type",
},
InvalidArgCopySource: {
Description: "You can only specify a copy source header for copy requests.",
ArgumentName: "x-amz-copy-source",
+2 -2
View File
@@ -178,7 +178,7 @@ type ListObjectsResult struct {
IsTruncated *bool
Contents []Object
CommonPrefixes []types.CommonPrefix
EncodingType types.EncodingType
EncodingType types.EncodingType `xml:",omitempty"`
}
type ListObjectsV2Result struct {
@@ -194,7 +194,7 @@ type ListObjectsV2Result struct {
IsTruncated *bool
Contents []Object
CommonPrefixes []types.CommonPrefix
EncodingType types.EncodingType
EncodingType types.EncodingType `xml:",omitempty"`
}
type Object struct {
+43
View File
@@ -894,3 +894,46 @@ func ListObjects_mp_masking_delimiter(s *S3Conf) error {
return nil
})
}
// ListObjects_url_encoding lists keys that need escaping with
// encoding-type=url: S3 returns EncodingType and encodes the key names as a
// query component, with "/" left as is. Any other encoding type is rejected.
func ListObjects_url_encoding(s *S3Conf) error {
testName := "ListObjects_url_encoding"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
_, err := putObjects(s3client, []string{"a+ b.txt", "dir a/obj"}, bucket)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{
Bucket: &bucket,
Delimiter: getPtr("/"),
EncodingType: types.EncodingTypeUrl,
})
cancel()
if err != nil {
return err
}
if out.EncodingType != types.EncodingTypeUrl {
return fmt.Errorf("expected the encoding type to be %q, instead got %q",
types.EncodingTypeUrl, out.EncodingType)
}
if len(out.Contents) != 1 || getString(out.Contents[0].Key) != "a%2B+b.txt" {
return fmt.Errorf("expected the key %q, instead got %v", "a%2B+b.txt", out.Contents)
}
if len(out.CommonPrefixes) != 1 || getString(out.CommonPrefixes[0].Prefix) != "dir+a/" {
return fmt.Errorf("expected the common prefix %q, instead got %v", "dir+a/", out.CommonPrefixes)
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.ListObjects(ctx, &s3.ListObjectsInput{
Bucket: &bucket,
EncodingType: "gibberish",
})
cancel()
return checkApiErr(err, s3err.GetInvalidArgumentErr(s3err.InvalidArgEncodingType, "gibberish"))
})
}
+42
View File
@@ -20,6 +20,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/versity/versitygw/s3err"
)
func ListObjectsV2_start_after(s *S3Conf) error {
@@ -1091,3 +1092,44 @@ func ListObjectsV2_pagination_with_delimiter(s *S3Conf) error {
return nil
})
}
// ListObjectsV2_url_encoding is ListObjects_url_encoding for ListObjectsV2.
func ListObjectsV2_url_encoding(s *S3Conf) error {
testName := "ListObjectsV2_url_encoding"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
_, err := putObjects(s3client, []string{"a+ b.txt", "dir a/obj"}, bucket)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{
Bucket: &bucket,
Delimiter: getPtr("/"),
EncodingType: types.EncodingTypeUrl,
})
cancel()
if err != nil {
return err
}
if out.EncodingType != types.EncodingTypeUrl {
return fmt.Errorf("expected the encoding type to be %q, instead got %q",
types.EncodingTypeUrl, out.EncodingType)
}
if len(out.Contents) != 1 || getString(out.Contents[0].Key) != "a%2B+b.txt" {
return fmt.Errorf("expected the key %q, instead got %v", "a%2B+b.txt", out.Contents)
}
if len(out.CommonPrefixes) != 1 || getString(out.CommonPrefixes[0].Prefix) != "dir+a/" {
return fmt.Errorf("expected the common prefix %q, instead got %v", "dir+a/", out.CommonPrefixes)
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{
Bucket: &bucket,
EncodingType: "gibberish",
})
cancel()
return checkApiErr(err, s3err.GetInvalidArgumentErr(s3err.InvalidArgEncodingType, "gibberish"))
})
}
+4
View File
@@ -329,6 +329,7 @@ func TestListObjects(ts *TestState) {
ts.Run(ListObjects_mp_masking_with_marker)
ts.Run(ListObjects_mp_masking_truncation)
ts.Run(ListObjects_mp_masking_delimiter)
ts.Run(ListObjects_url_encoding)
//TODO: remove the condition after implementing checksums in azure
if !ts.conf.azureTests {
ts.Run(ListObjects_with_checksum)
@@ -349,6 +350,7 @@ func TestListObjectsV2(ts *TestState) {
ts.Run(ListObjectsV2_list_all_objs)
ts.Run(ListObjectsV2_full_pagination)
ts.Run(ListObjectsV2_pagination_with_delimiter)
ts.Run(ListObjectsV2_url_encoding)
ts.Run(ListObjectsV2_with_owner)
ts.Run(ListObjectsV2_non_truncated_common_prefixes)
//TODO: remove the condition after implementing checksums in azure
@@ -3187,6 +3189,7 @@ func GetIntTests() IntTests {
"ListObjects_mp_masking_with_marker": ListObjects_mp_masking_with_marker,
"ListObjects_mp_masking_truncation": ListObjects_mp_masking_truncation,
"ListObjects_mp_masking_delimiter": ListObjects_mp_masking_delimiter,
"ListObjects_url_encoding": ListObjects_url_encoding,
"ListObjectsV2_non_truncated_common_prefixes": ListObjectsV2_non_truncated_common_prefixes,
"ListObjectsV2_invalid_parent_prefix": ListObjectsV2_invalid_parent_prefix,
"ListObjectsV2_should_not_list_pending_mps": ListObjectsV2_should_not_list_pending_mps,
@@ -3206,6 +3209,7 @@ func GetIntTests() IntTests {
"ListObjectsV2_list_all_objs": ListObjectsV2_list_all_objs,
"ListObjectsV2_full_pagination": ListObjectsV2_full_pagination,
"ListObjectsV2_pagination_with_delimiter": ListObjectsV2_pagination_with_delimiter,
"ListObjectsV2_url_encoding": ListObjectsV2_url_encoding,
"ListObjectsV2_with_owner": ListObjectsV2_with_owner,
"ListObjectsV2_with_checksum": ListObjectsV2_with_checksum,
"ListObjectVersions_VD_success": ListObjectVersions_VD_success,
-7
View File
@@ -154,19 +154,12 @@ source ./tests/drivers/objects_and_versions.sh
# tags: curl,ListObjects,encoding-type,invalid-query
@test "REST - ListObjects - invalid encoding" {
if [ "$DIRECT" != "true" ]; then
skip "https://github.com/versity/versitygw/issues/1985"
fi
run objects_versions_invalid_encoding ""
assert_success
}
# tags: curl,ListObjects,encoding-type
@test "REST - ListObjects - encoding success" {
if [ "$DIRECT" != "true" ]; then
skip "https://github.com/versity/versitygw/issues/1985"
fi
run objects_versions_encoding_success "" "ListBucketResult" "Contents"
assert_success
}