From 8f27e881989c90a9af1b46fc31925fbe53e790bb Mon Sep 17 00:00:00 2001 From: Jon Austin <62040422+jonaustin09@users.noreply.github.com> Date: Tue, 6 Jun 2023 11:13:45 -0700 Subject: [PATCH] feat: GetObject range calculation moved to backend, created utility function for it in the backend (#61) --- backend/backend.go | 4 +- backend/backend_moq_test.go | 57 ++------------------------- backend/posix/posix.go | 36 ++++++++++++++++- s3api/controllers/backend_moq_test.go | 57 ++------------------------- s3api/controllers/base.go | 26 +----------- s3api/controllers/base_test.go | 2 +- 6 files changed, 48 insertions(+), 134 deletions(-) diff --git a/backend/backend.go b/backend/backend.go index 002d5cb..caa6bac 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -46,7 +46,7 @@ type Backend interface { PutObject(*s3.PutObjectInput) (string, error) HeadObject(bucket, object string) (*s3.HeadObjectOutput, error) - GetObject(bucket, object, acceptRange string, startOffset, length int64, writer io.Writer) (*s3.GetObjectOutput, error) + GetObject(bucket, object, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) GetObjectAcl(bucket, object string) (*s3.GetObjectAclOutput, error) GetObjectAttributes(bucket, object string, attributes []string) (*s3.GetObjectAttributesOutput, error) CopyObject(srcBucket, srcObject, DstBucket, dstObject string) (*s3.CopyObjectOutput, error) @@ -137,7 +137,7 @@ func (BackendUnsupported) DeleteObject(bucket, object string) error { func (BackendUnsupported) DeleteObjects(bucket string, objects *s3.DeleteObjectsInput) error { return s3err.GetAPIError(s3err.ErrNotImplemented) } -func (BackendUnsupported) GetObject(bucket, object, acceptRange string, startOffset, length int64, writer io.Writer) (*s3.GetObjectOutput, error) { +func (BackendUnsupported) GetObject(bucket, object, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) } func (BackendUnsupported) HeadObject(bucket, object string) (*s3.HeadObjectOutput, error) { diff --git a/backend/backend_moq_test.go b/backend/backend_moq_test.go index c4c41ec..d3f2e4d 100644 --- a/backend/backend_moq_test.go +++ b/backend/backend_moq_test.go @@ -47,10 +47,7 @@ var _ Backend = &BackendMock{} // GetBucketAclFunc: func(bucket string) (*s3.GetBucketAclOutput, error) { // panic("mock out the GetBucketAcl method") // }, -// GetIAMConfigFunc: func() ([]byte, error) { -// panic("mock out the GetIAMConfig method") -// }, -// GetObjectFunc: func(bucket string, object string, acceptRange string, startOffset int64, length int64, writer io.Writer) (*s3.GetObjectOutput, error) { +// GetObjectFunc: func(bucket string, object string, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) { // panic("mock out the GetObject method") // }, // GetObjectAclFunc: func(bucket string, object string) (*s3.GetObjectAclOutput, error) { @@ -153,11 +150,8 @@ type BackendMock struct { // GetBucketAclFunc mocks the GetBucketAcl method. GetBucketAclFunc func(bucket string) (*s3.GetBucketAclOutput, error) - // GetIAMConfigFunc mocks the GetIAMConfig method. - GetIAMConfigFunc func() ([]byte, error) - // GetObjectFunc mocks the GetObject method. - GetObjectFunc func(bucket string, object string, acceptRange string, startOffset int64, length int64, writer io.Writer) (*s3.GetObjectOutput, error) + GetObjectFunc func(bucket string, object string, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) // GetObjectAclFunc mocks the GetObjectAcl method. GetObjectAclFunc func(bucket string, object string) (*s3.GetObjectAclOutput, error) @@ -298,9 +292,6 @@ type BackendMock struct { // Bucket is the bucket argument value. Bucket string } - // GetIAMConfig holds details about calls to the GetIAMConfig method. - GetIAMConfig []struct { - } // GetObject holds details about calls to the GetObject method. GetObject []struct { // Bucket is the bucket argument value. @@ -309,10 +300,6 @@ type BackendMock struct { Object string // AcceptRange is the acceptRange argument value. AcceptRange string - // StartOffset is the startOffset argument value. - StartOffset int64 - // Length is the length argument value. - Length int64 // Writer is the writer argument value. Writer io.Writer } @@ -490,7 +477,6 @@ type BackendMock struct { lockDeleteObject sync.RWMutex lockDeleteObjects sync.RWMutex lockGetBucketAcl sync.RWMutex - lockGetIAMConfig sync.RWMutex lockGetObject sync.RWMutex lockGetObjectAcl sync.RWMutex lockGetObjectAttributes sync.RWMutex @@ -856,35 +842,8 @@ func (mock *BackendMock) GetBucketAclCalls() []struct { return calls } -// GetIAMConfig calls GetIAMConfigFunc. -func (mock *BackendMock) GetIAMConfig() ([]byte, error) { - if mock.GetIAMConfigFunc == nil { - panic("BackendMock.GetIAMConfigFunc: method is nil but Backend.GetIAMConfig was just called") - } - callInfo := struct { - }{} - mock.lockGetIAMConfig.Lock() - mock.calls.GetIAMConfig = append(mock.calls.GetIAMConfig, callInfo) - mock.lockGetIAMConfig.Unlock() - return mock.GetIAMConfigFunc() -} - -// GetIAMConfigCalls gets all the calls that were made to GetIAMConfig. -// Check the length with: -// -// len(mockedBackend.GetIAMConfigCalls()) -func (mock *BackendMock) GetIAMConfigCalls() []struct { -} { - var calls []struct { - } - mock.lockGetIAMConfig.RLock() - calls = mock.calls.GetIAMConfig - mock.lockGetIAMConfig.RUnlock() - return calls -} - // GetObject calls GetObjectFunc. -func (mock *BackendMock) GetObject(bucket string, object string, acceptRange string, startOffset int64, length int64, writer io.Writer) (*s3.GetObjectOutput, error) { +func (mock *BackendMock) GetObject(bucket string, object string, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) { if mock.GetObjectFunc == nil { panic("BackendMock.GetObjectFunc: method is nil but Backend.GetObject was just called") } @@ -892,21 +851,17 @@ func (mock *BackendMock) GetObject(bucket string, object string, acceptRange str Bucket string Object string AcceptRange string - StartOffset int64 - Length int64 Writer io.Writer }{ Bucket: bucket, Object: object, AcceptRange: acceptRange, - StartOffset: startOffset, - Length: length, Writer: writer, } mock.lockGetObject.Lock() mock.calls.GetObject = append(mock.calls.GetObject, callInfo) mock.lockGetObject.Unlock() - return mock.GetObjectFunc(bucket, object, acceptRange, startOffset, length, writer) + return mock.GetObjectFunc(bucket, object, acceptRange, writer) } // GetObjectCalls gets all the calls that were made to GetObject. @@ -917,16 +872,12 @@ func (mock *BackendMock) GetObjectCalls() []struct { Bucket string Object string AcceptRange string - StartOffset int64 - Length int64 Writer io.Writer } { var calls []struct { Bucket string Object string AcceptRange string - StartOffset int64 - Length int64 Writer io.Writer } mock.lockGetObject.RLock() diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 0c4aa9b..9b3885f 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -853,7 +853,7 @@ func (p *Posix) DeleteObjects(bucket string, objects *s3.DeleteObjectsInput) err return nil } -func (p *Posix) GetObject(bucket, object, acceptRange string, startOffset, length int64, writer io.Writer) (*s3.GetObjectOutput, error) { +func (p *Posix) GetObject(bucket, object, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) { _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -871,6 +871,11 @@ func (p *Posix) GetObject(bucket, object, acceptRange string, startOffset, lengt return nil, fmt.Errorf("stat object: %w", err) } + startOffset, length, err := parseRange(fi, acceptRange) + if err != nil { + return nil, err + } + if startOffset+length > fi.Size() { // TODO: is ErrInvalidRequest correct here? return nil, s3err.GetAPIError(s3err.ErrInvalidRequest) @@ -1142,3 +1147,32 @@ func isNoAttr(err error) bool { } return false } + +func parseRange(file fs.FileInfo, acceptRange string) (int64, int64, error) { + if acceptRange == "" { + return 0, file.Size(), nil + } + + bRangeSl := strings.Split(acceptRange, "=") + + if len(bRangeSl) < 2 { + return 0, 0, errors.New("invalid range parameter") + } + + bRange := strings.Split(bRangeSl[1], "-") + if len(bRange) < 2 { + return 0, 0, errors.New("invalid range parameter") + } + + startOffset, err := strconv.ParseInt(bRange[0], 10, 64) + if err != nil { + return 0, 0, errors.New("invalid range parameter") + } + + length, err := strconv.ParseInt(bRange[1], 10, 64) + if err != nil { + return 0, 0, errors.New("invalid range parameter") + } + + return int64(startOffset), int64(length), nil +} diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index 5dccd89..44df779 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -48,10 +48,7 @@ var _ backend.Backend = &BackendMock{} // GetBucketAclFunc: func(bucket string) (*s3.GetBucketAclOutput, error) { // panic("mock out the GetBucketAcl method") // }, -// GetIAMConfigFunc: func() ([]byte, error) { -// panic("mock out the GetIAMConfig method") -// }, -// GetObjectFunc: func(bucket string, object string, acceptRange string, startOffset int64, length int64, writer io.Writer) (*s3.GetObjectOutput, error) { +// GetObjectFunc: func(bucket string, object string, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) { // panic("mock out the GetObject method") // }, // GetObjectAclFunc: func(bucket string, object string) (*s3.GetObjectAclOutput, error) { @@ -154,11 +151,8 @@ type BackendMock struct { // GetBucketAclFunc mocks the GetBucketAcl method. GetBucketAclFunc func(bucket string) (*s3.GetBucketAclOutput, error) - // GetIAMConfigFunc mocks the GetIAMConfig method. - GetIAMConfigFunc func() ([]byte, error) - // GetObjectFunc mocks the GetObject method. - GetObjectFunc func(bucket string, object string, acceptRange string, startOffset int64, length int64, writer io.Writer) (*s3.GetObjectOutput, error) + GetObjectFunc func(bucket string, object string, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) // GetObjectAclFunc mocks the GetObjectAcl method. GetObjectAclFunc func(bucket string, object string) (*s3.GetObjectAclOutput, error) @@ -299,9 +293,6 @@ type BackendMock struct { // Bucket is the bucket argument value. Bucket string } - // GetIAMConfig holds details about calls to the GetIAMConfig method. - GetIAMConfig []struct { - } // GetObject holds details about calls to the GetObject method. GetObject []struct { // Bucket is the bucket argument value. @@ -310,10 +301,6 @@ type BackendMock struct { Object string // AcceptRange is the acceptRange argument value. AcceptRange string - // StartOffset is the startOffset argument value. - StartOffset int64 - // Length is the length argument value. - Length int64 // Writer is the writer argument value. Writer io.Writer } @@ -491,7 +478,6 @@ type BackendMock struct { lockDeleteObject sync.RWMutex lockDeleteObjects sync.RWMutex lockGetBucketAcl sync.RWMutex - lockGetIAMConfig sync.RWMutex lockGetObject sync.RWMutex lockGetObjectAcl sync.RWMutex lockGetObjectAttributes sync.RWMutex @@ -857,35 +843,8 @@ func (mock *BackendMock) GetBucketAclCalls() []struct { return calls } -// GetIAMConfig calls GetIAMConfigFunc. -func (mock *BackendMock) GetIAMConfig() ([]byte, error) { - if mock.GetIAMConfigFunc == nil { - panic("BackendMock.GetIAMConfigFunc: method is nil but Backend.GetIAMConfig was just called") - } - callInfo := struct { - }{} - mock.lockGetIAMConfig.Lock() - mock.calls.GetIAMConfig = append(mock.calls.GetIAMConfig, callInfo) - mock.lockGetIAMConfig.Unlock() - return mock.GetIAMConfigFunc() -} - -// GetIAMConfigCalls gets all the calls that were made to GetIAMConfig. -// Check the length with: -// -// len(mockedBackend.GetIAMConfigCalls()) -func (mock *BackendMock) GetIAMConfigCalls() []struct { -} { - var calls []struct { - } - mock.lockGetIAMConfig.RLock() - calls = mock.calls.GetIAMConfig - mock.lockGetIAMConfig.RUnlock() - return calls -} - // GetObject calls GetObjectFunc. -func (mock *BackendMock) GetObject(bucket string, object string, acceptRange string, startOffset int64, length int64, writer io.Writer) (*s3.GetObjectOutput, error) { +func (mock *BackendMock) GetObject(bucket string, object string, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) { if mock.GetObjectFunc == nil { panic("BackendMock.GetObjectFunc: method is nil but Backend.GetObject was just called") } @@ -893,21 +852,17 @@ func (mock *BackendMock) GetObject(bucket string, object string, acceptRange str Bucket string Object string AcceptRange string - StartOffset int64 - Length int64 Writer io.Writer }{ Bucket: bucket, Object: object, AcceptRange: acceptRange, - StartOffset: startOffset, - Length: length, Writer: writer, } mock.lockGetObject.Lock() mock.calls.GetObject = append(mock.calls.GetObject, callInfo) mock.lockGetObject.Unlock() - return mock.GetObjectFunc(bucket, object, acceptRange, startOffset, length, writer) + return mock.GetObjectFunc(bucket, object, acceptRange, writer) } // GetObjectCalls gets all the calls that were made to GetObject. @@ -918,16 +873,12 @@ func (mock *BackendMock) GetObjectCalls() []struct { Bucket string Object string AcceptRange string - StartOffset int64 - Length int64 Writer io.Writer } { var calls []struct { Bucket string Object string AcceptRange string - StartOffset int64 - Length int64 Writer io.Writer } mock.lockGetObject.RLock() diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 5d2a22d..bb02baa 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -48,7 +48,7 @@ func (c S3ApiController) ListBuckets(ctx *fiber.Ctx) error { } func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { - bucket, key, keyEnd, uploadId, maxPartsStr, partNumberMarkerStr := ctx.Params("bucket"), ctx.Params("key"), ctx.Params("*1"), ctx.Query("uploadId"), ctx.Query("max-parts"), ctx.Query("part-number-marker") + bucket, key, keyEnd, uploadId, maxPartsStr, partNumberMarkerStr, acceptRange := ctx.Params("bucket"), ctx.Params("key"), ctx.Params("*1"), ctx.Query("uploadId"), ctx.Query("max-parts"), ctx.Query("part-number-marker"), ctx.Get("Range") if keyEnd != "" { key = strings.Join([]string{key, keyEnd}, "/") } @@ -78,29 +78,7 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error { return Responce(ctx, res, err) } - acceptRange := ctx.Get("Range") - - bRangeSl := strings.Split(acceptRange, "=") - if len(bRangeSl) < 2 { - return errors.New("wrong api call") - } - - bRange := strings.Split(bRangeSl[1], "-") - if len(bRange) < 2 { - return errors.New("wrong api call") - } - - startOffset, err := strconv.Atoi(bRange[0]) - if err != nil { - return errors.New("wrong api call") - } - - length, err := strconv.Atoi(bRange[1]) - if err != nil { - return errors.New("wrong api call") - } - - res, err := c.be.GetObject(bucket, key, acceptRange, int64(startOffset), int64(length), ctx.Response().BodyWriter()) + res, err := c.be.GetObject(bucket, key, acceptRange, ctx.Response().BodyWriter()) return Responce(ctx, res, err) } diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index e7ce41d..658c92f 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -137,7 +137,7 @@ func TestS3ApiController_GetActions(t *testing.T) { GetObjectAttributesFunc: func(bucket, object string, attributes []string) (*s3.GetObjectAttributesOutput, error) { return &s3.GetObjectAttributesOutput{}, nil }, - GetObjectFunc: func(bucket, object, acceptRange string, startOffset, length int64, writer io.Writer) (*s3.GetObjectOutput, error) { + GetObjectFunc: func(bucket, object, acceptRange string, writer io.Writer) (*s3.GetObjectOutput, error) { return &s3.GetObjectOutput{Metadata: nil}, nil }, }}