fix(s3api): standardize ETag calculation in copy handlers (#9829)

* fix(s3api): standardize ETag calculation across S3 API handlers

* s3: make copyEntryETag nil-safe

---------

Co-authored-by: Chris Lu <chris.lu@gmail.com>

weed/s3api/s3api_object_handlers_copy.go

@@ -444,8 +444,11 @@ func cloneProtoEntry(entry *filer_pb.Entry) *filer_pb.Entry {
 	return proto.Clone(entry).(*filer_pb.Entry)
 }
 
-func copyEntryETag(fullPath util.FullPath, entry *filer_pb.Entry) string {
-	if entry != nil && entry.Extended != nil {
+func copyEntryETag(entry *filer_pb.Entry) string {
+	if entry == nil {
+		return ""
+	}
+	if entry.Extended != nil {
 		if etag, ok := entry.Extended[s3_constants.ExtETagKey]; ok && len(etag) > 0 {
 			return string(etag)
 		}
@@ -461,11 +464,10 @@ func copyEntryETag(fullPath util.FullPath, entry *filer_pb.Entry) string {
 		}
 	}
 	return filer.ETagEntry(&filer.Entry{
-		FullPath: fullPath,
-		Attr:     attr,
-		Chunks:   entry.Chunks,
-		Content:  entry.Content,
-		Remote:   entry.RemoteEntry,
+		Attr:    attr,
+		Chunks:  entry.Chunks,
+		Content: entry.Content,
+		Remote:  entry.RemoteEntry,
 	})
 }
 
@@ -494,7 +496,7 @@ func (s3a *S3ApiServer) finalizeCopyDestination(dstBucket, dstObject, dstVersion
 		dstEntry.Extended = make(map[string][]byte)
 	}
 
-	etag = copyEntryETag(dstPath, dstEntry)
+	etag = copyEntryETag(dstEntry)
 
 	switch dstVersioningState {
 	case s3_constants.VersioningEnabled:
@@ -987,19 +989,7 @@ func (s3a *S3ApiServer) CopyObjectPartHandler(w http.ResponseWriter, r *http.Req
 	}
 
 	// Calculate ETag for the part
-	partPath := util.FullPath(uploadDir + "/" + partName)
-	filerEntry := &filer.Entry{
-		FullPath: partPath,
-		Attr: filer.Attr{
-			FileSize: dstEntry.Attributes.FileSize,
-			Mtime:    time.Unix(dstEntry.Attributes.Mtime, 0),
-			Crtime:   time.Unix(dstEntry.Attributes.Crtime, 0),
-			Mime:     dstEntry.Attributes.Mime,
-		},
-		Chunks: dstEntry.Chunks,
-	}
-
-	etag := filer.ETagEntry(filerEntry)
+	etag := copyEntryETag(dstEntry)
 	setEtag(w, etag)
 
 	response := CopyPartResult{
@@ -1429,19 +1419,7 @@ func (s3a *S3ApiServer) copyChunksForRange(entry *filer_pb.Entry, startOffset, e
 
 // validateConditionalCopyHeaders validates the conditional copy headers against the source entry
 func (s3a *S3ApiServer) validateConditionalCopyHeaders(r *http.Request, entry *filer_pb.Entry) s3err.ErrorCode {
-	// Calculate ETag for the source entry. ETagEntry derives the tag from the
-	// chunks/Md5/remote-etag only, so no path is needed here.
-	filerEntry := &filer.Entry{
-		Attr: filer.Attr{
-			FileSize: entry.Attributes.FileSize,
-			Mtime:    time.Unix(entry.Attributes.Mtime, 0),
-			Crtime:   time.Unix(entry.Attributes.Crtime, 0),
-			Mime:     entry.Attributes.Mime,
-			Md5:      entry.Attributes.Md5,
-		},
-		Chunks: entry.Chunks,
-	}
-	sourceETag := filer.ETagEntry(filerEntry)
+	sourceETag := copyEntryETag(entry)
 
 	// Check X-Amz-Copy-Source-If-Match
 	if ifMatch := r.Header.Get(s3_constants.AmzCopySourceIfMatch); ifMatch != "" {

weed/s3api/s3api_object_handlers_copy_etag_test.go

@@ -0,0 +1,65 @@
+package s3api
+
+import (
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
+)
+
+func TestCopyEntryETagPrefersStoredExtendedETag(t *testing.T) {
+	storedETag := "11111111111111111111111111111111-2"
+	entry := newCopyETagTestEntry(t, storedETag, "22222222222222222222222222222222")
+
+	if got := copyEntryETag(entry); got != storedETag {
+		t.Fatalf("copyEntryETag() = %q, want stored extended ETag %q", got, storedETag)
+	}
+}
+
+func TestCopyEntryETagFallsBackToFilerETag(t *testing.T) {
+	computedETag := "33333333333333333333333333333333"
+	entry := newCopyETagTestEntry(t, "", computedETag)
+
+	if got := strings.Trim(copyEntryETag(entry), `"`); got != computedETag {
+		t.Fatalf("copyEntryETag() = %q, want fallback filer ETag %q", got, computedETag)
+	}
+}
+
+func TestValidateConditionalCopyHeadersUsesStoredExtendedETag(t *testing.T) {
+	storedETag := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-2"
+	entry := newCopyETagTestEntry(t, storedETag, "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")
+	s3a := &S3ApiServer{}
+
+	matchReq := httptest.NewRequest("PUT", "/dst", nil)
+	matchReq.Header.Set(s3_constants.AmzCopySourceIfMatch, `"`+storedETag+`"`)
+	if got := s3a.validateConditionalCopyHeaders(matchReq, entry); got != s3err.ErrNone {
+		t.Fatalf("validateConditionalCopyHeaders(If-Match stored ETag) = %v, want %v", got, s3err.ErrNone)
+	}
+
+	noneMatchReq := httptest.NewRequest("PUT", "/dst", nil)
+	noneMatchReq.Header.Set(s3_constants.AmzCopySourceIfNoneMatch, storedETag)
+	if got := s3a.validateConditionalCopyHeaders(noneMatchReq, entry); got != s3err.ErrPreconditionFailed {
+		t.Fatalf("validateConditionalCopyHeaders(If-None-Match stored ETag) = %v, want %v", got, s3err.ErrPreconditionFailed)
+	}
+}
+
+func newCopyETagTestEntry(t *testing.T, storedETag, computedETag string) *filer_pb.Entry {
+	t.Helper()
+
+	entry := &filer_pb.Entry{
+		Name: "object",
+		Attributes: &filer_pb.FuseAttributes{
+			FileSize: 5,
+			Md5:      mustDecodeHexETagForTest(t, computedETag),
+		},
+	}
+	if storedETag != "" {
+		entry.Extended = map[string][]byte{
+			s3_constants.ExtETagKey: []byte(storedETag),
+		}
+	}
+	return entry
+}