s3api: preserve remote object ETags across cache fills (#11704)

* s3api: preserve remote object ETags across cache fills

* s3api: align remote copy validators and reject stale ETags

* s3api: keep remote ETag through metadata touches, drop it after resizes

The remote-ETag check required Mtime to match RemoteMtime, so a utimens
on a cold remote object returned an empty ETag until a later cache fill
restored the remote value. It also trusted LastLocalSyncTsNs alone, so a
setattr resize that leaves chunks and the stamp untouched kept serving
the old remote ETag for different local bytes.

Require FileSize to match RemoteSize for both the synced and remote-only
paths and stop comparing modification times: content changes already
surface through cleared sync stamps, chunks, or inline content, while a
metadata-only touch leaves the remote bytes authoritative. Regression
tests cover touched cold and cached entries and resized cached entries.

---------

Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>

weed/s3api/s3api_object_handlers_conditional_read_test.go

@@ -3,6 +3,7 @@ package s3api
 import (
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 	"time"
 
@@ -58,3 +59,182 @@ func TestValidateConditionalHeadersForReadsMissingObject(t *testing.T) {
 		})
 	}
 }
+
+func TestRemoteObjectETagAcrossCacheFill(t *testing.T) {
+	s3a := &S3ApiServer{}
+	entry := &filer_pb.Entry{
+		Name:        "object",
+		Attributes:  &filer_pb.FuseAttributes{FileSize: 2048, Mtime: 100},
+		RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: 2048, RemoteMtime: 100, RemoteETag: "remote-multipart-2"},
+	}
+	head := httptest.NewRecorder()
+	s3a.setResponseHeaders(head, httptest.NewRequest(http.MethodHead, "/bucket/object", nil), entry, 2048)
+	headETag := head.Header().Get("ETag")
+	if headETag != `"remote-multipart-2"` {
+		t.Errorf("cold ETag = %q", headETag)
+	}
+	entry.Chunks = []*filer_pb.FileChunk{
+		{Offset: 0, Size: 1024, ETag: "1B2M2Y8AsgTpgAmY7PhCfg=="},
+		{Offset: 1024, Size: 1024, ETag: "1B2M2Y8AsgTpgAmY7PhCfg=="},
+	}
+	entry.RemoteEntry.LastLocalSyncTsNs = 200
+	if got := s3a.getObjectETag(entry); got != headETag {
+		t.Errorf("cached ETag = %q, cold ETag = %q", got, headETag)
+	}
+	for _, method := range []string{http.MethodGet, http.MethodHead} {
+		r := httptest.NewRequest(method, "/bucket/object", nil)
+		r.Header.Set(s3_constants.IfMatch, headETag)
+		r.Header.Set("Range", "bytes=1024-2047")
+		headers, code := parseConditionalHeaders(r)
+		if code != s3err.ErrNone {
+			t.Fatal(code)
+		}
+		result := s3a.validateConditionalHeadersForReads(r, headers, entry, "bucket", "object")
+		if result.ErrorCode != s3err.ErrNone {
+			t.Errorf("%s after cache fill: %v", method, result.ErrorCode)
+		}
+	}
+	entry.RemoteEntry.RemoteETag = `"remote-multipart-2"`
+	if got := s3a.getObjectETag(entry); got != headETag {
+		t.Errorf("quoted remote ETag = %q", got)
+	}
+	entry.Extended = map[string][]byte{s3_constants.ExtETagKey: []byte("local-etag")}
+	if got := s3a.getObjectETag(entry); got != `"local-etag"` {
+		t.Errorf("explicit ETag = %q", got)
+	}
+	entry.Extended = nil
+	entry.RemoteEntry.LastLocalSyncTsNs = 0
+	if got, want := s3a.getObjectETag(entry), s3a.calculateETagFromChunks(entry.Chunks); got != want {
+		t.Errorf("unsynced local ETag = %q, want %q", got, want)
+	}
+}
+
+// A metadata-only update such as utimens changes Mtime without touching the
+// remote bytes, so the object must keep its remote ETag; otherwise a later
+// cache fill restores it and breaks clients holding the interim value.
+func TestRemoteObjectETagMetadataOnlyTouch(t *testing.T) {
+	s3a := &S3ApiServer{}
+	for _, cached := range []bool{false, true} {
+		entry := &filer_pb.Entry{
+			Name:       "object",
+			Attributes: &filer_pb.FuseAttributes{FileSize: 2048, Mtime: 100},
+			RemoteEntry: &filer_pb.RemoteEntry{
+				RemoteSize: 2048, RemoteMtime: 100, RemoteETag: "remote-multipart-2",
+			},
+		}
+		if cached {
+			entry.Chunks = []*filer_pb.FileChunk{{Size: 2048, ETag: "1B2M2Y8AsgTpgAmY7PhCfg=="}}
+			entry.RemoteEntry.LastLocalSyncTsNs = 200
+		}
+		entry.Attributes.Mtime = 101
+		entry.Attributes.MtimeNs = 500
+		if got := s3a.getObjectETag(entry); got != `"remote-multipart-2"` {
+			t.Errorf("cached=%v touched ETag = %q", cached, got)
+		}
+	}
+}
+
+// Extending or shrinking a cached remote file through setattr changes FileSize
+// while leaving its chunks and sync stamp alone; the remote ETag no longer
+// describes the local bytes and conditional reads must not answer 304 for it.
+func TestRemoteObjectETagLocalResize(t *testing.T) {
+	s3a := &S3ApiServer{}
+	for _, size := range []uint64{0, 1024, 4096} {
+		entry := &filer_pb.Entry{
+			Name:       "object",
+			Attributes: &filer_pb.FuseAttributes{FileSize: 2048, Mtime: 100},
+			Chunks:     []*filer_pb.FileChunk{{Size: 2048, ETag: "1B2M2Y8AsgTpgAmY7PhCfg=="}},
+			RemoteEntry: &filer_pb.RemoteEntry{
+				RemoteSize: 2048, RemoteMtime: 100, RemoteETag: "remote-multipart-2",
+				LastLocalSyncTsNs: 200,
+			},
+		}
+		entry.Attributes.FileSize = size
+		if got := s3a.getObjectETag(entry); got == `"remote-multipart-2"` {
+			t.Errorf("resized to %d retained remote ETag", size)
+		}
+		r := httptest.NewRequest(http.MethodHead, "/bucket/object", nil)
+		r.Header.Set(s3_constants.IfNoneMatch, `"remote-multipart-2"`)
+		headers, code := parseConditionalHeaders(r)
+		if code != s3err.ErrNone {
+			t.Fatal(code)
+		}
+		if got := s3a.validateConditionalHeadersForReads(r, headers, entry, "bucket", "object"); got.ErrorCode != s3err.ErrNone {
+			t.Errorf("resized to %d read: %v", size, got.ErrorCode)
+		}
+	}
+}
+
+func TestRemoteObjectETagCopyConditions(t *testing.T) {
+	s3a := &S3ApiServer{}
+	entry := &filer_pb.Entry{
+		Attributes:  &filer_pb.FuseAttributes{FileSize: 2048, Mtime: 100},
+		RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: 2048, RemoteMtime: 100, RemoteETag: "remote-multipart-2"},
+	}
+	for _, cached := range []bool{false, true} {
+		if cached {
+			entry.Chunks = []*filer_pb.FileChunk{{Size: 2048, ETag: "1B2M2Y8AsgTpgAmY7PhCfg=="}}
+			entry.RemoteEntry.LastLocalSyncTsNs = 200
+		}
+		for _, condition := range []string{s3_constants.AmzCopySourceIfMatch, s3_constants.AmzCopySourceIfNoneMatch} {
+			r := httptest.NewRequest(http.MethodPut, "/bucket/copy", nil)
+			r.Header.Set(condition, s3a.getObjectETag(entry))
+			want := s3err.ErrNone
+			if condition == s3_constants.AmzCopySourceIfNoneMatch {
+				want = s3err.ErrPreconditionFailed
+			}
+			if got := s3a.validateConditionalCopyHeaders(r, entry); got != want {
+				t.Errorf("cached=%v %s: got %v, want %v", cached, condition, got, want)
+			}
+		}
+	}
+}
+
+func TestRemoteObjectETagLocalChanges(t *testing.T) {
+	s3a := &S3ApiServer{}
+	for _, content := range [][]byte{nil, []byte("new local content")} {
+		entry := &filer_pb.Entry{
+			Attributes:  &filer_pb.FuseAttributes{FileSize: uint64(len(content)), Mtime: 101},
+			Content:     content,
+			RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: 2048, RemoteMtime: 100, RemoteETag: "old-origin"},
+		}
+		if got, want := copyEntryETag(entry), strings.Trim(s3a.getObjectETag(entry), `"`); got != want {
+			t.Errorf("local copy ETag = %q, read ETag = %q", got, want)
+		}
+		if got := s3a.getObjectETag(entry); got == `"old-origin"` {
+			t.Errorf("local content %q retained remote ETag", content)
+		}
+		r := httptest.NewRequest(http.MethodHead, "/bucket/object", nil)
+		r.Header.Set(s3_constants.IfNoneMatch, `"old-origin"`)
+		headers, code := parseConditionalHeaders(r)
+		if code != s3err.ErrNone {
+			t.Fatal(code)
+		}
+		if got := s3a.validateConditionalHeadersForReads(r, headers, entry, "bucket", "object"); got.ErrorCode != s3err.ErrNone {
+			t.Errorf("local content %q read: %v", content, got.ErrorCode)
+		}
+		r.Header.Set(s3_constants.AmzCopySourceIfMatch, `"old-origin"`)
+		if got := s3a.validateConditionalCopyHeaders(r, entry); got != s3err.ErrPreconditionFailed {
+			t.Errorf("local content %q copy: %v", content, got)
+		}
+	}
+}
+
+func TestRemoteObjectETagFallback(t *testing.T) {
+	s3a := &S3ApiServer{}
+	entry := &filer_pb.Entry{
+		Attributes:  &filer_pb.FuseAttributes{Md5: []byte{0xab, 0xcd}},
+		RemoteEntry: &filer_pb.RemoteEntry{},
+	}
+	if got := s3a.getObjectETag(entry); got != `"abcd"` {
+		t.Errorf("missing remote ETag = %q", got)
+	}
+	entry.RemoteEntry.RemoteETag = "empty-origin"
+	if got := s3a.getObjectETag(entry); got != `"empty-origin"` {
+		t.Errorf("empty remote object ETag = %q", got)
+	}
+	entry.RemoteEntry = nil
+	if got := s3a.getObjectETag(entry); got != `"abcd"` {
+		t.Errorf("local object ETag = %q", got)
+	}
+}

