fix: NotImplemented for GetObject/HeadObject PartNumber

Fixes #1520

Removes the incorrect logic for HeadObject returning successful response, when querying an incomplete multipart upload.

Implements the logic to return `NotImplemented` error if `GetObject`/`HeadObject` is attempted with `partNumber` in azure and posix backends. The front-end part is preserved to be used in s3 proxy backend.

backend/azure/azure.go

@@ -408,6 +408,11 @@ func (az *Azure) DeleteBucketTagging(ctx context.Context, bucket string) error {
 }
 
 func (az *Azure) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.GetObjectOutput, error) {
+	if input.PartNumber != nil {
+		// querying an object with part number is not supported
+		return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
+	}
+
 	client, err := az.getBlobClient(*input.Bucket, *input.Key)
 	if err != nil {
 		return nil, err
@@ -482,48 +487,8 @@ func (az *Azure) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.G
 
 func (az *Azure) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) {
 	if input.PartNumber != nil {
-		client, err := az.getBlockBlobClient(*input.Bucket, *input.Key)
-		if err != nil {
-			return nil, err
-		}
-
-		res, err := client.GetBlockList(ctx, blockblob.BlockListTypeUncommitted, nil)
-		if err != nil {
-			return nil, azureErrToS3Err(err)
-		}
-
-		if res.ETag != nil && res.LastModified != nil {
-			err = backend.EvaluatePreconditions(convertAzureEtag(res.ETag), *res.LastModified,
-				backend.PreConditions{
-					IfMatch:       input.IfMatch,
-					IfNoneMatch:   input.IfNoneMatch,
-					IfModSince:    input.IfModifiedSince,
-					IfUnmodeSince: input.IfUnmodifiedSince,
-				})
-			if err != nil {
-				return nil, err
-			}
-		}
-
-		partsCount := int32(len(res.UncommittedBlocks))
-
-		for _, block := range res.UncommittedBlocks {
-			partNumber, err := decodeBlockId(*block.Name)
-			if err != nil {
-				return nil, err
-			}
-
-			if partNumber == int(*input.PartNumber) {
-				return &s3.HeadObjectOutput{
-					ContentLength: block.Size,
-					ETag:          block.Name,
-					PartsCount:    &partsCount,
-					StorageClass:  types.StorageClassStandard,
-				}, nil
-			}
-		}
-
-		return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
+		// querying an object with part number is not supported
+		return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
 	}
 
 	client, err := az.getBlobClient(*input.Bucket, *input.Key)

backend/posix/posix.go

@@ -1853,18 +1853,6 @@ func (p *Posix) checkUploadIDExists(bucket, object, uploadID string) ([32]byte,
 	return sum, nil
 }
 
-func (p *Posix) retrieveUploadId(bucket, object string) (string, [32]byte, error) {
-	sum := sha256.Sum256([]byte(object))
-	objdir := filepath.Join(bucket, MetaTmpMultipartDir, fmt.Sprintf("%x", sum))
-
-	entries, err := os.ReadDir(objdir)
-	if err != nil || len(entries) == 0 {
-		return "", [32]byte{}, s3err.GetAPIError(s3err.ErrNoSuchKey)
-	}
-
-	return entries[0].Name(), sum, nil
-}
-
 type objectMetadata struct {
 	ContentType        *string
 	ContentEncoding    *string
@@ -3413,6 +3401,11 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 		versionId = *input.VersionId
 	}
 
+	if input.PartNumber != nil {
+		// querying an object by part number is not supported
+		return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
+	}
+
 	if !p.versioningEnabled() && versionId != "" {
 		//TODO: Maybe we need to return our custom error here?
 		return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
@@ -3658,6 +3651,11 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.
 	}
 	versionId := backend.GetStringFromPtr(input.VersionId)
 
+	if input.PartNumber != nil {
+		// querying an object by part number is not supported
+		return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
+	}
+
 	if !p.versioningEnabled() && versionId != "" {
 		//TODO: Maybe we need to return our custom error here?
 		return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
@@ -3666,77 +3664,6 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.
 	bucket := *input.Bucket
 	object := *input.Key
 
-	if input.PartNumber != nil {
-		uploadId, sum, err := p.retrieveUploadId(bucket, object)
-		if err != nil {
-			return nil, err
-		}
-
-		ents, err := os.ReadDir(filepath.Join(bucket, MetaTmpMultipartDir, fmt.Sprintf("%x", sum), uploadId))
-		if errors.Is(err, fs.ErrNotExist) {
-			return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
-		}
-		if err != nil {
-			return nil, fmt.Errorf("read parts: %w", err)
-		}
-
-		partPath := filepath.Join(MetaTmpMultipartDir, fmt.Sprintf("%x", sum), uploadId, fmt.Sprintf("%v", *input.PartNumber))
-
-		part, err := os.Stat(filepath.Join(bucket, partPath))
-		if errors.Is(err, fs.ErrNotExist) {
-			return nil, s3err.GetAPIError(s3err.ErrInvalidPart)
-		}
-		if errors.Is(err, syscall.ENAMETOOLONG) {
-			return nil, s3err.GetAPIError(s3err.ErrKeyTooLong)
-		}
-		if err != nil {
-			return nil, fmt.Errorf("stat part: %w", err)
-		}
-
-		size := part.Size()
-
-		startOffset, length, isValid, err := backend.ParseObjectRange(size, getString(input.Range))
-		if err != nil {
-			return nil, err
-		}
-
-		// retreive the part etag
-		b, err := p.meta.RetrieveAttribute(nil, bucket, partPath, etagkey)
-		etag := string(b)
-		if err != nil {
-			etag = ""
-		}
-
-		// evaluate preconditions
-		err = backend.EvaluatePreconditions(etag, part.ModTime(), backend.PreConditions{
-			IfMatch:       input.IfMatch,
-			IfNoneMatch:   input.IfNoneMatch,
-			IfModSince:    input.IfModifiedSince,
-			IfUnmodeSince: input.IfUnmodifiedSince,
-		})
-		if err != nil {
-			return nil, err
-		}
-
-		var contentRange string
-		if isValid {
-			contentRange = fmt.Sprintf("bytes %v-%v/%v",
-				startOffset, startOffset+length-1, size)
-		}
-
-		partsCount := int32(len(ents))
-
-		return &s3.HeadObjectOutput{
-			AcceptRanges:  backend.GetPtrFromString("bytes"),
-			LastModified:  backend.GetTimePtr(part.ModTime()),
-			ETag:          &etag,
-			PartsCount:    &partsCount,
-			ContentLength: &length,
-			StorageClass:  types.StorageClassStandard,
-			ContentRange:  &contentRange,
-		}, nil
-	}
-
 	_, err := os.Stat(bucket)
 	if errors.Is(err, fs.ErrNotExist) {
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket)

s3api/controllers/object-get.go

@@ -380,6 +380,7 @@ func (c S3ApiController) GetObject(ctx *fiber.Ctx) (*Response, error) {
 	versionId := ctx.Query("versionId")
 	acceptRange := ctx.Get("Range")
 	checksumMode := types.ChecksumMode(ctx.Get("x-amz-checksum-mode"))
+	partNumberQuery := int32(ctx.QueryInt("partNumber", -1))
 
 	// Extract response override query parameters
 	responseOverrides := map[string]*string{
@@ -440,6 +441,20 @@ func (c S3ApiController) GetObject(ctx *fiber.Ctx) (*Response, error) {
 		}, err
 	}
 
+	var partNumber *int32
+	if ctx.Request().URI().QueryArgs().Has("partNumber") {
+		if partNumberQuery < minPartNumber || partNumberQuery > maxPartNumber {
+			debuglogger.Logf("invalid part number: %d", partNumberQuery)
+			return &Response{
+				MetaOpts: &MetaOptions{
+					BucketOwner: parsedAcl.Owner,
+				},
+			}, s3err.GetAPIError(s3err.ErrInvalidPartNumber)
+		}
+
+		partNumber = &partNumberQuery
+	}
+
 	// validate the checksum mode
 	if checksumMode != "" && checksumMode != types.ChecksumModeEnabled {
 		debuglogger.Logf("invalid x-amz-checksum-mode header value: %v", checksumMode)
@@ -462,6 +477,7 @@ func (c S3ApiController) GetObject(ctx *fiber.Ctx) (*Response, error) {
 		IfUnmodifiedSince: conditionalHeaders.IfUnmodeSince,
 		VersionId:         &versionId,
 		ChecksumMode:      checksumMode,
+		PartNumber:        partNumber,
 	})
 	if err != nil {
 		var headers map[string]*string

s3api/controllers/object-get_test.go

@@ -710,6 +710,23 @@ func TestS3ApiController_GetObject(t *testing.T) {
 				err: s3err.GetInvalidChecksumHeaderErr("x-amz-checksum-mode"),
 			},
 		},
+		{
+			name: "invalid part number",
+			input: testInput{
+				locals: defaultLocals,
+				queries: map[string]string{
+					"partNumber": "-2",
+				},
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{
+						BucketOwner: "root",
+					},
+				},
+				err: s3err.GetAPIError(s3err.ErrInvalidPartNumber),
+			},
+		},
 		{
 			name: "backend returns error",
 			input: testInput{
@@ -733,8 +750,6 @@ func TestS3ApiController_GetObject(t *testing.T) {
 				err: s3err.GetAPIError(s3err.ErrInvalidAccessKeyID),
 			},
 		},
-		// TODO: add a test case for overflowing content-length
-		// simulate a 32 bit arch to test the case
 		{
 			name: "successful response",
 			input: testInput{

tests/integration/group-tests.go

@@ -171,8 +171,7 @@ func TestPutObject(s *S3Conf) {
 func TestHeadObject(s *S3Conf) {
 	HeadObject_non_existing_object(s)
 	HeadObject_invalid_part_number(s)
-	HeadObject_non_existing_mp(s)
-	HeadObject_mp_success(s)
+	HeadObject_part_number_not_supported(s)
 	HeadObject_directory_object_noslash(s)
 	HeadObject_non_existing_dir_object(s)
 	HeadObject_invalid_parent_dir(s)
@@ -222,6 +221,8 @@ func TestGetObject(s *S3Conf) {
 	GetObject_overrides_success(s)
 	GetObject_overrides_presign_success(s)
 	GetObject_overrides_fail_public(s)
+	GetObject_invalid_part_number(s)
+	GetObject_part_number_not_supported(s)
 }
 
 func TestListObjects(s *S3Conf) {
@@ -1117,8 +1118,7 @@ func GetIntTests() IntTests {
 		"PutObject_racey_success":                                                 PutObject_racey_success,
 		"HeadObject_non_existing_object":                                          HeadObject_non_existing_object,
 		"HeadObject_invalid_part_number":                                          HeadObject_invalid_part_number,
-		"HeadObject_non_existing_mp":                                              HeadObject_non_existing_mp,
-		"HeadObject_mp_success":                                                   HeadObject_mp_success,
+		"HeadObject_part_number_not_supported":                                    HeadObject_part_number_not_supported,
 		"HeadObject_directory_object_noslash":                                     HeadObject_directory_object_noslash,
 		"HeadObject_non_existing_dir_object":                                      HeadObject_non_existing_dir_object,
 		"HeadObject_name_too_long":                                                HeadObject_name_too_long,
@@ -1154,6 +1154,8 @@ func GetIntTests() IntTests {
 		"GetObject_overrides_success":                                             GetObject_overrides_success,
 		"GetObject_overrides_presign_success":                                     GetObject_overrides_presign_success,
 		"GetObject_overrides_fail_public":                                         GetObject_overrides_fail_public,
+		"GetObject_invalid_part_number":                                           GetObject_invalid_part_number,
+		"GetObject_part_number_not_supported":                                     GetObject_part_number_not_supported,
 		"ListObjects_non_existing_bucket":                                         ListObjects_non_existing_bucket,
 		"ListObjects_with_prefix":                                                 ListObjects_with_prefix,
 		"ListObjects_truncated":                                                   ListObjects_truncated,

tests/integration/tests.go

@@ -3569,8 +3569,8 @@ func HeadObject_invalid_part_number(s *S3Conf) error {
 	})
 }
 
-func HeadObject_non_existing_mp(s *S3Conf) error {
-	testName := "HeadObject_non_existing_mp"
+func HeadObject_part_number_not_supported(s *S3Conf) error {
+	testName := "HeadObject_part_number_not_supported"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
 		partNumber := int32(4)
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
@@ -3580,65 +3580,7 @@ func HeadObject_non_existing_mp(s *S3Conf) error {
 			PartNumber: &partNumber,
 		})
 		cancel()
-		if err := checkSdkApiErr(err, "NotFound"); err != nil {
-			return err
-		}
-		return nil
-	})
-}
-
-func HeadObject_mp_success(s *S3Conf) error {
-	testName := "HeadObject_mp_success"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		obj := "my-obj"
-		partCount, partSize := int64(5), int64(1024)
-		partNumber := int32(3)
-
-		mp, err := createMp(s3client, bucket, obj)
-		if err != nil {
-			return err
-		}
-
-		parts, _, err := uploadParts(s3client, partCount*partSize, partCount, bucket, obj, *mp.UploadId)
-		if err != nil {
-			return err
-		}
-
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		out, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{
-			Bucket:     &bucket,
-			Key:        &obj,
-			PartNumber: &partNumber,
-		})
-		cancel()
-		if err != nil {
-			return err
-		}
-
-		if out.ContentLength == nil {
-			return fmt.Errorf("expected non nil content length")
-		}
-		if *out.ContentLength != int64(partSize) {
-			return fmt.Errorf("expected content length to be %v, instead got %v",
-				partSize, *out.ContentLength)
-		}
-		if getString(out.ETag) != getString(parts[partNumber-1].ETag) {
-			return fmt.Errorf("expected ETag to be %v, instead got %v",
-				getString(parts[partNumber-1].ETag), getString(out.ETag))
-		}
-		if out.PartsCount == nil {
-			return fmt.Errorf("expected non nil parts count")
-		}
-		if *out.PartsCount != int32(partCount) {
-			return fmt.Errorf("expected part count to be %v, instead got %v",
-				partCount, *out.PartsCount)
-		}
-		if out.StorageClass != types.StorageClassStandard {
-			return fmt.Errorf("expected the storage class to be %v, instead got %v",
-				types.StorageClassStandard, out.StorageClass)
-		}
-
-		return nil
+		return checkSdkApiErr(err, "NotImplemented")
 	})
 }
 
@@ -5560,6 +5502,36 @@ func GetObject_overrides_fail_public(s *S3Conf) error {
 	}, withAnonymousClient())
 }
 
+func GetObject_invalid_part_number(s *S3Conf) error {
+	testName := "GetObject_invalid_part_number"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		defer cancel()
+		_, err := s3client.GetObject(ctx, &s3.GetObjectInput{
+			Bucket:     &bucket,
+			Key:        getPtr("obj"),
+			PartNumber: getPtr(int32(-3)),
+		})
+
+		return checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidPartNumber))
+	})
+}
+
+func GetObject_part_number_not_supported(s *S3Conf) error {
+	testName := "GetObject_part_number_not_supported"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		defer cancel()
+		_, err := s3client.GetObject(ctx, &s3.GetObjectInput{
+			Bucket:     &bucket,
+			Key:        getPtr("obj"),
+			PartNumber: getPtr(int32(3)),
+		})
+
+		return checkApiErr(err, s3err.GetAPIError(s3err.ErrNotImplemented))
+	})
+}
+
 func ListObjects_non_existing_bucket(s *S3Conf) error {
 	testName := "ListObjects_non_existing_bucket"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {