From 50357ce61ad90bda98ddadfcb18d3d34a3fe0841 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Fri, 9 Jun 2023 10:35:21 -0700 Subject: [PATCH 1/6] feat: add log for internal server errors not of type s3err.APIError --- s3api/controllers/base.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 862478a2..071fa656 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "net/http" + "os" "strconv" "strings" "time" @@ -396,6 +397,10 @@ func Responce[R comparable](ctx *fiber.Ctx, resp R, err error) error { ctx.Status(serr.HTTPStatusCode) return ctx.Send(s3err.GetAPIErrorResponse(serr, "", "", "")) } + + fmt.Fprintf(os.Stderr, "Internal Error, req:\n%v\nerr:\n%v\n", + ctx.Request(), err) + return ctx.Send(s3err.GetAPIErrorResponse( s3err.GetAPIError(s3err.ErrInternalError), "", "", "")) } From 2697edd40ab46606e7ef2490f4c69450038ac59e Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Fri, 9 Jun 2023 18:53:00 -0700 Subject: [PATCH 2/6] head object time format --- s3api/controllers/base.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 071fa656..bea3b767 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -313,6 +313,10 @@ func (c S3ApiController) HeadBucket(ctx *fiber.Ctx) error { return Responce(ctx, res, err) } +const ( + timefmt = "Mon, 02 Jan 2006 15:04:05 GMT" +) + func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error { bucket := ctx.Params("bucket") key := ctx.Params("key") @@ -346,7 +350,7 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error { }, { Key: "Last-Modified", - Value: res.LastModified.Format("20060102T150405Z"), + Value: res.LastModified.Format(timefmt), }, }) From ef06d11d7c755478fcf9b22c6ea1eec8a653ada6 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Tue, 6 Jun 2023 13:36:21 -0700 Subject: [PATCH 3/6] fix: get simple multipart upload tests passing --- backend/backend.go | 13 +-- backend/backend_moq_test.go | 13 +-- backend/common.go | 5 + backend/posix/posix.go | 139 ++++++++++++++++---------- backend/posix/posix_darwin.go | 2 +- backend/posix/posix_linux.go | 2 +- cmd/versitygw/main.go | 1 + s3api/controllers/backend_moq_test.go | 13 +-- s3api/controllers/base.go | 82 ++++++--------- s3api/controllers/base_test.go | 27 ++--- s3response/s3response.go | 96 ++++++++++++++++++ 11 files changed, 257 insertions(+), 136 deletions(-) create mode 100644 s3response/s3response.go diff --git a/backend/backend.go b/backend/backend.go index caa6bac6..12e31d4b 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -21,6 +21,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" + "github.com/versity/versitygw/s3response" ) //go:generate moq -out backend_moq_test.go . Backend @@ -39,8 +40,8 @@ type Backend interface { CreateMultipartUpload(*s3.CreateMultipartUploadInput) (*s3.CreateMultipartUploadOutput, error) CompleteMultipartUpload(bucket, object, uploadID string, parts []types.Part) (*s3.CompleteMultipartUploadOutput, error) AbortMultipartUpload(*s3.AbortMultipartUploadInput) error - ListMultipartUploads(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) - ListObjectParts(bucket, object, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) + ListMultipartUploads(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) + ListObjectParts(bucket, object, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) CopyPart(srcBucket, srcObject, DstBucket, uploadID, rangeHeader string, part int) (*types.CopyPartResult, error) PutObjectPart(bucket, object, uploadID string, part int, length int64, r io.Reader) (etag string, err error) @@ -115,11 +116,11 @@ func (BackendUnsupported) CompleteMultipartUpload(bucket, object, uploadID strin func (BackendUnsupported) AbortMultipartUpload(input *s3.AbortMultipartUploadInput) error { return s3err.GetAPIError(s3err.ErrNotImplemented) } -func (BackendUnsupported) ListMultipartUploads(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) { - return nil, s3err.GetAPIError(s3err.ErrNotImplemented) +func (BackendUnsupported) ListMultipartUploads(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) { + return s3response.ListMultipartUploadsResponse{}, s3err.GetAPIError(s3err.ErrNotImplemented) } -func (BackendUnsupported) ListObjectParts(bucket, object, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) { - return nil, s3err.GetAPIError(s3err.ErrNotImplemented) +func (BackendUnsupported) ListObjectParts(bucket, object, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) { + return s3response.ListPartsResponse{}, s3err.GetAPIError(s3err.ErrNotImplemented) } func (BackendUnsupported) CopyPart(srcBucket, srcObject, DstBucket, uploadID, rangeHeader string, part int) (*types.CopyPartResult, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) diff --git a/backend/backend_moq_test.go b/backend/backend_moq_test.go index d3f2e4d9..6ecfc5b2 100644 --- a/backend/backend_moq_test.go +++ b/backend/backend_moq_test.go @@ -6,6 +6,7 @@ package backend import ( "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/versity/versitygw/s3response" "io" "sync" ) @@ -68,10 +69,10 @@ var _ Backend = &BackendMock{} // ListBucketsFunc: func() (*s3.ListBucketsOutput, error) { // panic("mock out the ListBuckets method") // }, -// ListMultipartUploadsFunc: func(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) { +// ListMultipartUploadsFunc: func(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) { // panic("mock out the ListMultipartUploads method") // }, -// ListObjectPartsFunc: func(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) { +// ListObjectPartsFunc: func(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) { // panic("mock out the ListObjectParts method") // }, // ListObjectsFunc: func(bucket string, prefix string, marker string, delim string, maxkeys int) (*s3.ListObjectsOutput, error) { @@ -172,10 +173,10 @@ type BackendMock struct { ListBucketsFunc func() (*s3.ListBucketsOutput, error) // ListMultipartUploadsFunc mocks the ListMultipartUploads method. - ListMultipartUploadsFunc func(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) + ListMultipartUploadsFunc func(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) // ListObjectPartsFunc mocks the ListObjectParts method. - ListObjectPartsFunc func(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) + ListObjectPartsFunc func(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) // ListObjectsFunc mocks the ListObjects method. ListObjectsFunc func(bucket string, prefix string, marker string, delim string, maxkeys int) (*s3.ListObjectsOutput, error) @@ -1094,7 +1095,7 @@ func (mock *BackendMock) ListBucketsCalls() []struct { } // ListMultipartUploads calls ListMultipartUploadsFunc. -func (mock *BackendMock) ListMultipartUploads(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) { +func (mock *BackendMock) ListMultipartUploads(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) { if mock.ListMultipartUploadsFunc == nil { panic("BackendMock.ListMultipartUploadsFunc: method is nil but Backend.ListMultipartUploads was just called") } @@ -1126,7 +1127,7 @@ func (mock *BackendMock) ListMultipartUploadsCalls() []struct { } // ListObjectParts calls ListObjectPartsFunc. -func (mock *BackendMock) ListObjectParts(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) { +func (mock *BackendMock) ListObjectParts(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) { if mock.ListObjectPartsFunc == nil { panic("BackendMock.ListObjectPartsFunc: method is nil but Backend.ListObjectParts was just called") } diff --git a/backend/common.go b/backend/common.go index af18ad4a..67772077 100644 --- a/backend/common.go +++ b/backend/common.go @@ -24,6 +24,11 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3/types" ) +var ( + // RFC3339TimeFormat RFC3339 time format + RFC3339TimeFormat = "2006-01-02T15:04:05.999Z" +) + func IsValidBucketName(name string) bool { return true } type ByBucketName []types.Bucket diff --git a/backend/posix/posix.go b/backend/posix/posix.go index a5994ec9..68cd269f 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -36,6 +36,7 @@ import ( "github.com/pkg/xattr" "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3err" + "github.com/versity/versitygw/s3response" ) type Posix struct { @@ -231,8 +232,10 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [ // check all parts ok last := len(parts) - 1 partsize := int64(0) + var totalsize int64 for i, p := range parts { - fi, err := os.Lstat(filepath.Join(objdir, uploadID, fmt.Sprintf("%v", p.PartNumber))) + partPath := filepath.Join(objdir, uploadID, fmt.Sprintf("%v", p.PartNumber)) + fi, err := os.Lstat(partPath) if err != nil { return nil, s3err.GetAPIError(s3err.ErrInvalidPart) } @@ -240,13 +243,21 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [ if i == 0 { partsize = fi.Size() } + totalsize += fi.Size() // all parts except the last need to be the same size if i < last && partsize != fi.Size() { return nil, s3err.GetAPIError(s3err.ErrInvalidPart) } + + b, err := xattr.Get(partPath, "user.etag") + etag := string(b) + if err != nil { + etag = "" + } + parts[i].ETag = &etag } - f, err := openTmpFile(filepath.Join(bucket, metaTmpDir), bucket, object, 0) + f, err := openTmpFile(filepath.Join(bucket, metaTmpDir), bucket, object, totalsize) if err != nil { return nil, fmt.Errorf("open temp file: %w", err) } @@ -272,11 +283,8 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [ dir := filepath.Dir(objname) if dir != "" { if err = mkdirAll(dir, os.FileMode(0755), bucket, object); err != nil { - if err != nil && os.IsExist(err) { - return nil, s3err.GetAPIError(s3err.ErrObjectParentIsFile) - } if err != nil { - return nil, fmt.Errorf("make object parent directories: %w", err) + return nil, s3err.GetAPIError(s3err.ErrExistingObjectIsDirectory) } } } @@ -479,24 +487,40 @@ func (p *Posix) AbortMultipartUpload(mpu *s3.AbortMultipartUploadInput) error { return nil } -func (p *Posix) ListMultipartUploads(mpu *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) { +func (p *Posix) ListMultipartUploads(mpu *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) { bucket := *mpu.Bucket + var delimiter string + if mpu.Delimiter != nil { + delimiter = *mpu.Delimiter + } + var prefix string + if mpu.Prefix != nil { + prefix = *mpu.Prefix + } + + var lmu s3response.ListMultipartUploadsResponse _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) + return lmu, s3err.GetAPIError(s3err.ErrNoSuchBucket) } if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return lmu, fmt.Errorf("stat bucket: %w", err) } // ignore readdir error and use the empty list returned objs, _ := os.ReadDir(filepath.Join(bucket, metaTmpMultipartDir)) - var uploads []types.MultipartUpload + var uploads []s3response.Upload - keyMarker := *mpu.KeyMarker - uploadIDMarker := *mpu.UploadIdMarker + var keyMarker string + if mpu.KeyMarker != nil { + keyMarker = *mpu.KeyMarker + } + var uploadIDMarker string + if mpu.UploadIdMarker != nil { + uploadIDMarker = *mpu.UploadIdMarker + } var pastMarker bool if keyMarker == "" && uploadIDMarker == "" { pastMarker = true @@ -512,7 +536,7 @@ func (p *Posix) ListMultipartUploads(mpu *s3.ListMultipartUploadsInput) (*s3.Lis continue } objectName := string(b) - if !strings.HasPrefix(objectName, *mpu.Prefix) { + if mpu.Prefix != nil && !strings.HasPrefix(objectName, *mpu.Prefix) { continue } @@ -538,64 +562,71 @@ func (p *Posix) ListMultipartUploads(mpu *s3.ListMultipartUploadsInput) (*s3.Lis upiddir := filepath.Join(bucket, metaTmpMultipartDir, obj.Name(), upid.Name()) loadUserMetaData(upiddir, userMetaData) + fi, err := upid.Info() + if err != nil { + return lmu, fmt.Errorf("stat %q: %w", upid.Name(), err) + } + uploadID := upid.Name() - uploads = append(uploads, types.MultipartUpload{ - Key: &objectName, - UploadId: &uploadID, + uploads = append(uploads, s3response.Upload{ + Key: objectName, + UploadID: uploadID, + Initiated: fi.ModTime().Format(backend.RFC3339TimeFormat), }) if len(uploads) == int(mpu.MaxUploads) { - return &s3.ListMultipartUploadsOutput{ - Bucket: &bucket, - Delimiter: mpu.Delimiter, + return s3response.ListMultipartUploadsResponse{ + Bucket: bucket, + Delimiter: delimiter, IsTruncated: i != len(objs) || j != len(upids), - KeyMarker: &keyMarker, - MaxUploads: mpu.MaxUploads, - NextKeyMarker: &objectName, - NextUploadIdMarker: &uploadID, - Prefix: mpu.Prefix, - UploadIdMarker: mpu.UploadIdMarker, + KeyMarker: keyMarker, + MaxUploads: int(mpu.MaxUploads), + NextKeyMarker: objectName, + NextUploadIDMarker: uploadID, + Prefix: prefix, + UploadIDMarker: uploadIDMarker, Uploads: uploads, }, nil } } } - return &s3.ListMultipartUploadsOutput{ - Bucket: &bucket, - Delimiter: mpu.Delimiter, - KeyMarker: &keyMarker, - MaxUploads: mpu.MaxUploads, - Prefix: mpu.Prefix, - UploadIdMarker: mpu.UploadIdMarker, + return s3response.ListMultipartUploadsResponse{ + Bucket: bucket, + Delimiter: delimiter, + KeyMarker: keyMarker, + MaxUploads: int(mpu.MaxUploads), + Prefix: prefix, + UploadIDMarker: uploadIDMarker, Uploads: uploads, }, nil } -func (p *Posix) ListObjectParts(bucket, object, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) { +func (p *Posix) ListObjectParts(bucket, object, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) { + var lpr s3response.ListPartsResponse _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) + return lpr, s3err.GetAPIError(s3err.ErrNoSuchBucket) } if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return lpr, fmt.Errorf("stat bucket: %w", err) } sum, err := p.checkUploadIDExists(bucket, object, uploadID) if err != nil { - return nil, err + return lpr, err } objdir := filepath.Join(bucket, metaTmpMultipartDir, fmt.Sprintf("%x", sum)) ents, err := os.ReadDir(filepath.Join(objdir, uploadID)) if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetAPIError(s3err.ErrNoSuchUpload) + return lpr, s3err.GetAPIError(s3err.ErrNoSuchUpload) } if err != nil { - return nil, fmt.Errorf("readdir upload: %w", err) + return lpr, fmt.Errorf("readdir upload: %w", err) } - var parts []types.Part + var parts []s3response.Part for _, e := range ents { pn, _ := strconv.Atoi(e.Name()) if pn <= partNumberMarker { @@ -614,10 +645,10 @@ func (p *Posix) ListObjectParts(bucket, object, uploadID string, partNumberMarke continue } - parts = append(parts, types.Part{ - PartNumber: int32(pn), - ETag: &etag, - LastModified: backend.GetTimePtr(fi.ModTime()), + parts = append(parts, s3response.Part{ + PartNumber: pn, + ETag: etag, + LastModified: fi.ModTime().Format(backend.RFC3339TimeFormat), Size: fi.Size(), }) } @@ -626,12 +657,12 @@ func (p *Posix) ListObjectParts(bucket, object, uploadID string, partNumberMarke func(i int, j int) bool { return parts[i].PartNumber < parts[j].PartNumber }) oldLen := len(parts) - if len(parts) > maxParts { + if maxParts > 0 && len(parts) > maxParts { parts = parts[:maxParts] } newLen := len(parts) - nextpart := int32(0) + nextpart := 0 if len(parts) != 0 { nextpart = parts[len(parts)-1].PartNumber } @@ -640,15 +671,15 @@ func (p *Posix) ListObjectParts(bucket, object, uploadID string, partNumberMarke upiddir := filepath.Join(objdir, uploadID) loadUserMetaData(upiddir, userMetaData) - return &s3.ListPartsOutput{ - Bucket: &bucket, + return s3response.ListPartsResponse{ + Bucket: bucket, IsTruncated: oldLen != newLen, - Key: &object, - MaxParts: int32(maxParts), - NextPartNumberMarker: backend.GetStringPtr(fmt.Sprintf("%v", nextpart)), - PartNumberMarker: backend.GetStringPtr(fmt.Sprintf("%v", partNumberMarker)), + Key: object, + MaxParts: maxParts, + NextPartNumberMarker: nextpart, + PartNumberMarker: partNumberMarker, Parts: parts, - UploadId: &uploadID, + UploadID: uploadID, }, nil } @@ -689,7 +720,7 @@ func (p *Posix) PutObjectPart(bucket, object, uploadID string, part int, length } dataSum := hash.Sum(nil) - etag := hex.EncodeToString(dataSum[:]) + etag := hex.EncodeToString(dataSum) xattr.Set(partPath, "user.etag", []byte(etag)) return etag, nil @@ -741,7 +772,7 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) { if dir != "" { err = mkdirAll(dir, os.FileMode(0755), *po.Bucket, *po.Key) if err != nil { - return "", fmt.Errorf("make object parent directories: %w", err) + return "", s3err.GetAPIError(s3err.ErrExistingObjectIsDirectory) } } diff --git a/backend/posix/posix_darwin.go b/backend/posix/posix_darwin.go index e67f93b8..819d78f4 100644 --- a/backend/posix/posix_darwin.go +++ b/backend/posix/posix_darwin.go @@ -76,7 +76,7 @@ func (tmp *tmpfile) link() error { func (tmp *tmpfile) Write(b []byte) (int, error) { if int64(len(b)) > tmp.size { - return 0, fmt.Errorf("write exceeds content length") + return 0, fmt.Errorf("write exceeds content length %v", tmp.size) } n, err := tmp.f.Write(b) diff --git a/backend/posix/posix_linux.go b/backend/posix/posix_linux.go index 9ce49d3b..abc41ce2 100644 --- a/backend/posix/posix_linux.go +++ b/backend/posix/posix_linux.go @@ -150,7 +150,7 @@ func (tmp *tmpfile) fallbackLink() error { func (tmp *tmpfile) Write(b []byte) (int, error) { if int64(len(b)) > tmp.size { - return 0, fmt.Errorf("write exceeds content length") + return 0, fmt.Errorf("write exceeds content length %v", tmp.size) } n, err := tmp.f.Write(b) diff --git a/cmd/versitygw/main.go b/cmd/versitygw/main.go index 95e28144..ba6a286a 100644 --- a/cmd/versitygw/main.go +++ b/cmd/versitygw/main.go @@ -136,6 +136,7 @@ func runGateway(be backend.Backend) error { app := fiber.New(fiber.Config{ AppName: "versitygw", ServerHeader: "VERSITYGW", + BodyLimit: 5 * 1024 * 1024 * 1024, }) var opts []s3api.Option diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index 44df7796..4e6b425c 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -7,6 +7,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/backend" + "github.com/versity/versitygw/s3response" "io" "sync" ) @@ -69,10 +70,10 @@ var _ backend.Backend = &BackendMock{} // ListBucketsFunc: func() (*s3.ListBucketsOutput, error) { // panic("mock out the ListBuckets method") // }, -// ListMultipartUploadsFunc: func(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) { +// ListMultipartUploadsFunc: func(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) { // panic("mock out the ListMultipartUploads method") // }, -// ListObjectPartsFunc: func(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) { +// ListObjectPartsFunc: func(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) { // panic("mock out the ListObjectParts method") // }, // ListObjectsFunc: func(bucket string, prefix string, marker string, delim string, maxkeys int) (*s3.ListObjectsOutput, error) { @@ -173,10 +174,10 @@ type BackendMock struct { ListBucketsFunc func() (*s3.ListBucketsOutput, error) // ListMultipartUploadsFunc mocks the ListMultipartUploads method. - ListMultipartUploadsFunc func(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) + ListMultipartUploadsFunc func(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) // ListObjectPartsFunc mocks the ListObjectParts method. - ListObjectPartsFunc func(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) + ListObjectPartsFunc func(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) // ListObjectsFunc mocks the ListObjects method. ListObjectsFunc func(bucket string, prefix string, marker string, delim string, maxkeys int) (*s3.ListObjectsOutput, error) @@ -1095,7 +1096,7 @@ func (mock *BackendMock) ListBucketsCalls() []struct { } // ListMultipartUploads calls ListMultipartUploadsFunc. -func (mock *BackendMock) ListMultipartUploads(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) { +func (mock *BackendMock) ListMultipartUploads(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) { if mock.ListMultipartUploadsFunc == nil { panic("BackendMock.ListMultipartUploadsFunc: method is nil but Backend.ListMultipartUploads was just called") } @@ -1127,7 +1128,7 @@ func (mock *BackendMock) ListMultipartUploadsCalls() []struct { } // ListObjectParts calls ListObjectPartsFunc. -func (mock *BackendMock) ListObjectParts(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) { +func (mock *BackendMock) ListObjectParts(bucket string, object string, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) { if mock.ListObjectPartsFunc == nil { panic("BackendMock.ListObjectPartsFunc: method is nil but Backend.ListObjectParts was just called") } diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index bea3b767..da019ee4 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -24,7 +24,6 @@ import ( "os" "strconv" "strings" - "time" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -53,25 +52,21 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { key := ctx.Params("key") keyEnd := ctx.Params("*1") uploadId := ctx.Query("uploadId") - maxPartsStr := ctx.Query("max-parts") - partNumberMarkerStr := ctx.Query("part-number-marker") + maxParts := ctx.QueryInt("max-parts", 0) + partNumberMarker := ctx.QueryInt("part-number-marker", 0) acceptRange := ctx.Get("Range") if keyEnd != "" { key = strings.Join([]string{key, keyEnd}, "/") } if uploadId != "" { - maxParts, err := strconv.Atoi(maxPartsStr) - if err != nil && maxPartsStr != "" { - return errors.New("wrong api call") + if maxParts < 0 || (maxParts == 0 && ctx.Query("max-parts") != "") { + return ErrorResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidMaxParts)) } - - partNumberMarker, err := strconv.Atoi(partNumberMarkerStr) - if err != nil && partNumberMarkerStr != "" { - return errors.New("wrong api call") + if partNumberMarker < 0 || (partNumberMarker == 0 && ctx.Query("part-number-marker") != "") { + return ErrorResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidPartNumberMarker)) } - - res, err := c.be.ListObjectParts(bucket, "", uploadId, partNumberMarker, maxParts) + res, err := c.be.ListObjectParts(bucket, key, uploadId, partNumberMarker, maxParts) return Responce(ctx, res, err) } @@ -186,40 +181,24 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { keyStart = keyStart + "/" } - if partNumberStr != "" { - copySrcModifSinceDate, err := time.Parse(time.RFC3339, copySrcModifSince) - if err != nil && copySrcModifSince != "" { - return errors.New("wrong api call") - } - - copySrcUnmodifSinceDate, err := time.Parse(time.RFC3339, copySrcUnmodifSince) - if err != nil && copySrcUnmodifSince != "" { - return errors.New("wrong api call") - } - - partNumber, err := strconv.ParseInt(partNumberStr, 10, 64) + var contentLength int64 + if contentLengthStr != "" { + var err error + contentLength, err = strconv.ParseInt(contentLengthStr, 10, 64) if err != nil { - return errors.New("wrong api call") + return ErrorResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidRequest)) } - - res, err := c.be.UploadPartCopy(&s3.UploadPartCopyInput{ - Bucket: &bucket, - Key: &keyStart, - PartNumber: int32(partNumber), - UploadId: &uploadId, - CopySource: ©Source, - CopySourceIfMatch: ©SrcIfMatch, - CopySourceIfNoneMatch: ©SrcIfNoneMatch, - CopySourceIfModifiedSince: ©SrcModifSinceDate, - CopySourceIfUnmodifiedSince: ©SrcUnmodifSinceDate, - }) - - return Responce(ctx, res, err) } - if uploadId != "" { + if uploadId != "" && partNumberStr != "" { + partNumber := ctx.QueryInt("partNumber", -1) + if partNumber < 1 { + return ErrorResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidPart)) + } + body := io.ReadSeeker(bytes.NewReader([]byte(ctx.Body()))) - res, err := c.be.UploadPart(bucket, keyStart, uploadId, body) + res, err := c.be.PutObjectPart(bucket, keyStart, uploadId, + partNumber, contentLength, body) return Responce(ctx, res, err) } @@ -242,6 +221,8 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { } if copySource != "" { + _, _, _, _ = copySrcIfMatch, copySrcIfNoneMatch, + copySrcModifSince, copySrcUnmodifSince copySourceSplit := strings.Split(copySource, "/") srcBucket, srcObject := copySourceSplit[0], copySourceSplit[1:] @@ -249,11 +230,6 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { return Responce(ctx, res, err) } - contentLength, err := strconv.ParseInt(contentLengthStr, 10, 64) - if err != nil { - return errors.New("wrong api call") - } - metadata := utils.GetUserMetaData(&ctx.Request().Header) res, err := c.be.PutObject(&s3.PutObjectInput{ @@ -381,20 +357,22 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { } if uploadId != "" { - var parts []types.Part + data := struct { + Parts []types.Part `xml:"Part"` + }{} - if err := xml.Unmarshal(ctx.Body(), &parts); err != nil { + if err := xml.Unmarshal(ctx.Body(), &data); err != nil { return errors.New("wrong api call") } - res, err := c.be.CompleteMultipartUpload(bucket, "", uploadId, parts) + res, err := c.be.CompleteMultipartUpload(bucket, key, uploadId, data.Parts) return Responce(ctx, res, err) } res, err := c.be.CreateMultipartUpload(&s3.CreateMultipartUploadInput{Bucket: &bucket, Key: &key}) return Responce(ctx, res, err) } -func Responce[R comparable](ctx *fiber.Ctx, resp R, err error) error { +func Responce[R any](ctx *fiber.Ctx, resp R, err error) error { if err != nil { serr, ok := err.(s3err.APIError) if ok { @@ -414,6 +392,10 @@ func Responce[R comparable](ctx *fiber.Ctx, resp R, err error) error { return err } + if len(b) > 0 { + ctx.Response().Header.SetContentType(fiber.MIMEApplicationXML) + } + return ctx.Send(b) } diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index c27d6d56..48df0559 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -29,6 +29,7 @@ import ( "github.com/valyala/fasthttp" "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3err" + "github.com/versity/versitygw/s3response" ) func TestNew(t *testing.T) { @@ -128,8 +129,8 @@ func TestS3ApiController_GetActions(t *testing.T) { app := fiber.New() s3ApiController := S3ApiController{be: &BackendMock{ - ListObjectPartsFunc: func(bucket, object, uploadID string, partNumberMarker int, maxParts int) (*s3.ListPartsOutput, error) { - return &s3.ListPartsOutput{}, nil + ListObjectPartsFunc: func(bucket, object, uploadID string, partNumberMarker int, maxParts int) (s3response.ListPartsResponse, error) { + return s3response.ListPartsResponse{}, nil }, GetObjectAclFunc: func(bucket, object string) (*s3.GetObjectAclOutput, error) { return &s3.GetObjectAclOutput{}, nil @@ -169,16 +170,16 @@ func TestS3ApiController_GetActions(t *testing.T) { req: httptest.NewRequest(http.MethodGet, "/my-bucket/key?uploadId=hello&max-parts=InvalidMaxParts", nil), }, wantErr: false, - statusCode: 500, + statusCode: 400, }, { - name: "Get-actions-invalid-part-number", + name: "Get-actions-invalid-part-number-marker", app: app, args: args{ req: httptest.NewRequest(http.MethodGet, "/my-bucket/key?uploadId=hello&max-parts=200&part-number-marker=InvalidPartNumber", nil), }, wantErr: false, - statusCode: 500, + statusCode: 400, }, { name: "Get-actions-list-object-parts-success", @@ -233,8 +234,8 @@ func TestS3ApiController_ListActions(t *testing.T) { GetBucketAclFunc: func(bucket string) (*s3.GetBucketAclOutput, error) { return &s3.GetBucketAclOutput{}, nil }, - ListMultipartUploadsFunc: func(output *s3.ListMultipartUploadsInput) (*s3.ListMultipartUploadsOutput, error) { - return &s3.ListMultipartUploadsOutput{}, nil + ListMultipartUploadsFunc: func(output *s3.ListMultipartUploadsInput) (s3response.ListMultipartUploadsResponse, error) { + return s3response.ListMultipartUploadsResponse{}, nil }, ListObjectsV2Func: func(bucket, prefix, marker, delim string, maxkeys int) (*s3.ListObjectsV2Output, error) { return &s3.ListObjectsV2Output{}, nil @@ -441,13 +442,13 @@ func TestS3ApiController_PutActions(t *testing.T) { statusCode int }{ { - name: "Upload-copy-part-error-case", + name: "Upload-put-part-error-case", app: app, args: args{ - req: httptest.NewRequest(http.MethodPut, "/my-bucket/my-key?partNumber=invalid", nil), + req: httptest.NewRequest(http.MethodPut, "/my-bucket/my-key?uploadId=abc&partNumber=invalid", nil), }, wantErr: false, - statusCode: 500, + statusCode: 400, }, { name: "Upload-copy-part-success", @@ -517,11 +518,13 @@ func TestS3ApiController_PutActions(t *testing.T) { resp, err := tt.app.Test(tt.args.req) if (err != nil) != tt.wantErr { - t.Errorf("S3ApiController.GetActions() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("S3ApiController.GetActions() %v error = %v, wantErr %v", + tt.name, err, tt.wantErr) } if resp.StatusCode != tt.statusCode { - t.Errorf("S3ApiController.GetActions() statusCode = %v, wantStatusCode = %v", resp.StatusCode, tt.statusCode) + t.Errorf("S3ApiController.GetActions() %v statusCode = %v, wantStatusCode = %v", + tt.name, resp.StatusCode, tt.statusCode) } } } diff --git a/s3response/s3response.go b/s3response/s3response.go new file mode 100644 index 00000000..fec10330 --- /dev/null +++ b/s3response/s3response.go @@ -0,0 +1,96 @@ +// Copyright 2023 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package s3response + +import ( + "encoding/xml" +) + +// Part describes part metadata. +type Part struct { + PartNumber int + LastModified string + ETag string + Size int64 +} + +// ListPartsResponse - s3 api list parts response. +type ListPartsResponse struct { + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ListPartsResult" json:"-"` + + Bucket string + Key string + UploadID string `xml:"UploadId"` + + Initiator Initiator + Owner Owner + + // The class of storage used to store the object. + StorageClass string + + PartNumberMarker int + NextPartNumberMarker int + MaxParts int + IsTruncated bool + + // List of parts. + Parts []Part `xml:"Part"` +} + +// ListMultipartUploadsResponse - s3 api list multipart uploads response. +type ListMultipartUploadsResponse struct { + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ListMultipartUploadsResult" json:"-"` + + Bucket string + KeyMarker string + UploadIDMarker string `xml:"UploadIdMarker"` + NextKeyMarker string + NextUploadIDMarker string `xml:"NextUploadIdMarker"` + Delimiter string + Prefix string + EncodingType string `xml:"EncodingType,omitempty"` + MaxUploads int + IsTruncated bool + + // List of pending uploads. + Uploads []Upload `xml:"Upload"` + + // Delimed common prefixes. + CommonPrefixes []CommonPrefix +} + +// Upload desribes in progress multipart upload +type Upload struct { + Key string + UploadID string `xml:"UploadId"` + Initiator Initiator + Owner Owner + StorageClass string + Initiated string +} + +// CommonPrefix ListObjectsResponse common prefixes (directory abstraction) +type CommonPrefix struct { + Prefix string +} + +// Initiator same fields as Owner +type Initiator Owner + +// Owner bucket ownership +type Owner struct { + ID string + DisplayName string +} From 33e1d391389dff5702c5d809f40aad04048d2c51 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 10 Jun 2023 11:04:08 -0700 Subject: [PATCH 4/6] cleanup responses to split out expected xml body response --- s3api/controllers/base.go | 102 +++++++++++++++------------- s3api/controllers/base_test.go | 68 +++++++++++++++++-- s3api/middlewares/authentication.go | 28 ++++---- s3api/middlewares/md5.go | 2 +- 4 files changed, 134 insertions(+), 66 deletions(-) diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index da019ee4..4780b56e 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -44,7 +44,7 @@ func New(be backend.Backend) S3ApiController { func (c S3ApiController) ListBuckets(ctx *fiber.Ctx) error { res, err := c.be.ListBuckets() - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { @@ -61,29 +61,30 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { if uploadId != "" { if maxParts < 0 || (maxParts == 0 && ctx.Query("max-parts") != "") { - return ErrorResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidMaxParts)) + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidMaxParts)) } if partNumberMarker < 0 || (partNumberMarker == 0 && ctx.Query("part-number-marker") != "") { - return ErrorResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidPartNumberMarker)) + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidPartNumberMarker)) } res, err := c.be.ListObjectParts(bucket, key, uploadId, partNumberMarker, maxParts) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } if ctx.Request().URI().QueryArgs().Has("acl") { res, err := c.be.GetObjectAcl(bucket, key) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } if attrs := ctx.Get("X-Amz-Object-Attributes"); attrs != "" { res, err := c.be.GetObjectAttributes(bucket, key, strings.Split(attrs, ",")) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } - res, err := c.be.GetObject(bucket, key, acceptRange, ctx.Response().BodyWriter()) + _, err := c.be.GetObject(bucket, key, acceptRange, ctx.Response().BodyWriter()) if err != nil { - return Responce(ctx, res, err) + return SendResponse(ctx, err) } + // TODO set response headers? return nil } @@ -96,21 +97,21 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { if ctx.Request().URI().QueryArgs().Has("acl") { res, err := c.be.GetBucketAcl(ctx.Params("bucket")) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } if ctx.Request().URI().QueryArgs().Has("uploads") { res, err := c.be.ListMultipartUploads(&s3.ListMultipartUploadsInput{Bucket: aws.String(ctx.Params("bucket"))}) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } if ctx.QueryInt("list-type") == 2 { res, err := c.be.ListObjectsV2(bucket, prefix, marker, delimiter, maxkeys) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } res, err := c.be.ListObjects(bucket, prefix, marker, delimiter, maxkeys) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { @@ -139,11 +140,11 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { GrantWriteACP: &grantWriteACP, }) - return Responce[any](ctx, nil, err) + return SendResponse(ctx, err) } err := c.be.PutBucket(bucket) - return Responce[any](ctx, nil, err) + return SendResponse(ctx, err) } func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { @@ -186,20 +187,21 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { var err error contentLength, err = strconv.ParseInt(contentLengthStr, 10, 64) if err != nil { - return ErrorResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidRequest)) + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidRequest)) } } if uploadId != "" && partNumberStr != "" { partNumber := ctx.QueryInt("partNumber", -1) if partNumber < 1 { - return ErrorResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidPart)) + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidPart)) } body := io.ReadSeeker(bytes.NewReader([]byte(ctx.Body()))) - res, err := c.be.PutObjectPart(bucket, keyStart, uploadId, + etag, err := c.be.PutObjectPart(bucket, keyStart, uploadId, partNumber, contentLength, body) - return Responce(ctx, res, err) + ctx.Response().Header.Set("Etag", etag) + return SendResponse(ctx, err) } if grants != "" || acl != "" { @@ -217,7 +219,7 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { GrantWrite: &granWrite, GrantWriteACP: &grantWriteACP, }) - return Responce[any](ctx, nil, err) + return SendResponse(ctx, err) } if copySource != "" { @@ -227,24 +229,25 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { srcBucket, srcObject := copySourceSplit[0], copySourceSplit[1:] res, err := c.be.CopyObject(srcBucket, strings.Join(srcObject, "/"), bucket, keyStart) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } metadata := utils.GetUserMetaData(&ctx.Request().Header) - res, err := c.be.PutObject(&s3.PutObjectInput{ + etag, err := c.be.PutObject(&s3.PutObjectInput{ Bucket: &bucket, Key: &keyStart, ContentLength: contentLength, Metadata: metadata, Body: bytes.NewReader(ctx.Request().Body()), }) - return Responce(ctx, res, err) + ctx.Response().Header.Set("ETag", etag) + return SendResponse(ctx, err) } func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error { err := c.be.DeleteBucket(ctx.Params("bucket")) - return Responce[any](ctx, nil, err) + return SendResponse(ctx, err) } func (c S3ApiController) DeleteObjects(ctx *fiber.Ctx) error { @@ -254,7 +257,7 @@ func (c S3ApiController) DeleteObjects(ctx *fiber.Ctx) error { } err := c.be.DeleteObjects(ctx.Params("bucket"), &s3.DeleteObjectsInput{Delete: &dObj}) - return Responce[any](ctx, nil, err) + return SendResponse(ctx, err) } func (c S3ApiController) DeleteActions(ctx *fiber.Ctx) error { @@ -277,16 +280,17 @@ func (c S3ApiController) DeleteActions(ctx *fiber.Ctx) error { ExpectedBucketOwner: &expectedBucketOwner, RequestPayer: types.RequestPayer(requestPayer), }) - return Responce[any](ctx, nil, err) + return SendResponse(ctx, err) } err := c.be.DeleteObject(bucket, key) - return Responce[any](ctx, nil, err) + return SendResponse(ctx, err) } func (c S3ApiController) HeadBucket(ctx *fiber.Ctx) error { - res, err := c.be.HeadBucket(ctx.Params("bucket")) - return Responce(ctx, res, err) + _, err := c.be.HeadBucket(ctx.Params("bucket")) + // TODO: set bucket response headers + return SendResponse(ctx, err) } const ( @@ -303,7 +307,7 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error { res, err := c.be.HeadObject(bucket, key) if err != nil { - return ErrorResponse(ctx, err) + return SendResponse(ctx, err) } utils.SetMetaHeaders(ctx, res.Metadata) @@ -330,10 +334,7 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error { }, }) - // https://github.com/gofiber/fiber/issues/2080 - // ctx.SendStatus() sets incorrect content length on HEAD request - ctx.Status(http.StatusOK) - return nil + return SendResponse(ctx, nil) } func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { @@ -353,7 +354,7 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { return errors.New("wrong api call") } err := c.be.RestoreObject(bucket, key, &restoreRequest) - return Responce[any](ctx, nil, err) + return SendResponse(ctx, err) } if uploadId != "" { @@ -366,13 +367,30 @@ func (c S3ApiController) CreateActions(ctx *fiber.Ctx) error { } res, err := c.be.CompleteMultipartUpload(bucket, key, uploadId, data.Parts) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } res, err := c.be.CreateMultipartUpload(&s3.CreateMultipartUploadInput{Bucket: &bucket, Key: &key}) - return Responce(ctx, res, err) + return SendXMLResponse(ctx, res, err) } -func Responce[R any](ctx *fiber.Ctx, resp R, err error) error { +func SendResponse(ctx *fiber.Ctx, err error) error { + if err != nil { + serr, ok := err.(s3err.APIError) + if ok { + ctx.Status(serr.HTTPStatusCode) + return ctx.Send(s3err.GetAPIErrorResponse(serr, "", "", "")) + } + return ctx.Send(s3err.GetAPIErrorResponse( + s3err.GetAPIError(s3err.ErrInternalError), "", "", "")) + } + + // https://github.com/gofiber/fiber/issues/2080 + // ctx.SendStatus() sets incorrect content length on HEAD request + ctx.Status(http.StatusOK) + return nil +} + +func SendXMLResponse(ctx *fiber.Ctx, resp any, err error) error { if err != nil { serr, ok := err.(s3err.APIError) if ok { @@ -398,13 +416,3 @@ func Responce[R any](ctx *fiber.Ctx, resp R, err error) error { return ctx.Send(b) } - -func ErrorResponse(ctx *fiber.Ctx, err error) error { - serr, ok := err.(s3err.APIError) - if ok { - ctx.Status(serr.HTTPStatusCode) - return ctx.Send(s3err.GetAPIErrorResponse(serr, "", "", "")) - } - return ctx.Send(s3err.GetAPIErrorResponse( - s3err.GetAPIError(s3err.ErrInternalError), "", "", "")) -} diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index 48df0559..898ecfbd 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -954,7 +954,7 @@ func TestS3ApiController_CreateActions(t *testing.T) { } } -func Test_responce(t *testing.T) { +func Test_XMLresponse(t *testing.T) { type args struct { ctx *fiber.Ctx resp any @@ -1011,14 +1011,74 @@ func Test_responce(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := Responce(tt.args.ctx, tt.args.resp, tt.args.err); (err != nil) != tt.wantErr { - t.Errorf("responce() error = %v, wantErr %v", err, tt.wantErr) + if err := SendXMLResponse(tt.args.ctx, tt.args.resp, tt.args.err); (err != nil) != tt.wantErr { + t.Errorf("responce() %v error = %v, wantErr %v", tt.name, err, tt.wantErr) } statusCode := tt.args.ctx.Response().StatusCode() if statusCode != tt.statusCode { - t.Errorf("responce() code = %v, wantErr %v", statusCode, tt.wantErr) + t.Errorf("responce() %v code = %v, wantErr %v", tt.name, statusCode, tt.wantErr) + } + }) + } +} + +func Test_response(t *testing.T) { + type args struct { + ctx *fiber.Ctx + resp any + err error + } + app := fiber.New() + + tests := []struct { + name string + args args + wantErr bool + statusCode int + }{ + { + name: "Internal-server-error", + args: args{ + ctx: app.AcquireCtx(&fasthttp.RequestCtx{}), + resp: nil, + err: s3err.GetAPIError(16), + }, + wantErr: false, + statusCode: 500, + }, + { + name: "Error-not-implemented", + args: args{ + ctx: app.AcquireCtx(&fasthttp.RequestCtx{}), + resp: nil, + err: s3err.GetAPIError(50), + }, + wantErr: false, + statusCode: 501, + }, + { + name: "Successful-response", + args: args{ + ctx: app.AcquireCtx(&fasthttp.RequestCtx{}), + resp: "Valid response", + err: nil, + }, + wantErr: false, + statusCode: 200, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := SendResponse(tt.args.ctx, tt.args.err); (err != nil) != tt.wantErr { + t.Errorf("responce() %v error = %v, wantErr %v", tt.name, err, tt.wantErr) + } + + statusCode := tt.args.ctx.Response().StatusCode() + + if statusCode != tt.statusCode { + t.Errorf("responce() %v code = %v, wantErr %v", tt.name, statusCode, tt.wantErr) } }) } diff --git a/s3api/middlewares/authentication.go b/s3api/middlewares/authentication.go index 4fcf6294..90f11951 100644 --- a/s3api/middlewares/authentication.go +++ b/s3api/middlewares/authentication.go @@ -47,48 +47,48 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fib return func(ctx *fiber.Ctx) error { authorization := ctx.Get("Authorization") if authorization == "" { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrAuthHeaderEmpty)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrAuthHeaderEmpty)) } // Check the signature version authParts := strings.Split(authorization, " ") if len(authParts) < 4 { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrMissingFields)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrMissingFields)) } if authParts[0] != "AWS4-HMAC-SHA256" { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrSignatureVersionNotSupported)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrSignatureVersionNotSupported)) } credKv := strings.Split(authParts[1], "=") if len(credKv) != 2 { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrCredMalformed)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrCredMalformed)) } creds := strings.Split(credKv[1], "/") if len(creds) < 4 { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrCredMalformed)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrCredMalformed)) } signHdrKv := strings.Split(authParts[2], "=") if len(signHdrKv) != 2 { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrCredMalformed)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrCredMalformed)) } signedHdrs := strings.Split(signHdrKv[1], ";") account := acct.getAccount(creds[0]) if account == nil { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrInvalidAccessKeyID)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidAccessKeyID)) } // Check X-Amz-Date header date := ctx.Get("X-Amz-Date") if date == "" { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrMissingDateHeader)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrMissingDateHeader)) } // Parse the date and check the date validity tdate, err := time.Parse(iso8601Format, date) if err != nil { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrMalformedDate)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrMalformedDate)) } // Calculate the hash of the request payload @@ -99,13 +99,13 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fib // Compare the calculated hash with the hash provided if hashPayloadHeader != hexPayload { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrContentSHA256Mismatch)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrContentSHA256Mismatch)) } // Create a new http request instance from fasthttp request req, err := utils.CreateHttpRequestFromCtx(ctx, signedHdrs) if err != nil { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrInternalError)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrInternalError)) } signer := v4.NewSigner() @@ -120,18 +120,18 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fib } }) if signErr != nil { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrInternalError)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrInternalError)) } parts := strings.Split(req.Header.Get("Authorization"), " ") if len(parts) < 4 { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrMissingFields)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrMissingFields)) } calculatedSign := strings.Split(parts[3], "=")[1] expectedSign := strings.Split(authParts[3], "=")[1] if expectedSign != calculatedSign { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrSignatureDoesNotMatch)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrSignatureDoesNotMatch)) } ctx.Locals("role", account.Role) diff --git a/s3api/middlewares/md5.go b/s3api/middlewares/md5.go index 33bdcdf7..e5b82333 100644 --- a/s3api/middlewares/md5.go +++ b/s3api/middlewares/md5.go @@ -34,7 +34,7 @@ func VerifyMD5Body() fiber.Handler { calculatedSum := base64.StdEncoding.EncodeToString(sum[:]) if incomingSum != calculatedSum { - return controllers.Responce[any](ctx, nil, s3err.GetAPIError(s3err.ErrInvalidDigest)) + return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidDigest)) } return ctx.Next() From feace16fa948693525bdcb6ea35f80a9a4b432f8 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 10 Jun 2023 11:10:27 -0700 Subject: [PATCH 5/6] set response headers for get object --- s3api/controllers/base.go | 71 ++++++++++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 12 deletions(-) diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 4780b56e..1dce0ca8 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -80,12 +80,49 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { return SendXMLResponse(ctx, res, err) } - _, err := c.be.GetObject(bucket, key, acceptRange, ctx.Response().BodyWriter()) + res, err := c.be.GetObject(bucket, key, acceptRange, ctx.Response().BodyWriter()) if err != nil { return SendResponse(ctx, err) } - // TODO set response headers? - return nil + if res == nil { + return SendResponse(ctx, fmt.Errorf("get object nil response")) + } + + utils.SetMetaHeaders(ctx, res.Metadata) + var lastmod string + if res.LastModified != nil { + lastmod = res.LastModified.Format(timefmt) + } + utils.SetResponseHeaders(ctx, []utils.CustomHeader{ + { + Key: "Content-Length", + Value: fmt.Sprint(res.ContentLength), + }, + { + Key: "Content-Type", + Value: getstring(res.ContentType), + }, + { + Key: "Content-Encoding", + Value: getstring(res.ContentEncoding), + }, + { + Key: "ETag", + Value: getstring(res.ETag), + }, + { + Key: "Last-Modified", + Value: lastmod, + }, + }) + return ctx.SendStatus(http.StatusOK) +} + +func getstring(s *string) string { + if s == nil { + return "" + } + return *s } func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { @@ -309,8 +346,15 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error { if err != nil { return SendResponse(ctx, err) } + if res == nil { + return SendResponse(ctx, fmt.Errorf("head object nil response")) + } utils.SetMetaHeaders(ctx, res.Metadata) + var lastmod string + if res.LastModified != nil { + lastmod = res.LastModified.Format(timefmt) + } utils.SetResponseHeaders(ctx, []utils.CustomHeader{ { Key: "Content-Length", @@ -318,19 +362,19 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error { }, { Key: "Content-Type", - Value: *res.ContentType, + Value: getstring(res.ContentType), }, { Key: "Content-Encoding", - Value: *res.ContentEncoding, + Value: getstring(res.ContentEncoding), }, { Key: "ETag", - Value: *res.ETag, + Value: getstring(res.ETag), }, { Key: "Last-Modified", - Value: res.LastModified.Format(timefmt), + Value: lastmod, }, }) @@ -406,12 +450,15 @@ func SendXMLResponse(ctx *fiber.Ctx, resp any, err error) error { } var b []byte - if b, err = xml.Marshal(resp); err != nil { - return err - } - if len(b) > 0 { - ctx.Response().Header.SetContentType(fiber.MIMEApplicationXML) + if resp != nil { + if b, err = xml.Marshal(resp); err != nil { + return err + } + + if len(b) > 0 { + ctx.Response().Header.SetContentType(fiber.MIMEApplicationXML) + } } return ctx.Send(b) From afb40db50ea4ecb5ff501bdc1fc81a5bb6a156c9 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Mon, 12 Jun 2023 08:41:02 -0700 Subject: [PATCH 6/6] Create CODE_OF_CONDUCT.md --- CODE_OF_CONDUCT.md | 128 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 CODE_OF_CONDUCT.md diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 00000000..d17b9650 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,128 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, religion, or sexual identity +and orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment for our +community include: + +* Demonstrating empathy and kindness toward other people +* Being respectful of differing opinions, viewpoints, and experiences +* Giving and gracefully accepting constructive feedback +* Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +* Focusing on what is best not just for us as individuals, but for the + overall community + +Examples of unacceptable behavior include: + +* The use of sexualized language or imagery, and sexual attention or + advances of any kind +* Trolling, insulting or derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or email + address, without their explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Community leaders are responsible for clarifying and enforcing our standards of +acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +Community leaders have the right and responsibility to remove, edit, or reject +comments, commits, code, wiki edits, issues, and other contributions that are +not aligned to this Code of Conduct, and will communicate reasons for moderation +decisions when appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when +an individual is officially representing the community in public spaces. +Examples of representing our community include using an official e-mail address, +posting via an official social media account, or acting as an appointed +representative at an online or offline event. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported to the community leaders responsible for enforcement at +versitygw@versity.com. +All complaints will be reviewed and investigated promptly and fairly. + +All community leaders are obligated to respect the privacy and security of the +reporter of any incident. + +## Enforcement Guidelines + +Community leaders will follow these Community Impact Guidelines in determining +the consequences for any action they deem in violation of this Code of Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed +unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from community leaders, providing +clarity around the nature of the violation and an explanation of why the +behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series +of actions. + +**Consequence**: A warning with consequences for continued behavior. No +interaction with the people involved, including unsolicited interaction with +those enforcing the Code of Conduct, for a specified period of time. This +includes avoiding interactions in community spaces as well as external channels +like social media. Violating these terms may lead to a temporary or +permanent ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including +sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public +communication with the community for a specified period of time. No public or +private interaction with the people involved, including unsolicited interaction +with those enforcing the Code of Conduct, is allowed during this period. +Violating these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community +standards, including sustained inappropriate behavior, harassment of an +individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within +the community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.0, available at +https://www.contributor-covenant.org/version/2/0/code_of_conduct.html. + +Community Impact Guidelines were inspired by [Mozilla's code of conduct +enforcement ladder](https://github.com/mozilla/diversity). + +[homepage]: https://www.contributor-covenant.org + +For answers to common questions about this code of conduct, see the FAQ at +https://www.contributor-covenant.org/faq. Translations are available at +https://www.contributor-covenant.org/translations.