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

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")
 	}

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{

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{

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 {

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,
 			},
 		},
 	}