weed/s3api/s3api_object_handlers_copy.go

@@ -601,27 +601,7 @@ 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)
-		}
-	}
-	attr := filer.Attr{}
-	if entry.Attributes != nil {
-		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,
-		}
-	}
-	return filer.ETagEntry(&filer.Entry{
-		Attr:    attr,
-		Chunks:  entry.Chunks,
-		Content: entry.Content,
-		Remote:  entry.RemoteEntry,
-	})
+	return strings.Trim(objectETag(entry), `"`)
 }
 
 func copyEntryToTarget(dst, src *filer_pb.Entry) {

weed/s3api/s3api_object_handlers_put.go

@@ -2594,6 +2594,10 @@ func parseConditionalHeaders(r *http.Request) (conditionalHeaders, s3err.ErrorCo
 
 // S3ApiServer implements EntryGetter interface
 func (s3a *S3ApiServer) getObjectETag(entry *filer_pb.Entry) string {
+	return objectETag(entry)
+}
+
+func objectETag(entry *filer_pb.Entry) string {
 	// Try to get ETag from Extended attributes first
 	if etagBytes, hasETag := entry.Extended[s3_constants.ExtETagKey]; hasETag {
 		etag := string(etagBytes)
@@ -2605,13 +2609,27 @@ func (s3a *S3ApiServer) getObjectETag(entry *filer_pb.Entry) string {
 		}
 		// Empty stored ETag — fall through to Md5/chunk-based calculation
 	}
+	// The remote ETag still describes this entry only while the local copy
+	// holds the remote bytes: a synced cache fill, or a remote-only entry
+	// with no local content. Content changes made through the filer clear
+	// the sync stamp (Filer.UpdateEntry) or add chunks or inline content;
+	// resizes without new bytes show up in FileSize. Metadata-only updates
+	// like utimens keep the remote bytes, so Mtime is not part of the check.
+	if remote := entry.RemoteEntry; remote != nil && remote.RemoteETag != "" &&
+		entry.GetAttributes().GetFileSize() == uint64(remote.RemoteSize) &&
+		(remote.LastLocalSyncTsNs > 0 || (len(entry.Chunks) == 0 && len(entry.Content) == 0)) {
+		return quoteETag(remote.RemoteETag)
+	}
 	// Check for Md5 in Attributes (matches filer.ETag behavior)
 	// Note: len(nil slice) == 0 in Go, so no need for explicit nil check
 	if entry.Attributes != nil && len(entry.Attributes.Md5) > 0 {
 		return fmt.Sprintf("\"%x\"", entry.Attributes.Md5)
 	}
 	// Fallback: calculate ETag from chunks
-	return s3a.calculateETagFromChunks(entry.Chunks)
+	if len(entry.Chunks) == 0 {
+		return `""`
+	}
+	return quoteETag(filer.ETagChunks(entry.Chunks))
 }
 
 func (s3a *S3ApiServer) etagMatches(headerValue, objectETag string) bool {