From 82592d97f4aaaaab7f67eadc20ffb64d7bdcd4d3 Mon Sep 17 00:00:00 2001
From: jonaustin09 <jonaustin460@gmail.com>
Date: Fri, 27 Sep 2024 17:52:02 -0400
Subject: [PATCH] fix: Added the versionId prop in GetObject response, when
 attempting to get the latest object version without specifying the versionId

---
 backend/posix/posix.go     | 36 +++++++++++++++++++++++++++---------
 tests/integration/tests.go | 31 +++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/backend/posix/posix.go b/backend/posix/posix.go
index 06b7c06..8d0c80b 100644
--- a/backend/posix/posix.go
+++ b/backend/posix/posix.go
@@ -92,6 +92,8 @@ const (
 	deleteMarkerKey     = "delete-marker"
 	versionIdKey        = "version-id"
 
+	nullVersionId = "null"
+
 	doFalloc   = true
 	skipFalloc = false
 )
@@ -2444,8 +2446,12 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 	if input.Range == nil {
 		return nil, s3err.GetAPIError(s3err.ErrInvalidRange)
 	}
+	var versionId string
+	if input.VersionId != nil {
+		versionId = *input.VersionId
+	}
 
-	if !p.versioningEnabled() && *input.VersionId != "" {
+	if !p.versioningEnabled() && versionId != "" {
 		//TODO: Maybe we need to return our custom error here?
 		return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
 	}
@@ -2460,7 +2466,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 	}
 
 	object := *input.Key
-	if *input.VersionId != "" {
+	if versionId != "" {
 		vId, err := p.meta.RetrieveAttribute(bucket, object, versionIdKey)
 		if errors.Is(err, fs.ErrNotExist) {
 			return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
@@ -2470,12 +2476,12 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 		}
 		if errors.Is(err, meta.ErrNoSuchKey) {
 			bucket = filepath.Join(p.versioningDir, bucket)
-			object = filepath.Join(genObjVersionKey(object), *input.VersionId)
+			object = filepath.Join(genObjVersionKey(object), versionId)
 		}
 
-		if string(vId) != *input.VersionId {
+		if string(vId) != versionId {
 			bucket = filepath.Join(p.versioningDir, bucket)
-			object = filepath.Join(genObjVersionKey(object), *input.VersionId)
+			object = filepath.Join(genObjVersionKey(object), versionId)
 		}
 	}
 
@@ -2483,7 +2489,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 
 	fi, err := os.Stat(objPath)
 	if errors.Is(err, fs.ErrNotExist) {
-		if *input.VersionId != "" {
+		if versionId != "" {
 			return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
 		}
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
@@ -2502,7 +2508,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
 
-	if *input.VersionId != "" {
+	if versionId != "" {
 		isDelMarker, err := p.isObjDeleteMarker(bucket, object)
 		if err != nil {
 			return nil, err
@@ -2577,10 +2583,22 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 			TagCount:        tagCount,
 			ContentRange:    &contentRange,
 			StorageClass:    types.StorageClassStandard,
-			VersionId:       input.VersionId,
+			VersionId:       &versionId,
 		}, nil
 	}
 
+	// If versioning is configured get the object versionId
+	if p.versioningEnabled() && versionId == "" {
+		vId, err := p.meta.RetrieveAttribute(bucket, object, versionIdKey)
+		if errors.Is(err, meta.ErrNoSuchKey) {
+			versionId = nullVersionId
+		} else if err != nil {
+			return nil, err
+		}
+
+		versionId = string(vId)
+	}
+
 	userMetaData := make(map[string]string)
 
 	contentType, contentEncoding := p.loadUserMetaData(bucket, object, userMetaData)
@@ -2622,7 +2640,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
 		TagCount:        tagCount,
 		ContentRange:    &contentRange,
 		StorageClass:    types.StorageClassStandard,
-		VersionId:       input.VersionId,
+		VersionId:       &versionId,
 		Body:            &backend.FileSectionReadCloser{R: rdr, F: f},
 	}, nil
 }
diff --git a/tests/integration/tests.go b/tests/integration/tests.go
index 9d47b8e..4c98ac8 100644
--- a/tests/integration/tests.go
+++ b/tests/integration/tests.go
@@ -10811,6 +10811,7 @@ func Versioning_GetObject_success(s *S3Conf) error {
 			return err
 		}
 
+		// Get the object by versionId
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		out, err := s3client.GetObject(ctx, &s3.GetObjectInput{
 			Bucket:    &bucket,
@@ -10833,12 +10834,42 @@ func Versioning_GetObject_success(s *S3Conf) error {
 		if err != nil {
 			return err
 		}
+		defer out.Body.Close()
 
 		outCsum := sha256.Sum256(bdy)
 		if outCsum != r.csum {
 			return fmt.Errorf("incorrect output content")
 		}
 
+		// Get the object without versionId
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		out, err = s3client.GetObject(ctx, &s3.GetObjectInput{
+			Bucket: &bucket,
+			Key:    &obj,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if *out.ContentLength != dLen {
+			return fmt.Errorf("expected the object content-length to be %v, instead got %v", dLen, *out.ContentLength)
+		}
+		if *out.VersionId != *r.res.VersionId {
+			return fmt.Errorf("expected the versionId to be %v, instead got %v", *r.res.VersionId, *out.VersionId)
+		}
+
+		bdy, err = io.ReadAll(out.Body)
+		if err != nil {
+			return err
+		}
+		defer out.Body.Close()
+
+		outCsum = sha256.Sum256(bdy)
+		if outCsum != r.csum {
+			return fmt.Errorf("incorrect output content")
+		}
+
 		return nil
 	}, withVersioning())
 }