From 06e2f2183d8db0b955e27a23df3765c13e5950d9 Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Wed, 30 Oct 2024 15:42:15 -0400 Subject: [PATCH] fix: Changes GetObjectAttributes action xml encoding root element to GetObjectAttributesResponse. Adds input validation for x-amz-object-attributes header. Adds x-amz-delete-marker and x-maz-version-id headers for GetObjectAttributes action. Adds VersionId in HeadObject response, if it's not specified in the request --- backend/azure/azure.go | 10 +- backend/backend.go | 6 +- backend/posix/posix.go | 49 ++++-- backend/s3proxy/s3.go | 5 +- s3api/controllers/backend_moq_test.go | 6 +- s3api/controllers/base.go | 70 +++++++-- s3api/controllers/base_test.go | 4 +- s3api/utils/utils.go | 30 ++-- s3api/utils/utils_test.go | 51 ++++-- s3err/s3err.go | 6 + s3response/s3response.go | 46 +++--- tests/integration/group-tests.go | 9 ++ tests/integration/tests.go | 218 ++++++++++++++++++++++---- 13 files changed, 389 insertions(+), 121 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 35a803f..6c602e1 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -512,20 +512,22 @@ func (az *Azure) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3 return result, nil } -func (az *Azure) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) { +func (az *Azure) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) { data, err := az.HeadObject(ctx, &s3.HeadObjectInput{ Bucket: input.Bucket, Key: input.Key, }) if err != nil { - return s3response.GetObjectAttributesResult{}, err + return s3response.GetObjectAttributesResponse{}, err } - return s3response.GetObjectAttributesResult{ + return s3response.GetObjectAttributesResponse{ ETag: data.ETag, - LastModified: data.LastModified, ObjectSize: data.ContentLength, StorageClass: data.StorageClass, + LastModified: data.LastModified, + VersionId: data.VersionId, + DeleteMarker: data.DeleteMarker, }, nil } diff --git a/backend/backend.go b/backend/backend.go index 6a0ad73..1141d64 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -61,7 +61,7 @@ type Backend interface { HeadObject(context.Context, *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) GetObject(context.Context, *s3.GetObjectInput) (*s3.GetObjectOutput, error) GetObjectAcl(context.Context, *s3.GetObjectAclInput) (*s3.GetObjectAclOutput, error) - GetObjectAttributes(context.Context, *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) + GetObjectAttributes(context.Context, *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) CopyObject(context.Context, *s3.CopyObjectInput) (*s3.CopyObjectOutput, error) ListObjects(context.Context, *s3.ListObjectsInput) (s3response.ListObjectsResult, error) ListObjectsV2(context.Context, *s3.ListObjectsV2Input) (s3response.ListObjectsV2Result, error) @@ -185,8 +185,8 @@ func (BackendUnsupported) GetObject(context.Context, *s3.GetObjectInput) (*s3.Ge func (BackendUnsupported) GetObjectAcl(context.Context, *s3.GetObjectAclInput) (*s3.GetObjectAclOutput, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) } -func (BackendUnsupported) GetObjectAttributes(context.Context, *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) { - return s3response.GetObjectAttributesResult{}, s3err.GetAPIError(s3err.ErrNotImplemented) +func (BackendUnsupported) GetObjectAttributes(context.Context, *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) { + return s3response.GetObjectAttributesResponse{}, s3err.GetAPIError(s3err.ErrNotImplemented) } func (BackendUnsupported) CopyObject(context.Context, *s3.CopyObjectInput) (*s3.CopyObjectOutput, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 787e8c4..87d3c7e 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -2961,8 +2961,9 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. if input.Key == nil { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } + versionId := backend.GetStringFromPtr(input.VersionId) - if !p.versioningEnabled() && *input.VersionId != "" { + if !p.versioningEnabled() && versionId != "" { //TODO: Maybe we need to return our custom error here? return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId) } @@ -3022,7 +3023,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. return nil, fmt.Errorf("stat bucket: %w", err) } - if *input.VersionId != "" { + if versionId != "" { vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) @@ -3032,12 +3033,12 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. } if errors.Is(err, meta.ErrNoSuchKey) { bucket = filepath.Join(p.versioningDir, bucket) - object = filepath.Join(genObjVersionKey(object), *input.VersionId) + object = filepath.Join(genObjVersionKey(object), versionId) } - if string(vId) != *input.VersionId { + if string(vId) != versionId { bucket = filepath.Join(p.versioningDir, bucket) - object = filepath.Join(genObjVersionKey(object), *input.VersionId) + object = filepath.Join(genObjVersionKey(object), versionId) } } @@ -3045,7 +3046,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. fi, err := os.Stat(objPath) if errors.Is(err, fs.ErrNotExist) { - if *input.VersionId != "" { + if versionId != "" { return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId) } return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) @@ -3063,7 +3064,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } - if *input.VersionId != "" { + if p.versioningEnabled() { isDelMarker, err := p.isObjDeleteMarker(bucket, object) if err != nil { return nil, err @@ -3078,6 +3079,15 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. } } + if p.versioningEnabled() && versionId == "" { + vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) + if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { + return nil, fmt.Errorf("get object versionId: %v", err) + } + + versionId = string(vId) + } + userMetaData := make(map[string]string) contentType, contentEncoding := p.loadUserMetaData(bucket, object, userMetaData) @@ -3094,7 +3104,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. size := fi.Size() var objectLockLegalHoldStatus types.ObjectLockLegalHoldStatus - status, err := p.GetObjectLegalHold(ctx, bucket, object, *input.VersionId) + status, err := p.GetObjectLegalHold(ctx, bucket, object, versionId) if err == nil { if *status { objectLockLegalHoldStatus = types.ObjectLockLegalHoldStatusOn @@ -3105,7 +3115,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. var objectLockMode types.ObjectLockMode var objectLockRetainUntilDate *time.Time - retention, err := p.GetObjectRetention(ctx, bucket, object, *input.VersionId) + retention, err := p.GetObjectRetention(ctx, bucket, object, versionId) if err == nil { var config types.ObjectLockRetention if err := json.Unmarshal(retention, &config); err == nil { @@ -3114,8 +3124,6 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. } } - //TODO: the method must handle multipart upload case - return &s3.HeadObjectOutput{ ContentLength: &size, ContentType: &contentType, @@ -3127,25 +3135,34 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. ObjectLockMode: objectLockMode, ObjectLockRetainUntilDate: objectLockRetainUntilDate, StorageClass: types.StorageClassStandard, - VersionId: input.VersionId, + VersionId: &versionId, }, nil } -func (p *Posix) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) { +func (p *Posix) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) { data, err := p.HeadObject(ctx, &s3.HeadObjectInput{ Bucket: input.Bucket, Key: input.Key, VersionId: input.VersionId, }) if err != nil { - return s3response.GetObjectAttributesResult{}, nil + if errors.Is(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)) && data != nil { + return s3response.GetObjectAttributesResponse{ + DeleteMarker: data.DeleteMarker, + VersionId: data.VersionId, + }, s3err.GetAPIError(s3err.ErrNoSuchKey) + } + + return s3response.GetObjectAttributesResponse{}, err } - return s3response.GetObjectAttributesResult{ + return s3response.GetObjectAttributesResponse{ ETag: data.ETag, - LastModified: data.LastModified, ObjectSize: data.ContentLength, StorageClass: data.StorageClass, + LastModified: data.LastModified, + VersionId: data.VersionId, + DeleteMarker: data.DeleteMarker, }, nil } diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index 9f97611..9f0b0b1 100644 --- a/backend/s3proxy/s3.go +++ b/backend/s3proxy/s3.go @@ -392,7 +392,7 @@ func (s *S3Proxy) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3. return output, nil } -func (s *S3Proxy) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) { +func (s *S3Proxy) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) { out, err := s.client.GetObjectAttributes(ctx, input) parts := s3response.ObjectParts{} @@ -419,12 +419,11 @@ func (s *S3Proxy) GetObjectAttributes(ctx context.Context, input *s3.GetObjectAt } } - return s3response.GetObjectAttributesResult{ + return s3response.GetObjectAttributesResponse{ ETag: out.ETag, LastModified: out.LastModified, ObjectSize: out.ObjectSize, StorageClass: out.StorageClass, - VersionId: out.VersionId, ObjectParts: &parts, }, handleError(err) } diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index d9e76e8..06124fa 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -83,7 +83,7 @@ var _ backend.Backend = &BackendMock{} // GetObjectAclFunc: func(contextMoqParam context.Context, getObjectAclInput *s3.GetObjectAclInput) (*s3.GetObjectAclOutput, error) { // panic("mock out the GetObjectAcl method") // }, -// GetObjectAttributesFunc: func(contextMoqParam context.Context, getObjectAttributesInput *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) { +// GetObjectAttributesFunc: func(contextMoqParam context.Context, getObjectAttributesInput *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) { // panic("mock out the GetObjectAttributes method") // }, // GetObjectLegalHoldFunc: func(contextMoqParam context.Context, bucket string, object string, versionId string) (*bool, error) { @@ -244,7 +244,7 @@ type BackendMock struct { GetObjectAclFunc func(contextMoqParam context.Context, getObjectAclInput *s3.GetObjectAclInput) (*s3.GetObjectAclOutput, error) // GetObjectAttributesFunc mocks the GetObjectAttributes method. - GetObjectAttributesFunc func(contextMoqParam context.Context, getObjectAttributesInput *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) + GetObjectAttributesFunc func(contextMoqParam context.Context, getObjectAttributesInput *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) // GetObjectLegalHoldFunc mocks the GetObjectLegalHold method. GetObjectLegalHoldFunc func(contextMoqParam context.Context, bucket string, object string, versionId string) (*bool, error) @@ -1518,7 +1518,7 @@ func (mock *BackendMock) GetObjectAclCalls() []struct { } // GetObjectAttributes calls GetObjectAttributesFunc. -func (mock *BackendMock) GetObjectAttributes(contextMoqParam context.Context, getObjectAttributesInput *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) { +func (mock *BackendMock) GetObjectAttributes(contextMoqParam context.Context, getObjectAttributesInput *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) { if mock.GetObjectAttributesFunc == nil { panic("BackendMock.GetObjectAttributesFunc: method is nil but Backend.GetObjectAttributes was just called") } diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 46381a3..6f1207b 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -51,8 +51,9 @@ type S3ApiController struct { } const ( - iso8601Format = "20060102T150405Z" - defaultContentType = "binary/octet-stream" + iso8601Format = "20060102T150405Z" + iso8601TimeFormatExtended = "Mon Jan _2 15:04:05 2006" + defaultContentType = "binary/octet-stream" ) func New(be backend.Backend, iam auth.IAMService, logger s3log.AuditLogger, evs s3event.S3EventSender, mm *metrics.Manager, debug bool, readonly bool) S3ApiController { @@ -379,7 +380,19 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { BucketOwner: parsedAcl.Owner, }) } - attrs := utils.ParseObjectAttributes(ctx) + attrs, err := utils.ParseObjectAttributes(ctx) + if err != nil { + if c.debug { + log.Printf("error parsing object attributes: %v", err) + } + return SendXMLResponse(ctx, nil, err, + &MetaOpts{ + Logger: c.logger, + MetricsMng: c.mm, + Action: metrics.ActionGetObjectAttributes, + BucketOwner: parsedAcl.Owner, + }) + } res, err := c.be.GetObjectAttributes(ctx.Context(), &s3.GetObjectAttributesInput{ @@ -390,6 +403,22 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { VersionId: &versionId, }) if err != nil { + hdrs := []utils.CustomHeader{} + + if res.DeleteMarker != nil { + hdrs = append(hdrs, utils.CustomHeader{ + Key: "x-amz-delete-marker", + Value: "true", + }) + } + if getstring(res.VersionId) != "" { + hdrs = append(hdrs, utils.CustomHeader{ + Key: "x-amz-version-id", + Value: getstring(res.VersionId), + }) + } + + utils.SetResponseHeaders(ctx, hdrs) return SendXMLResponse(ctx, nil, err, &MetaOpts{ Logger: c.logger, @@ -398,7 +427,31 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { BucketOwner: parsedAcl.Owner, }) } - return SendXMLResponse(ctx, utils.FilterObjectAttributes(attrs, res), err, + + hdrs := []utils.CustomHeader{} + + if getstring(res.VersionId) != "" { + hdrs = append(hdrs, utils.CustomHeader{ + Key: "x-amz-version-id", + Value: getstring(res.VersionId), + }) + } + if res.DeleteMarker != nil && *res.DeleteMarker { + hdrs = append(hdrs, utils.CustomHeader{ + Key: "x-amz-delete-marker", + Value: "true", + }) + } + if res.LastModified != nil { + hdrs = append(hdrs, utils.CustomHeader{ + Key: "Last-Modified", + Value: res.LastModified.UTC().Format(iso8601TimeFormatExtended), + }) + } + + utils.SetResponseHeaders(ctx, hdrs) + + return SendXMLResponse(ctx, utils.FilterObjectAttributes(attrs, res), nil, &MetaOpts{ Logger: c.logger, MetricsMng: c.mm, @@ -2900,15 +2953,6 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error { BucketOwner: parsedAcl.Owner, }) } - if res == nil { - return SendResponse(ctx, fmt.Errorf("head object nil response"), - &MetaOpts{ - Logger: c.logger, - MetricsMng: c.mm, - Action: metrics.ActionHeadObject, - BucketOwner: parsedAcl.Owner, - }) - } utils.SetMetaHeaders(ctx, res.Metadata) headers := []utils.CustomHeader{ diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index 8a17d2d..a8881d4 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -187,8 +187,8 @@ func TestS3ApiController_GetActions(t *testing.T) { GetObjectAclFunc: func(context.Context, *s3.GetObjectAclInput) (*s3.GetObjectAclOutput, error) { return &s3.GetObjectAclOutput{}, nil }, - GetObjectAttributesFunc: func(context.Context, *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResult, error) { - return s3response.GetObjectAttributesResult{}, nil + GetObjectAttributesFunc: func(context.Context, *s3.GetObjectAttributesInput) (s3response.GetObjectAttributesResponse, error) { + return s3response.GetObjectAttributesResponse{}, nil }, GetObjectFunc: func(context.Context, *s3.GetObjectInput) (*s3.GetObjectOutput, error) { return &s3.GetObjectOutput{ diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index 1a6a5f4..6302ba8 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -254,35 +254,47 @@ func ParseDeleteObjects(objs []types.ObjectIdentifier) (result []string) { return } -func FilterObjectAttributes(attrs map[types.ObjectAttributes]struct{}, output s3response.GetObjectAttributesResult) s3response.GetObjectAttributesResult { - if _, ok := attrs[types.ObjectAttributesEtag]; !ok { +func FilterObjectAttributes(attrs map[s3response.ObjectAttributes]struct{}, output s3response.GetObjectAttributesResponse) s3response.GetObjectAttributesResponse { + // These properties shouldn't appear in the final response body + output.LastModified = nil + output.VersionId = nil + output.DeleteMarker = nil + + if _, ok := attrs[s3response.ObjectAttributesEtag]; !ok { output.ETag = nil } - if _, ok := attrs[types.ObjectAttributesObjectParts]; !ok { + if _, ok := attrs[s3response.ObjectAttributesObjectParts]; !ok { output.ObjectParts = nil } - if _, ok := attrs[types.ObjectAttributesObjectSize]; !ok { + if _, ok := attrs[s3response.ObjectAttributesObjectSize]; !ok { output.ObjectSize = nil } - if _, ok := attrs[types.ObjectAttributesStorageClass]; !ok { + if _, ok := attrs[s3response.ObjectAttributesStorageClass]; !ok { output.StorageClass = "" } + fmt.Printf("%+v\n", output) return output } -func ParseObjectAttributes(ctx *fiber.Ctx) map[types.ObjectAttributes]struct{} { - attrs := map[types.ObjectAttributes]struct{}{} +func ParseObjectAttributes(ctx *fiber.Ctx) (map[s3response.ObjectAttributes]struct{}, error) { + attrs := map[s3response.ObjectAttributes]struct{}{} + var err error ctx.Request().Header.VisitAll(func(key, value []byte) { if string(key) == "X-Amz-Object-Attributes" { oattrs := strings.Split(string(value), ",") for _, a := range oattrs { - attrs[types.ObjectAttributes(a)] = struct{}{} + attr := s3response.ObjectAttributes(a) + if !attr.IsValid() { + err = s3err.GetAPIError(s3err.ErrInvalidObjectAttributes) + break + } + attrs[attr] = struct{}{} } } }) - return attrs + return attrs, err } type objLockCfg struct { diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index 47c931b..1f76759 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -19,10 +19,12 @@ import ( "net/http" "reflect" "testing" + "time" "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/gofiber/fiber/v2" "github.com/valyala/fasthttp" + "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3response" ) @@ -283,47 +285,68 @@ func TestParseUint(t *testing.T) { func TestFilterObjectAttributes(t *testing.T) { type args struct { - attrs map[types.ObjectAttributes]struct{} - output s3response.GetObjectAttributesResult + attrs map[s3response.ObjectAttributes]struct{} + output s3response.GetObjectAttributesResponse } + etag, objSize := "etag", int64(3222) + delMarker := true + tests := []struct { name string args args - want s3response.GetObjectAttributesResult + want s3response.GetObjectAttributesResponse }{ { name: "keep only ETag", args: args{ - attrs: map[types.ObjectAttributes]struct{}{ - types.ObjectAttributesEtag: {}, + attrs: map[s3response.ObjectAttributes]struct{}{ + s3response.ObjectAttributesEtag: {}, }, - output: s3response.GetObjectAttributesResult{ + output: s3response.GetObjectAttributesResponse{ ObjectSize: &objSize, ETag: &etag, }, }, - want: s3response.GetObjectAttributesResult{ETag: &etag}, + want: s3response.GetObjectAttributesResponse{ETag: &etag}, }, { name: "keep multiple props", args: args{ - attrs: map[types.ObjectAttributes]struct{}{ - types.ObjectAttributesEtag: {}, - types.ObjectAttributesObjectSize: {}, - types.ObjectAttributesStorageClass: {}, + attrs: map[s3response.ObjectAttributes]struct{}{ + s3response.ObjectAttributesEtag: {}, + s3response.ObjectAttributesObjectSize: {}, + s3response.ObjectAttributesStorageClass: {}, }, - output: s3response.GetObjectAttributesResult{ + output: s3response.GetObjectAttributesResponse{ ObjectSize: &objSize, ETag: &etag, ObjectParts: &s3response.ObjectParts{}, VersionId: &etag, }, }, - want: s3response.GetObjectAttributesResult{ + want: s3response.GetObjectAttributesResponse{ ETag: &etag, ObjectSize: &objSize, - VersionId: &etag, + }, + }, + { + name: "make sure LastModified, DeleteMarker and VersionId are removed", + args: args{ + attrs: map[s3response.ObjectAttributes]struct{}{ + s3response.ObjectAttributesEtag: {}, + }, + output: s3response.GetObjectAttributesResponse{ + ObjectSize: &objSize, + ETag: &etag, + ObjectParts: &s3response.ObjectParts{}, + VersionId: &etag, + LastModified: backend.GetTimePtr(time.Now()), + DeleteMarker: &delMarker, + }, + }, + want: s3response.GetObjectAttributesResponse{ + ETag: &etag, }, }, } diff --git a/s3err/s3err.go b/s3err/s3err.go index 8bea6b4..0a885be 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -70,6 +70,7 @@ const ( ErrInvalidMaxUploads ErrInvalidMaxParts ErrInvalidPartNumberMarker + ErrInvalidObjectAttributes ErrInvalidPart ErrInvalidPartNumber ErrInternalError @@ -219,6 +220,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "Argument partNumberMarker must be an integer.", HTTPStatusCode: http.StatusBadRequest, }, + ErrInvalidObjectAttributes: { + Code: "InvalidArgument", + Description: "Invalid attribute name specified.", + HTTPStatusCode: http.StatusBadRequest, + }, ErrNoSuchBucket: { Code: "NoSuchBucket", Description: "The specified bucket does not exist.", diff --git a/s3response/s3response.go b/s3response/s3response.go index 4880ae8..310d3ef 100644 --- a/s3response/s3response.go +++ b/s3response/s3response.go @@ -78,30 +78,34 @@ type ListPartsResult struct { Parts []Part `xml:"Part"` } -type GetObjectAttributesResult struct { - ETag *string - LastModified *time.Time - ObjectSize *int64 - StorageClass types.StorageClass - VersionId *string - ObjectParts *ObjectParts +type ObjectAttributes string + +const ( + ObjectAttributesEtag ObjectAttributes = "ETag" + ObjectAttributesChecksum ObjectAttributes = "Checksum" + ObjectAttributesObjectParts ObjectAttributes = "ObjectParts" + ObjectAttributesStorageClass ObjectAttributes = "StorageClass" + ObjectAttributesObjectSize ObjectAttributes = "ObjectSize" +) + +func (o ObjectAttributes) IsValid() bool { + return o == ObjectAttributesChecksum || + o == ObjectAttributesEtag || + o == ObjectAttributesObjectParts || + o == ObjectAttributesObjectSize || + o == ObjectAttributesStorageClass } -func (r GetObjectAttributesResult) MarshalXML(e *xml.Encoder, start xml.StartElement) error { - type Alias GetObjectAttributesResult - aux := &struct { - LastModified *string `xml:"LastModified"` - *Alias - }{ - Alias: (*Alias)(&r), - } +type GetObjectAttributesResponse struct { + ETag *string + ObjectSize *int64 + StorageClass types.StorageClass `xml:",omitempty"` + ObjectParts *ObjectParts - if r.LastModified != nil { - formattedTime := r.LastModified.UTC().Format(iso8601TimeFormat) - aux.LastModified = &formattedTime - } - - return e.EncodeElement(aux, start) + // Not included in the response body + VersionId *string + LastModified *time.Time + DeleteMarker *bool } type ObjectParts struct { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 5331567..47ede0c 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -157,6 +157,7 @@ func TestHeadObject(s *S3Conf) { func TestGetObjectAttributes(s *S3Conf) { GetObjectAttributes_non_existing_bucket(s) GetObjectAttributes_non_existing_object(s) + GetObjectAttributes_invalid_attrs(s) GetObjectAttributes_existing_object(s) } @@ -565,6 +566,7 @@ func TestVersioning(s *S3Conf) { // HeadObject action Versioning_HeadObject_invalid_versionId(s) Versioning_HeadObject_success(s) + Versioning_HeadObject_without_versionId(s) Versioning_HeadObject_delete_marker(s) // GetObject action Versioning_GetObject_invalid_versionId(s) @@ -572,6 +574,9 @@ func TestVersioning(s *S3Conf) { Versioning_GetObject_delete_marker_without_versionId(s) Versioning_GetObject_delete_marker(s) Versioning_GetObject_null_versionId_obj(s) + // GetObjectAttributes action + Versioning_GetObjectAttributes_object_version(s) + Versioning_GetObjectAttributes_delete_marker(s) // DeleteObject(s) actions Versioning_DeleteObject_delete_object_version(s) Versioning_DeleteObject_non_existing_object(s) @@ -720,6 +725,7 @@ func GetIntTests() IntTests { "HeadObject_success": HeadObject_success, "GetObjectAttributes_non_existing_bucket": GetObjectAttributes_non_existing_bucket, "GetObjectAttributes_non_existing_object": GetObjectAttributes_non_existing_object, + "GetObjectAttributes_invalid_attrs": GetObjectAttributes_invalid_attrs, "GetObjectAttributes_existing_object": GetObjectAttributes_existing_object, "GetObject_non_existing_key": GetObject_non_existing_key, "GetObject_directory_object_noslash": GetObject_directory_object_noslash, @@ -960,12 +966,15 @@ func GetIntTests() IntTests { "Versioning_CopyObject_special_chars": Versioning_CopyObject_special_chars, "Versioning_HeadObject_invalid_versionId": Versioning_HeadObject_invalid_versionId, "Versioning_HeadObject_success": Versioning_HeadObject_success, + "Versioning_HeadObject_without_versionId": Versioning_HeadObject_without_versionId, "Versioning_HeadObject_delete_marker": Versioning_HeadObject_delete_marker, "Versioning_GetObject_invalid_versionId": Versioning_GetObject_invalid_versionId, "Versioning_GetObject_success": Versioning_GetObject_success, "Versioning_GetObject_delete_marker_without_versionId": Versioning_GetObject_delete_marker_without_versionId, "Versioning_GetObject_delete_marker": Versioning_GetObject_delete_marker, "Versioning_GetObject_null_versionId_obj": Versioning_GetObject_null_versionId_obj, + "Versioning_GetObjectAttributes_object_version": Versioning_GetObjectAttributes_object_version, + "Versioning_GetObjectAttributes_delete_marker": Versioning_GetObjectAttributes_delete_marker, "Versioning_DeleteObject_delete_object_version": Versioning_DeleteObject_delete_object_version, "Versioning_DeleteObject_non_existing_object": Versioning_DeleteObject_non_existing_object, "Versioning_DeleteObject_delete_a_delete_marker": Versioning_DeleteObject_delete_a_delete_marker, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index c8413ad..8ad971b 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -3375,9 +3375,11 @@ func GetObjectAttributes_non_existing_object(s *S3Conf) error { return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.GetObjectAttributes(ctx, &s3.GetObjectAttributesInput{ - Bucket: &bucket, - Key: getPtr("my-obj"), - ObjectAttributes: []types.ObjectAttributes{}, + Bucket: &bucket, + Key: getPtr("my-obj"), + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesEtag, + }, }) cancel() if err := checkSdkApiErr(err, "NoSuchKey"); err != nil { @@ -3388,6 +3390,33 @@ func GetObjectAttributes_non_existing_object(s *S3Conf) error { }) } +func GetObjectAttributes_invalid_attrs(s *S3Conf) error { + testName := "GetObjectAttributes_invalid_attrs" + 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 + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObjectAttributes(ctx, &s3.GetObjectAttributesInput{ + Bucket: &bucket, + Key: &obj, + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesEtag, + types.ObjectAttributes("Invalid_argument"), + }, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidObjectAttributes)); err != nil { + return err + } + + return nil + }) +} + func GetObjectAttributes_existing_object(s *S3Conf) error { testName := "GetObjectAttributes_existing_object" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -3438,11 +3467,14 @@ func GetObjectAttributes_existing_object(s *S3Conf) error { return fmt.Errorf("expected object size to be %v, instead got %v", data_len, *out.ObjectSize) } if out.Checksum != nil { - return fmt.Errorf("expected checksum do be nil, instead got %v", *out.Checksum) + return fmt.Errorf("expected checksum to be nil, instead got %v", *out.Checksum) } if out.StorageClass != types.StorageClassStandard { return fmt.Errorf("expected the storage class to be %v, instead got %v", types.StorageClassStandard, out.StorageClass) } + if out.LastModified == nil { + return fmt.Errorf("expected non nil LastModified") + } return nil }) @@ -10848,19 +10880,6 @@ func PutObject_name_too_long(s *S3Conf) error { }) } -// Versioning tests -func PutBucketVersioning_non_existing_bucket(s *S3Conf) error { - testName := "PutBucketVersioning_non_existing_bucket" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - err := putBucketVersioningStatus(s3client, getBucketName(), types.BucketVersioningStatusSuspended) - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)); err != nil { - return err - } - - return nil - }) -} - func HeadObject_name_too_long(s *S3Conf) error { testName := "HeadObject_name_too_long" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -10878,6 +10897,35 @@ func HeadObject_name_too_long(s *S3Conf) error { }) } +func DeleteObject_name_too_long(s *S3Conf) error { + testName := "DeleteObject_name_too_long" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: getPtr(genRandString(300)), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrKeyTooLong)); err != nil { + return err + } + return nil + }) +} + +// Versioning tests +func PutBucketVersioning_non_existing_bucket(s *S3Conf) error { + testName := "PutBucketVersioning_non_existing_bucket" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + err := putBucketVersioningStatus(s3client, getBucketName(), types.BucketVersioningStatusSuspended) + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)); err != nil { + return err + } + + return nil + }) +} + func PutBucketVersioning_invalid_status(s *S3Conf) error { testName := "PutBucketVersioning_invalid_status" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -11405,22 +11453,6 @@ func Versioning_HeadObject_invalid_versionId(s *S3Conf) error { }) } -func DeleteObject_name_too_long(s *S3Conf) error { - testName := "DeleteObject_name_too_long" - return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ - Bucket: &bucket, - Key: getPtr(genRandString(300)), - }) - cancel() - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrKeyTooLong)); err != nil { - return err - } - return nil - }) -} - func Versioning_HeadObject_success(s *S3Conf) error { testName := "Versioning_HeadObject_success" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -11456,6 +11488,35 @@ func Versioning_HeadObject_success(s *S3Conf) error { }, withVersioning(types.BucketVersioningStatusEnabled)) } +func Versioning_HeadObject_without_versionId(s *S3Conf) error { + testName := "Versioning_HeadObject_without_versionId" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + versions, err := createObjVersions(s3client, bucket, obj, 3) + if err != nil { + return err + } + + lastVersion := versions[0] + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + if err != nil { + return err + } + + if getString(res.VersionId) != *lastVersion.VersionId { + return fmt.Errorf("expected versionId to be %v, instead got %v", *lastVersion.VersionId, getString(res.VersionId)) + } + + return nil + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + func Versioning_HeadObject_delete_marker(s *S3Conf) error { testName := "Versioning_HeadObject_delete_marker" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -11727,6 +11788,97 @@ func Versioning_GetObject_null_versionId_obj(s *S3Conf) error { }) } +func Versioning_GetObjectAttributes_object_version(s *S3Conf) error { + testName := "Versioning_GetObjectAttributes_object_version" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + versions, err := createObjVersions(s3client, bucket, obj, 1) + if err != nil { + return err + } + version := versions[0] + + getObjAttrs := func(versionId *string) (*s3.GetObjectAttributesOutput, error) { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.GetObjectAttributes(ctx, &s3.GetObjectAttributesInput{ + Bucket: &bucket, + Key: &obj, + VersionId: versionId, + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesEtag, + }, + }) + cancel() + return res, err + } + + // By specifying the versionId + res, err := getObjAttrs(version.VersionId) + if err != nil { + return err + } + + if getString(res.ETag) != *version.ETag { + return fmt.Errorf("expected the uploaded object ETag to be %v, instead got %v", *version.ETag, getString(res.ETag)) + } + if getString(res.VersionId) != *version.VersionId { + return fmt.Errorf("expected the uploaded versionId to be %v, instead got %v", *version.VersionId, getString(res.VersionId)) + } + + // Without versionId + res, err = getObjAttrs(nil) + if err != nil { + return err + } + + if getString(res.ETag) != *version.ETag { + return fmt.Errorf("expected the uploaded object ETag to be %v, instead got %v", *version.ETag, getString(res.ETag)) + } + if getString(res.VersionId) != *version.VersionId { + return fmt.Errorf("expected the uploaded object versionId to be %v, instead got %v", *version.VersionId, getString(res.VersionId)) + } + + return nil + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + +func Versioning_GetObjectAttributes_delete_marker(s *S3Conf) error { + testName := "Versioning_GetObjectAttributes_delete_marker" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + _, err := createObjVersions(s3client, bucket, obj, 1) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObjectAttributes(ctx, &s3.GetObjectAttributesInput{ + Bucket: &bucket, + Key: &obj, + VersionId: res.VersionId, + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesEtag, + }, + }) + cancel() + if err := checkSdkApiErr(err, "NoSuchKey"); err != nil { + return err + } + + return nil + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + func Versioning_DeleteObject_delete_object_version(s *S3Conf) error { testName := "Versioning_DeleteObject_delete_object_version" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {