mirror of
https://github.com/versity/versitygw.git
synced 2026-09-22 16:04:15 +00:00
fix: reject empty versionId query parameter in object actions
S3 returns `InvalidArgument` when an object action receives a `versionId` query parameter with no value. The gateway silently treated it as an unversioned request instead. Added a shared `versionId` validation helper and apply it to the object actions that accept the parameter, so malformed requests are rejected up front rather than reaching the backend.
This commit is contained in:
@@ -36,6 +36,14 @@ func (c S3ApiController) DeleteObjectTagging(ctx fiber.Ctx) (*Response, error) {
|
||||
isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
action := auth.DeleteObjectTaggingAction
|
||||
if versionId != "" {
|
||||
action = auth.DeleteObjectVersionTaggingAction
|
||||
@@ -131,6 +139,14 @@ func (c S3ApiController) DeleteObject(ctx fiber.Ctx) (*Response, error) {
|
||||
isBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
action := auth.DeleteObjectAction
|
||||
if versionId != "" {
|
||||
action = auth.DeleteObjectVersionAction
|
||||
|
||||
@@ -33,6 +33,23 @@ func TestS3ApiController_DeleteObjectTagging(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
@@ -207,6 +224,23 @@ func TestS3ApiController_DeleteObject(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
|
||||
@@ -40,6 +40,14 @@ func (c S3ApiController) GetObjectTagging(ctx fiber.Ctx) (*Response, error) {
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
isPublicBucket := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
action := auth.GetObjectTaggingAction
|
||||
if versionId != "" {
|
||||
action = auth.GetObjectVersionTaggingAction
|
||||
@@ -101,6 +109,14 @@ func (c S3ApiController) GetObjectRetention(ctx fiber.Ctx) (*Response, error) {
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
isPublicBucket := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
err := c.verifyAccess(ctx, auth.AccessOptions{
|
||||
Acl: parsedAcl,
|
||||
AclPermission: auth.PermissionRead,
|
||||
@@ -147,6 +163,14 @@ func (c S3ApiController) GetObjectLegalHold(ctx fiber.Ctx) (*Response, error) {
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
isPublicBucket := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
err := c.verifyAccess(ctx, auth.AccessOptions{
|
||||
Acl: parsedAcl,
|
||||
AclPermission: auth.PermissionRead,
|
||||
@@ -289,6 +313,14 @@ func (c S3ApiController) GetObjectAttributes(ctx fiber.Ctx) (*Response, error) {
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
isPublicBucket := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
action := auth.GetObjectAttributesAction
|
||||
if versionId != "" {
|
||||
action = auth.GetObjectVersionAttributesAction
|
||||
@@ -412,6 +444,14 @@ func (c S3ApiController) GetObject(ctx fiber.Ctx) (*Response, error) {
|
||||
}, s3err.GetAPIError(s3err.ErrAnonymousResponseHeaders)
|
||||
}
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
action := auth.GetObjectAction
|
||||
if ctx.Request().URI().QueryArgs().Has("versionId") {
|
||||
action = auth.GetObjectVersionAction
|
||||
|
||||
@@ -40,6 +40,23 @@ func TestS3ApiController_GetObjectTagging(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
@@ -142,6 +159,23 @@ func TestS3ApiController_GetObjectRetention(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
@@ -245,6 +279,23 @@ func TestS3ApiController_GetObjectLegalHold(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
@@ -552,6 +603,23 @@ func TestS3ApiController_GetObjectAttributes(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
@@ -674,6 +742,23 @@ func TestS3ApiController_GetObject(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
|
||||
@@ -71,6 +71,14 @@ func (c S3ApiController) HeadObject(ctx fiber.Ctx) (*Response, error) {
|
||||
}, s3err.GetAPIError(s3err.ErrAnonymousResponseHeaders)
|
||||
}
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
action := auth.GetObjectAction
|
||||
if ctx.Request().URI().QueryArgs().Has("versionId") {
|
||||
action = auth.GetObjectVersionAction
|
||||
|
||||
@@ -39,6 +39,23 @@ func TestS3ApiController_HeadObject(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
|
||||
@@ -42,6 +42,14 @@ func (c S3ApiController) PutObjectTagging(ctx fiber.Ctx) (*Response, error) {
|
||||
IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
action := auth.PutObjectTaggingAction
|
||||
if versionId != "" {
|
||||
action = auth.PutObjectVersionTaggingAction
|
||||
@@ -96,6 +104,14 @@ func (c S3ApiController) PutObjectRetention(ctx fiber.Ctx) (*Response, error) {
|
||||
IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
err := c.verifyAccess(ctx, auth.AccessOptions{
|
||||
Acl: parsedAcl,
|
||||
AclPermission: auth.PermissionWrite,
|
||||
@@ -161,6 +177,14 @@ func (c S3ApiController) PutObjectLegalHold(ctx fiber.Ctx) (*Response, error) {
|
||||
IsBucketPublic := utils.ContextKeyPublicBucket.IsSet(ctx)
|
||||
parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL)
|
||||
|
||||
if err := utils.ValidateVersionId(ctx); err != nil {
|
||||
return &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: parsedAcl.Owner,
|
||||
},
|
||||
}, err
|
||||
}
|
||||
|
||||
err := c.verifyAccess(ctx, auth.AccessOptions{
|
||||
Acl: parsedAcl,
|
||||
AclPermission: auth.PermissionWrite,
|
||||
|
||||
@@ -53,6 +53,23 @@ func TestS3ApiController_PutObjectTagging(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
@@ -172,6 +189,23 @@ func TestS3ApiController_PutObjectRetention(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
@@ -301,6 +335,23 @@ func TestS3ApiController_PutObjectLegalHold(t *testing.T) {
|
||||
input testInput
|
||||
output testOutput
|
||||
}{
|
||||
{
|
||||
name: "empty versionId query param",
|
||||
input: testInput{
|
||||
locals: defaultLocals,
|
||||
queries: map[string]string{
|
||||
"versionId": "",
|
||||
},
|
||||
},
|
||||
output: testOutput{
|
||||
response: &Response{
|
||||
MetaOpts: &MetaOptions{
|
||||
BucketOwner: "root",
|
||||
},
|
||||
},
|
||||
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "verify access fails",
|
||||
input: testInput{
|
||||
|
||||
@@ -962,6 +962,20 @@ func GetQueryParam(ctx fiber.Ctx, key string) *string {
|
||||
return &value
|
||||
}
|
||||
|
||||
// ValidateVersionId ensures the versionId query parameter, if specified, isn't
|
||||
// empty. S3 rejects both "?versionId=" and the valueless "?versionId" form, and
|
||||
// rejects the request if any of the repeated values is empty.
|
||||
func ValidateVersionId(ctx fiber.Ctx) error {
|
||||
for _, val := range ctx.Request().URI().QueryArgs().PeekMulti("versionId") {
|
||||
if len(val) == 0 {
|
||||
debuglogger.Logf("empty versionId query parameter")
|
||||
return s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, "")
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// ApplyOverride returns the override value if it exists and status is 200, otherwise returns original
|
||||
func ApplyOverride(original, override *string) *string {
|
||||
if override != nil {
|
||||
|
||||
@@ -52,6 +52,7 @@ const (
|
||||
InvalidArgMetadataDirective
|
||||
InvalidArgTaggingDirective
|
||||
InvalidArgVersionId
|
||||
InvalidArgEmptyVersionId
|
||||
InvalidArgChecksumPart
|
||||
InvalidArgMissingUploadId
|
||||
InvalidArgUploadIdMarker
|
||||
@@ -175,6 +176,10 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
|
||||
Description: "Invalid version id specified",
|
||||
ArgumentName: "versionId",
|
||||
},
|
||||
InvalidArgEmptyVersionId: {
|
||||
Description: "Version id cannot be the empty string",
|
||||
ArgumentName: "versionId",
|
||||
},
|
||||
InvalidArgChecksumPart: {
|
||||
Description: "Invalid Base64 or multiple checksums present in request",
|
||||
ArgumentName: "Checksum",
|
||||
|
||||
@@ -383,3 +383,7 @@ func DeleteObject_expected_bucket_owner(s *S3Conf) error {
|
||||
return err
|
||||
})
|
||||
}
|
||||
|
||||
func DeleteObject_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "DeleteObject_empty_version_id", http.MethodDelete, "", nil)
|
||||
}
|
||||
|
||||
@@ -196,3 +196,7 @@ func DeleteObjectTagging_expected_bucket_owner(s *S3Conf) error {
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
func DeleteObjectTagging_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "DeleteObjectTagging_empty_version_id", http.MethodDelete, "tagging", nil)
|
||||
}
|
||||
|
||||
@@ -1767,3 +1767,7 @@ func GetObject_ranged_with_checksum_mode(s *S3Conf) error {
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
func GetObject_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "GetObject_empty_version_id", http.MethodGet, "", nil)
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ import (
|
||||
"crypto/rand"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
@@ -273,3 +274,7 @@ func GetObjectAttributes_checksums(s *S3Conf) error {
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
func GetObjectAttributes_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "GetObjectAttributes_empty_version_id", http.MethodGet, "attributes", nil)
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ package integration
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
@@ -143,3 +144,7 @@ func GetObjectLegalHold_success(s *S3Conf) error {
|
||||
return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: key, removeOnlyLeglHold: true}})
|
||||
}, withLock())
|
||||
}
|
||||
|
||||
func GetObjectLegalHold_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "GetObjectLegalHold_empty_version_id", http.MethodGet, "legal-hold", nil)
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ package integration
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
@@ -157,3 +158,7 @@ func GetObjectRetention_success(s *S3Conf) error {
|
||||
return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: key, isCompliance: true}})
|
||||
}, withLock())
|
||||
}
|
||||
|
||||
func GetObjectRetention_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "GetObjectRetention_empty_version_id", http.MethodGet, "retention", nil)
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ package integration
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
@@ -128,3 +129,7 @@ func GetObjectTagging_success(s *S3Conf) error {
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
func GetObjectTagging_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "GetObjectTagging_empty_version_id", http.MethodGet, "tagging", nil)
|
||||
}
|
||||
|
||||
@@ -1510,3 +1510,7 @@ func HeadObject_overrides_fail_public(s *S3Conf) error {
|
||||
return nil
|
||||
}, withAnonymousClient())
|
||||
}
|
||||
|
||||
func HeadObject_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "HeadObject_empty_version_id", http.MethodHead, "", nil)
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ package integration
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
@@ -149,3 +150,7 @@ func PutObjectLegalHold_success(s *S3Conf) error {
|
||||
return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: key, removeOnlyLeglHold: true}})
|
||||
}, withLock())
|
||||
}
|
||||
|
||||
func PutObjectLegalHold_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "PutObjectLegalHold_empty_version_id", http.MethodPut, "legal-hold", []byte("<LegalHold><Status>ON</Status></LegalHold>"))
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ package integration
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
@@ -648,3 +649,7 @@ func PutObjectRetention_success(s *S3Conf) error {
|
||||
return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: key, isCompliance: true}})
|
||||
}, withLock())
|
||||
}
|
||||
|
||||
func PutObjectRetention_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "PutObjectRetention_empty_version_id", http.MethodPut, "retention", []byte("<Retention><Mode>GOVERNANCE</Mode><RetainUntilDate>2222-01-01T00:00:00Z</RetainUntilDate></Retention>"))
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ package integration
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
@@ -229,3 +230,7 @@ func PutObjectTagging_success(s *S3Conf) error {
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
func PutObjectTagging_empty_version_id(s *S3Conf) error {
|
||||
return testEmptyVersionId(s, "PutObjectTagging_empty_version_id", http.MethodPut, "tagging", []byte("<Tagging><TagSet><Tag><Key>key</Key><Value>val</Value></Tag></TagSet></Tagging>"))
|
||||
}
|
||||
|
||||
@@ -234,6 +234,7 @@ func TestHeadObject(ts *TestState) {
|
||||
ts.Run(HeadObject_mp_part_number_resp_status)
|
||||
ts.Run(HeadObject_non_mp_part_number_1_success)
|
||||
ts.Run(HeadObject_empty_object_part_number_1)
|
||||
ts.Run(HeadObject_empty_version_id)
|
||||
}
|
||||
|
||||
func TestGetObjectAttributes(ts *TestState) {
|
||||
@@ -248,6 +249,7 @@ func TestGetObjectAttributes(ts *TestState) {
|
||||
if !ts.conf.azureTests {
|
||||
ts.Run(GetObjectAttributes_checksums)
|
||||
}
|
||||
ts.Run(GetObjectAttributes_empty_version_id)
|
||||
}
|
||||
|
||||
func TestGetObject(ts *TestState) {
|
||||
@@ -281,6 +283,7 @@ func TestGetObject(ts *TestState) {
|
||||
ts.Run(GetObject_mp_part_number_resp_status)
|
||||
ts.Run(GetObject_non_mp_part_number_1_success)
|
||||
ts.Run(GetObject_empty_object_part_number_1)
|
||||
ts.Run(GetObject_empty_version_id)
|
||||
}
|
||||
|
||||
func TestListObjects(ts *TestState) {
|
||||
@@ -350,6 +353,7 @@ func TestDeleteObject(ts *TestState) {
|
||||
ts.Run(DeleteObject_success_status_code)
|
||||
ts.Run(DeleteObject_incorrect_expected_bucket_owner)
|
||||
ts.Run(DeleteObject_expected_bucket_owner)
|
||||
ts.Run(DeleteObject_empty_version_id)
|
||||
}
|
||||
|
||||
func TestDeleteObjects(ts *TestState) {
|
||||
@@ -406,6 +410,7 @@ func TestPutObjectTagging(ts *TestState) {
|
||||
ts.Run(PutObjectTagging_tag_count_limit)
|
||||
ts.Run(PutObjectTagging_invalid_tags)
|
||||
ts.Run(PutObjectTagging_success)
|
||||
ts.Run(PutObjectTagging_empty_version_id)
|
||||
}
|
||||
|
||||
func TestGetObjectTagging(ts *TestState) {
|
||||
@@ -413,6 +418,7 @@ func TestGetObjectTagging(ts *TestState) {
|
||||
ts.Run(GetObjectTagging_unset_tags)
|
||||
ts.Run(GetObjectTagging_invalid_parent)
|
||||
ts.Run(GetObjectTagging_success)
|
||||
ts.Run(GetObjectTagging_empty_version_id)
|
||||
}
|
||||
|
||||
func TestDeleteObjectTagging(ts *TestState) {
|
||||
@@ -420,6 +426,7 @@ func TestDeleteObjectTagging(ts *TestState) {
|
||||
ts.Run(DeleteObjectTagging_success_status)
|
||||
ts.Run(DeleteObjectTagging_success)
|
||||
ts.Run(DeleteObjectTagging_expected_bucket_owner)
|
||||
ts.Run(DeleteObjectTagging_empty_version_id)
|
||||
}
|
||||
|
||||
func TestCreateMultipartUpload(ts *TestState) {
|
||||
@@ -782,6 +789,7 @@ func TestPutObjectRetention(ts *TestState) {
|
||||
ts.Run(PutObjectRetention_shorten_compliance_denied)
|
||||
ts.Run(PutObjectRetention_rewrite_same_date)
|
||||
ts.Run(PutObjectRetention_success)
|
||||
ts.Run(PutObjectRetention_empty_version_id)
|
||||
}
|
||||
|
||||
func TestGetObjectRetention(ts *TestState) {
|
||||
@@ -790,6 +798,7 @@ func TestGetObjectRetention(ts *TestState) {
|
||||
ts.Run(GetObjectRetention_disabled_lock)
|
||||
ts.Run(GetObjectRetention_unset_config)
|
||||
ts.Run(GetObjectRetention_success)
|
||||
ts.Run(GetObjectRetention_empty_version_id)
|
||||
}
|
||||
|
||||
func TestPutObjectLegalHold(ts *TestState) {
|
||||
@@ -799,6 +808,7 @@ func TestPutObjectLegalHold(ts *TestState) {
|
||||
ts.Run(PutObjectLegalHold_invalid_status)
|
||||
ts.Run(PutObjectLegalHold_unset_bucket_object_lock_config)
|
||||
ts.Run(PutObjectLegalHold_success)
|
||||
ts.Run(PutObjectLegalHold_empty_version_id)
|
||||
}
|
||||
|
||||
func TestGetObjectLegalHold(ts *TestState) {
|
||||
@@ -807,6 +817,7 @@ func TestGetObjectLegalHold(ts *TestState) {
|
||||
ts.Run(GetObjectLegalHold_disabled_lock)
|
||||
ts.Run(GetObjectLegalHold_unset_config)
|
||||
ts.Run(GetObjectLegalHold_success)
|
||||
ts.Run(GetObjectLegalHold_empty_version_id)
|
||||
}
|
||||
|
||||
func TestNotImplementedActions(ts *TestState) {
|
||||
@@ -2884,6 +2895,7 @@ func GetIntTests() IntTests {
|
||||
"HeadObject_mp_part_number_exceeds_parts_count": HeadObject_mp_part_number_exceeds_parts_count,
|
||||
"HeadObject_mp_part_number_success": HeadObject_mp_part_number_success,
|
||||
"HeadObject_mp_part_number_resp_status": HeadObject_mp_part_number_resp_status,
|
||||
"HeadObject_empty_version_id": HeadObject_empty_version_id,
|
||||
"HeadObject_non_mp_part_number_1_success": HeadObject_non_mp_part_number_1_success,
|
||||
"HeadObject_empty_object_part_number_1": HeadObject_empty_object_part_number_1,
|
||||
"GetObjectAttributes_non_existing_bucket": GetObjectAttributes_non_existing_bucket,
|
||||
@@ -2894,6 +2906,7 @@ func GetIntTests() IntTests {
|
||||
"GetObjectAttributes_empty_attrs": GetObjectAttributes_empty_attrs,
|
||||
"GetObjectAttributes_existing_object": GetObjectAttributes_existing_object,
|
||||
"GetObjectAttributes_checksums": GetObjectAttributes_checksums,
|
||||
"GetObjectAttributes_empty_version_id": GetObjectAttributes_empty_version_id,
|
||||
"GetObject_non_existing_key": GetObject_non_existing_key,
|
||||
"GetObject_directory_object_noslash": GetObject_directory_object_noslash,
|
||||
"GetObject_with_range": GetObject_with_range,
|
||||
@@ -2921,6 +2934,7 @@ func GetIntTests() IntTests {
|
||||
"GetObject_mp_part_number_resp_status": GetObject_mp_part_number_resp_status,
|
||||
"GetObject_non_mp_part_number_1_success": GetObject_non_mp_part_number_1_success,
|
||||
"GetObject_empty_object_part_number_1": GetObject_empty_object_part_number_1,
|
||||
"GetObject_empty_version_id": GetObject_empty_version_id,
|
||||
"ListObjects_non_existing_bucket": ListObjects_non_existing_bucket,
|
||||
"ListObjects_with_prefix": ListObjects_with_prefix,
|
||||
"ListObjects_truncated": ListObjects_truncated,
|
||||
@@ -2970,6 +2984,7 @@ func GetIntTests() IntTests {
|
||||
"DeleteObject_directory_object": DeleteObject_directory_object,
|
||||
"DeleteObject_success": DeleteObject_success,
|
||||
"DeleteObject_success_status_code": DeleteObject_success_status_code,
|
||||
"DeleteObject_empty_version_id": DeleteObject_empty_version_id,
|
||||
"DeleteObject_incorrect_expected_bucket_owner": DeleteObject_incorrect_expected_bucket_owner,
|
||||
"DeleteObject_expected_bucket_owner": DeleteObject_expected_bucket_owner,
|
||||
"DeleteObjects_empty_input": DeleteObjects_empty_input,
|
||||
@@ -3018,13 +3033,16 @@ func GetIntTests() IntTests {
|
||||
"PutObjectTagging_tag_count_limit": PutObjectTagging_tag_count_limit,
|
||||
"PutObjectTagging_invalid_tags": PutObjectTagging_invalid_tags,
|
||||
"PutObjectTagging_success": PutObjectTagging_success,
|
||||
"PutObjectTagging_empty_version_id": PutObjectTagging_empty_version_id,
|
||||
"GetObjectTagging_non_existing_object": GetObjectTagging_non_existing_object,
|
||||
"GetObjectTagging_unset_tags": GetObjectTagging_unset_tags,
|
||||
"GetObjectTagging_invalid_parent": GetObjectTagging_invalid_parent,
|
||||
"GetObjectTagging_success": GetObjectTagging_success,
|
||||
"GetObjectTagging_empty_version_id": GetObjectTagging_empty_version_id,
|
||||
"DeleteObjectTagging_non_existing_object": DeleteObjectTagging_non_existing_object,
|
||||
"DeleteObjectTagging_success_status": DeleteObjectTagging_success_status,
|
||||
"DeleteObjectTagging_success": DeleteObjectTagging_success,
|
||||
"DeleteObjectTagging_empty_version_id": DeleteObjectTagging_empty_version_id,
|
||||
"DeleteObjectTagging_expected_bucket_owner": DeleteObjectTagging_expected_bucket_owner,
|
||||
"CreateMultipartUpload_non_existing_bucket": CreateMultipartUpload_non_existing_bucket,
|
||||
"CreateMultipartUpload_long_metadata": CreateMultipartUpload_long_metadata,
|
||||
@@ -3291,22 +3309,26 @@ func GetIntTests() IntTests {
|
||||
"PutObjectRetention_shorten_compliance_denied": PutObjectRetention_shorten_compliance_denied,
|
||||
"PutObjectRetention_rewrite_same_date": PutObjectRetention_rewrite_same_date,
|
||||
"PutObjectRetention_success": PutObjectRetention_success,
|
||||
"PutObjectRetention_empty_version_id": PutObjectRetention_empty_version_id,
|
||||
"GetObjectRetention_non_existing_bucket": GetObjectRetention_non_existing_bucket,
|
||||
"GetObjectRetention_non_existing_object": GetObjectRetention_non_existing_object,
|
||||
"GetObjectRetention_disabled_lock": GetObjectRetention_disabled_lock,
|
||||
"GetObjectRetention_unset_config": GetObjectRetention_unset_config,
|
||||
"GetObjectRetention_success": GetObjectRetention_success,
|
||||
"GetObjectRetention_empty_version_id": GetObjectRetention_empty_version_id,
|
||||
"PutObjectLegalHold_non_existing_bucket": PutObjectLegalHold_non_existing_bucket,
|
||||
"PutObjectLegalHold_non_existing_object": PutObjectLegalHold_non_existing_object,
|
||||
"PutObjectLegalHold_invalid_body": PutObjectLegalHold_invalid_body,
|
||||
"PutObjectLegalHold_invalid_status": PutObjectLegalHold_invalid_status,
|
||||
"PutObjectLegalHold_unset_bucket_object_lock_config": PutObjectLegalHold_unset_bucket_object_lock_config,
|
||||
"PutObjectLegalHold_success": PutObjectLegalHold_success,
|
||||
"PutObjectLegalHold_empty_version_id": PutObjectLegalHold_empty_version_id,
|
||||
"GetObjectLegalHold_non_existing_bucket": GetObjectLegalHold_non_existing_bucket,
|
||||
"GetObjectLegalHold_non_existing_object": GetObjectLegalHold_non_existing_object,
|
||||
"GetObjectLegalHold_disabled_lock": GetObjectLegalHold_disabled_lock,
|
||||
"GetObjectLegalHold_unset_config": GetObjectLegalHold_unset_config,
|
||||
"GetObjectLegalHold_success": GetObjectLegalHold_success,
|
||||
"GetObjectLegalHold_empty_version_id": GetObjectLegalHold_empty_version_id,
|
||||
"PutBucketAnalyticsConfiguration_not_implemented": PutBucketAnalyticsConfiguration_not_implemented,
|
||||
"GetBucketAnalyticsConfiguration_not_implemented": GetBucketAnalyticsConfiguration_not_implemented,
|
||||
"ListBucketAnalyticsConfiguration_not_implemented": ListBucketAnalyticsConfiguration_not_implemented,
|
||||
|
||||
@@ -496,6 +496,61 @@ func checkHTTPResponseApiErr(resp *http.Response, expected s3err.S3Error) error
|
||||
return compareS3ApiError(expected, &errResp)
|
||||
}
|
||||
|
||||
// testEmptyVersionId verifies an action rejects an empty versionId query
|
||||
// parameter. The SDK drops empty query parameters, so the request has to be
|
||||
// signed and sent by hand. subresource is the action's query flag, e.g.
|
||||
// "tagging", and is empty for the actions addressed by the bare object path.
|
||||
// body is required for the actions whose routes are guarded by the checksum
|
||||
// middleware, which rejects empty bodies before the controller runs.
|
||||
func testEmptyVersionId(s *S3Conf, testName, method, subresource string, body []byte) error {
|
||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||
obj := "my-obj"
|
||||
_, err := putObjects(s3client, []string{obj}, bucket)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
var headers map[string]string
|
||||
if len(body) != 0 {
|
||||
sum := md5.Sum(body)
|
||||
headers = map[string]string{
|
||||
"Content-Md5": base64.StdEncoding.EncodeToString(sum[:]),
|
||||
}
|
||||
}
|
||||
|
||||
query := "versionId="
|
||||
if subresource != "" {
|
||||
query = fmt.Sprintf("%v&%v", subresource, query)
|
||||
}
|
||||
|
||||
req, err := createSignedReq(method, s.endpoint,
|
||||
fmt.Sprintf("%v/%v?%v", bucket, obj, query), s.awsID, s.awsSecret,
|
||||
"s3", s.awsRegion, "", body, time.Now(), headers)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
resp, err := s.httpClient.Do(req)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
expected := s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, "")
|
||||
// HEAD responses carry no body: only the status code is verifiable
|
||||
if method == http.MethodHead {
|
||||
resp.Body.Close()
|
||||
if resp.StatusCode != expected.StatusCode() {
|
||||
return fmt.Errorf("expected response status code to be %v, instead got %v",
|
||||
expected.StatusCode(), resp.StatusCode)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
return checkHTTPResponseApiErr(resp, expected)
|
||||
})
|
||||
}
|
||||
|
||||
func checkIAMAuthRequest(s *S3Conf, req *http.Request, expected iamerr.APIError) error {
|
||||
resp, err := s.httpClient.Do(req)
|
||||
if err != nil {
|
||||
@@ -1438,6 +1493,15 @@ func getPtr[T any](str T) *T {
|
||||
return &str
|
||||
}
|
||||
|
||||
// getNonEmptyPtr returns nil for an empty string, rather than a pointer to
|
||||
// the empty string.
|
||||
func getNonEmptyPtr(s string) *string {
|
||||
if s == "" {
|
||||
return nil
|
||||
}
|
||||
return &s
|
||||
}
|
||||
|
||||
func checksumHeaderName(algo types.ChecksumAlgorithm) string {
|
||||
return fmt.Sprintf("x-amz-checksum-%s", strings.ToLower(string(algo)))
|
||||
}
|
||||
@@ -2942,7 +3006,7 @@ func cleanupLockedObjects(client *s3.Client, bucket string, objs []objToDelete)
|
||||
_, err := client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
|
||||
Bucket: &bucket,
|
||||
Key: &obj.key,
|
||||
VersionId: getPtr(obj.versionId),
|
||||
VersionId: getNonEmptyPtr(obj.versionId),
|
||||
LegalHold: &types.ObjectLockLegalHold{
|
||||
Status: types.ObjectLockLegalHoldStatusOff, // Disable legal hold
|
||||
},
|
||||
@@ -2980,7 +3044,7 @@ func cleanupLockedObjects(client *s3.Client, bucket string, objs []objToDelete)
|
||||
_, err := client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
|
||||
Bucket: &bucket,
|
||||
Key: &obj.key,
|
||||
VersionId: getPtr(obj.versionId),
|
||||
VersionId: getNonEmptyPtr(obj.versionId),
|
||||
BypassGovernanceRetention: getBoolPtr(true),
|
||||
Retention: &types.ObjectLockRetention{
|
||||
Mode: types.ObjectLockRetentionModeGovernance,
|
||||
@@ -3021,7 +3085,7 @@ func waitOutComplianceRetention(client *s3.Client, bucket string, obj objToDelet
|
||||
out, err := client.GetObjectRetention(ctx, &s3.GetObjectRetentionInput{
|
||||
Bucket: &bucket,
|
||||
Key: &obj.key,
|
||||
VersionId: getPtr(obj.versionId),
|
||||
VersionId: getNonEmptyPtr(obj.versionId),
|
||||
})
|
||||
cancel()
|
||||
|
||||
@@ -3048,7 +3112,7 @@ func waitOutComplianceRetention(client *s3.Client, bucket string, obj objToDelet
|
||||
_, err := client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
|
||||
Bucket: &bucket,
|
||||
Key: &obj.key,
|
||||
VersionId: getPtr(obj.versionId),
|
||||
VersionId: getNonEmptyPtr(obj.versionId),
|
||||
Retention: &types.ObjectLockRetention{
|
||||
Mode: types.ObjectLockRetentionModeCompliance,
|
||||
RetainUntilDate: &retDate,
|
||||
@@ -3092,7 +3156,7 @@ func lockObject(client *s3.Client, mode objectLockMode, bucket, object, versionI
|
||||
_, err := client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
|
||||
Bucket: &bucket,
|
||||
Key: &object,
|
||||
VersionId: getPtr(versionId),
|
||||
VersionId: getNonEmptyPtr(versionId),
|
||||
LegalHold: &types.ObjectLockLegalHold{
|
||||
Status: types.ObjectLockLegalHoldStatusOn,
|
||||
},
|
||||
@@ -3110,7 +3174,7 @@ func lockObject(client *s3.Client, mode objectLockMode, bucket, object, versionI
|
||||
_, err := client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
|
||||
Bucket: &bucket,
|
||||
Key: &object,
|
||||
VersionId: getPtr(versionId),
|
||||
VersionId: getNonEmptyPtr(versionId),
|
||||
Retention: &types.ObjectLockRetention{
|
||||
Mode: m,
|
||||
RetainUntilDate: &date,
|
||||
|
||||
Reference in New Issue
Block a user