From 60151a70b0d14ed500fcd75e340c36eae1cb28e1 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Wed, 19 Feb 2025 22:07:25 +0400 Subject: [PATCH 1/2] fix: Fixes the typ in retrieveChecksums function in posix --- backend/posix/posix.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 5b3ac880..2d0a0dc9 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1405,7 +1405,7 @@ func (p *Posix) CompleteMultipartUpload(ctx context.Context, input *s3.CompleteM objdir := filepath.Join(metaTmpMultipartDir, fmt.Sprintf("%x", sum)) - checksums, err := p.retreiveChecksums(nil, bucket, filepath.Join(objdir, uploadID)) + checksums, err := p.retrieveChecksums(nil, bucket, filepath.Join(objdir, uploadID)) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return nil, fmt.Errorf("get mp checksums: %w", err) } @@ -1466,7 +1466,7 @@ func (p *Posix) CompleteMultipartUpload(ctx context.Context, input *s3.CompleteM return nil, s3err.GetAPIError(s3err.ErrInvalidPart) } - partChecksum, err := p.retreiveChecksums(nil, bucket, partObjPath) + partChecksum, err := p.retrieveChecksums(nil, bucket, partObjPath) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return nil, fmt.Errorf("get part checksum: %w", err) } @@ -1992,7 +1992,7 @@ func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUpl keyMarkerInd = len(uploads) } - checksum, err := p.retreiveChecksums(nil, bucket, filepath.Join(metaTmpMultipartDir, obj.Name(), uploadID)) + checksum, err := p.retrieveChecksums(nil, bucket, filepath.Join(metaTmpMultipartDir, obj.Name(), uploadID)) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return lmu, fmt.Errorf("get mp checksum: %w", err) } @@ -2122,7 +2122,7 @@ func (p *Posix) ListParts(_ context.Context, input *s3.ListPartsInput) (s3respon return lpr, fmt.Errorf("readdir upload: %w", err) } - checksum, err := p.retreiveChecksums(nil, tmpdir, uploadID) + checksum, err := p.retrieveChecksums(nil, tmpdir, uploadID) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return lpr, fmt.Errorf("get mp checksum: %w", err) } @@ -2145,7 +2145,7 @@ func (p *Posix) ListParts(_ context.Context, input *s3.ListPartsInput) (s3respon etag = "" } - checksum, err := p.retreiveChecksums(nil, bucket, partPath) + checksum, err := p.retrieveChecksums(nil, bucket, partPath) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { continue } @@ -2312,7 +2312,7 @@ func (p *Posix) UploadPart(ctx context.Context, input *s3.UploadPartInput) (*s3. tr = hashRdr } - checksums, chErr := p.retreiveChecksums(nil, bucket, mpPath) + checksums, chErr := p.retrieveChecksums(nil, bucket, mpPath) if chErr != nil && !errors.Is(chErr, meta.ErrNoSuchKey) { return nil, fmt.Errorf("retreive mp checksum: %w", chErr) } @@ -2519,12 +2519,12 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput) hash := md5.New() tr := io.TeeReader(rdr, hash) - mpChecksums, err := p.retreiveChecksums(nil, *upi.Bucket, filepath.Join(objdir, *upi.UploadId)) + mpChecksums, err := p.retrieveChecksums(nil, *upi.Bucket, filepath.Join(objdir, *upi.UploadId)) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return s3response.CopyPartResult{}, fmt.Errorf("retreive mp checksums: %w", err) } - checksums, err := p.retreiveChecksums(nil, objPath, "") + checksums, err := p.retrieveChecksums(nil, objPath, "") if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return s3response.CopyPartResult{}, fmt.Errorf("retreive object part checksums: %w", err) } @@ -3508,7 +3508,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO var cType types.ChecksumType // Skip the checksums retreival if object isn't requested fully if input.ChecksumMode == types.ChecksumModeEnabled && length-startOffset == objSize { - checksums, err = p.retreiveChecksums(f, bucket, object) + checksums, err = p.retrieveChecksums(f, bucket, object) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return nil, fmt.Errorf("get object checksums: %w", err) } @@ -3719,7 +3719,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. var checksums s3response.Checksum var cType types.ChecksumType if input.ChecksumMode == types.ChecksumModeEnabled { - checksums, err = p.retreiveChecksums(nil, bucket, object) + checksums, err = p.retrieveChecksums(nil, bucket, object) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return nil, fmt.Errorf("get object checksums: %w", err) } @@ -3905,7 +3905,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3. } } - checksums, err := p.retreiveChecksums(nil, dstBucket, dstObject) + checksums, err := p.retrieveChecksums(nil, dstBucket, dstObject) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return nil, fmt.Errorf("get obj checksums: %w", err) } @@ -3968,7 +3968,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3. } else { contentLength := fi.Size() - checksums, err := p.retreiveChecksums(f, srcBucket, srcObject) + checksums, err := p.retrieveChecksums(f, srcBucket, srcObject) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return nil, fmt.Errorf("get obj checksum: %w", err) } @@ -4111,7 +4111,7 @@ func (p *Posix) fileToObj(bucket string) backend.GetObjFunc { } // Retreive the object checksum algorithm - checksums, err := p.retreiveChecksums(nil, bucket, path) + checksums, err := p.retrieveChecksums(nil, bucket, path) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return s3response.Object{}, backend.ErrSkipObj } @@ -4787,7 +4787,7 @@ func (p *Posix) storeChecksums(f *os.File, bucket, object string, chs s3response return p.meta.StoreAttribute(f, bucket, object, checksumsKey, checksums) } -func (p *Posix) retreiveChecksums(f *os.File, bucket, object string) (checksums s3response.Checksum, err error) { +func (p *Posix) retrieveChecksums(f *os.File, bucket, object string) (checksums s3response.Checksum, err error) { checksumsAtr, err := p.meta.RetrieveAttribute(f, bucket, object, checksumsKey) if err != nil { return checksums, err From e5811e4ce756bb72de49356b09137a06ae54b450 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Thu, 20 Feb 2025 15:45:42 +0400 Subject: [PATCH 2/2] fix: Fixes the entity limiter validation for ListObjects(V2), ListParts, ListMultipartUploads, ListBuckets actions --- backend/posix/posix.go | 24 +++---- s3api/controllers/base.go | 26 ++++---- s3api/utils/utils.go | 10 ++- s3api/utils/utils_test.go | 8 +++ tests/integration/group-tests.go | 5 ++ tests/integration/tests.go | 105 +++++++++++++++++++++++++++++++ 6 files changed, 154 insertions(+), 24 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 2d0a0dc9..7f1bc402 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -2008,10 +2008,7 @@ func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUpl } } - maxUploads := 0 - if mpu.MaxUploads != nil { - maxUploads = int(*mpu.MaxUploads) - } + maxUploads := int(*mpu.MaxUploads) if (uploadIDMarker != "" && !uploadIdMarkerFound) || (keyMarker != "" && keyMarkerInd == -1) { return s3response.ListMultipartUploadsResult{ Bucket: bucket, @@ -2064,7 +2061,7 @@ func (p *Posix) ListMultipartUploads(_ context.Context, mpu *s3.ListMultipartUpl }, nil } -func (p *Posix) ListParts(_ context.Context, input *s3.ListPartsInput) (s3response.ListPartsResult, error) { +func (p *Posix) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3response.ListPartsResult, error) { var lpr s3response.ListPartsResult if input.Bucket == nil { @@ -2084,10 +2081,8 @@ func (p *Posix) ListParts(_ context.Context, input *s3.ListPartsInput) (s3respon if input.PartNumberMarker != nil { stringMarker = *input.PartNumberMarker } - maxParts := 0 - if input.MaxParts != nil { - maxParts = int(*input.MaxParts) - } + + maxParts := int(*input.MaxParts) var partNumberMarker int if stringMarker != "" { @@ -2127,8 +2122,15 @@ func (p *Posix) ListParts(_ context.Context, input *s3.ListPartsInput) (s3respon return lpr, fmt.Errorf("get mp checksum: %w", err) } - var parts []s3response.Part - for _, e := range ents { + parts := make([]s3response.Part, 0, len(ents)) + for i, e := range ents { + if i%128 == 0 { + select { + case <-ctx.Done(): + return s3response.ListPartsResult{}, ctx.Err() + default: + } + } pn, err := strconv.Atoi(e.Name()) if err != nil { // file is not a valid part file diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 9a496fbd..d1d1ee07 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -74,24 +74,28 @@ func (c S3ApiController) ListBuckets(ctx *fiber.Ctx) error { maxBucketsStr := ctx.Query("max-buckets") acct := ctx.Locals("account").(auth.Account) - maxBuckets, err := utils.ParseUint(maxBucketsStr) - if err != nil || maxBuckets > 10000 { - if c.debug { - log.Printf("error parsing max-buckets %q: %v\n", maxBucketsStr, err) + var maxBuckets int32 = 10000 + if maxBucketsStr != "" { + maxBucketsParsed, err := strconv.ParseInt(maxBucketsStr, 10, 32) + if err != nil || maxBucketsParsed < 0 || maxBucketsParsed > 10000 { + if c.debug { + log.Printf("error parsing max-buckets %q: %v\n", maxBucketsStr, err) + } + return SendXMLResponse(ctx, nil, s3err.GetAPIError(s3err.ErrInvalidMaxBuckets), + &MetaOpts{ + Logger: c.logger, + MetricsMng: c.mm, + Action: metrics.ActionListAllMyBuckets, + }) } - return SendXMLResponse(ctx, nil, s3err.GetAPIError(s3err.ErrInvalidMaxBuckets), - &MetaOpts{ - Logger: c.logger, - MetricsMng: c.mm, - Action: metrics.ActionListAllMyBuckets, - }) + maxBuckets = int32(maxBucketsParsed) } res, err := c.be.ListBuckets(ctx.Context(), s3response.ListBucketsInput{ Owner: acct.Access, IsAdmin: acct.Role == auth.RoleAdmin, - MaxBuckets: maxBuckets, + MaxBuckets: int32(maxBuckets), ContinuationToken: cToken, Prefix: prefix, }) diff --git a/s3api/utils/utils.go b/s3api/utils/utils.go index c1ace5b1..74c52faa 100644 --- a/s3api/utils/utils.go +++ b/s3api/utils/utils.go @@ -179,9 +179,15 @@ func ParseUint(str string) (int32, error) { if str == "" { return 1000, nil } - num, err := strconv.ParseUint(str, 10, 16) + num, err := strconv.ParseInt(str, 10, 32) if err != nil { - return 1000, fmt.Errorf("invalid uint: %w", err) + return 1000, fmt.Errorf("invalid int: %w", err) + } + if num < 0 { + return 1000, fmt.Errorf("negative uint: %v", num) + } + if num > 1000 { + num = 1000 } return int32(num), nil } diff --git a/s3api/utils/utils_test.go b/s3api/utils/utils_test.go index 6aa09078..9a0417ca 100644 --- a/s3api/utils/utils_test.go +++ b/s3api/utils/utils_test.go @@ -268,6 +268,14 @@ func TestParseUint(t *testing.T) { want: 23, wantErr: false, }, + { + name: "Parse-uint-greater-than-1000", + args: args{ + str: "25000000", + }, + want: 1000, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index f94dd40f..a2914d10 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -209,6 +209,7 @@ func TestListObjects(s *S3Conf) { ListObjects_paginated(s) ListObjects_invalid_max_keys(s) ListObjects_max_keys_0(s) + ListObjects_exceeding_max_keys(s) ListObjects_delimiter(s) ListObjects_max_keys_none(s) ListObjects_marker_not_from_obj_list(s) @@ -228,6 +229,7 @@ func TestListObjectsV2(s *S3Conf) { ListObjectsV2_single_dir_object_with_delim_and_prefix(s) ListObjectsV2_truncated_common_prefixes(s) ListObjectsV2_all_objs_max_keys(s) + ListObjectsV2_exceeding_max_keys(s) ListObjectsV2_list_all_objs(s) //TODO: remove the condition after implementing checksums in azure if !s.azureTests { @@ -356,6 +358,7 @@ func TestListParts(s *S3Conf) { ListParts_incorrect_object_key(s) ListParts_invalid_max_parts(s) ListParts_default_max_parts(s) + ListParts_exceeding_max_parts(s) ListParts_truncated(s) //TODO: remove the condition after implementing checksums in azure if !s.azureTests { @@ -369,6 +372,7 @@ func TestListMultipartUploads(s *S3Conf) { ListMultipartUploads_empty_result(s) ListMultipartUploads_invalid_max_uploads(s) ListMultipartUploads_max_uploads(s) + ListMultipartUploads_exceeding_max_uploads(s) ListMultipartUploads_incorrect_next_key_marker(s) ListMultipartUploads_ignore_upload_id_marker(s) //TODO: remove the condition after implementing checksums in azure @@ -961,6 +965,7 @@ func GetIntTests() IntTests { "ListMultipartUploads_empty_result": ListMultipartUploads_empty_result, "ListMultipartUploads_invalid_max_uploads": ListMultipartUploads_invalid_max_uploads, "ListMultipartUploads_max_uploads": ListMultipartUploads_max_uploads, + "ListMultipartUploads_exceeding_max_uploads": ListMultipartUploads_exceeding_max_uploads, "ListMultipartUploads_incorrect_next_key_marker": ListMultipartUploads_incorrect_next_key_marker, "ListMultipartUploads_ignore_upload_id_marker": ListMultipartUploads_ignore_upload_id_marker, "ListMultipartUploads_with_checksums": ListMultipartUploads_with_checksums, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index da3bde44..44f7febb 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -4540,6 +4540,31 @@ func ListObjects_max_keys_0(s *S3Conf) error { }) } +func ListObjects_exceeding_max_keys(s *S3Conf) error { + testName := "ListObjects_exceeding_max_keys" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + maxKeys := int32(233333333) + out, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ + Bucket: &bucket, + MaxKeys: &maxKeys, + }) + cancel() + if err != nil { + return nil + } + + if out.MaxKeys == nil { + return fmt.Errorf("unexpected nil max-keys") + } + if *out.MaxKeys != 1000 { + return fmt.Errorf("expected the max-keys to be %v, instaed got %v", 1000, *out.MaxKeys) + } + + return nil + }) +} + func ListObjects_delimiter(s *S3Conf) error { testName := "ListObjects_delimiter" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -5042,6 +5067,31 @@ func ListObjectsV2_all_objs_max_keys(s *S3Conf) error { }) } +func ListObjectsV2_exceeding_max_keys(s *S3Conf) error { + testName := "ListObjectsV2_exceeding_max_keys" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + maxKeys := int32(233453333) + out, err := s3client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{ + Bucket: &bucket, + MaxKeys: &maxKeys, + }) + cancel() + if err != nil { + return nil + } + + if out.MaxKeys == nil { + return fmt.Errorf("unexpected nil max-keys") + } + if *out.MaxKeys != 1000 { + return fmt.Errorf("expected the max-keys to be %v, instaed got %v", 1000, *out.MaxKeys) + } + + return nil + }) +} + func ListObjectsV2_list_all_objs(s *S3Conf) error { testName := "ListObjectsV2_list_all_objs" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -7981,6 +8031,36 @@ func ListParts_default_max_parts(s *S3Conf) error { }) } +func ListParts_exceeding_max_parts(s *S3Conf) error { + testName := "ListParts_exceeding_max_parts" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + mp, err := createMp(s3client, bucket, obj) + if err != nil { + return err + } + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.ListParts(ctx, &s3.ListPartsInput{ + Bucket: &bucket, + UploadId: mp.UploadId, + Key: &obj, + }) + cancel() + if err != nil { + return err + } + + if res.MaxParts == nil { + return fmt.Errorf("unexpected nil max-parts") + } + if *res.MaxParts != 1000 { + return fmt.Errorf("expected max-parts to be %v, instead got %v", 1000, *res.MaxParts) + } + + return nil + }) +} + func ListParts_truncated(s *S3Conf) error { testName := "ListParts_truncated" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -8235,6 +8315,31 @@ func ListMultipartUploads_max_uploads(s *S3Conf) error { }) } +func ListMultipartUploads_exceeding_max_uploads(s *S3Conf) error { + testName := "ListMultipartUploads_exceeding_max_uploads" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + maxUploads := int32(1343235) + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{ + Bucket: &bucket, + MaxUploads: &maxUploads, + }) + cancel() + if err != nil { + return err + } + + if res.MaxUploads == nil { + return fmt.Errorf("unexpected nil max-uploads") + } + if *res.MaxUploads != 1000 { + return fmt.Errorf("expected max-uploads to be %v, instaed got %v", 1000, *res.MaxUploads) + } + + return nil + }) +} + func ListMultipartUploads_incorrect_next_key_marker(s *S3Conf) error { testName := "ListMultipartUploads_incorrect_next_key_marker" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {