diff --git a/weed/s3api/s3api_object_handlers_conditional_read_test.go b/weed/s3api/s3api_object_handlers_conditional_read_test.go index 7946e1a9a..1c4dd2dcd 100644 --- a/weed/s3api/s3api_object_handlers_conditional_read_test.go +++ b/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) + } +} diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index b15e843cf..3d85058f8 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/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) { diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 7783029f5..3cb1caba5 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/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 {