From f3e4a736961d07a32810eb5d91142ff055be3dc1 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 14 Jul 2026 11:53:53 -0700 Subject: [PATCH] s3: return NoSuchKey/NoSuchBucket for a missing CopyObject source (#10332) * s3: CopyObject returns NoSuchKey for a missing copy source * s3: CopyObject returns NoSuchBucket for a missing source bucket --- weed/s3api/s3api_copy_source_classify_test.go | 14 +++++------ weed/s3api/s3api_object_handlers_copy.go | 24 +++++++++---------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/weed/s3api/s3api_copy_source_classify_test.go b/weed/s3api/s3api_copy_source_classify_test.go index 7a40a9051..9eeb3b8a6 100644 --- a/weed/s3api/s3api_copy_source_classify_test.go +++ b/weed/s3api/s3api_copy_source_classify_test.go @@ -24,14 +24,14 @@ func TestClassifyCopySourceError(t *testing.T) { want s3err.ErrorCode }{ {"regular entry", &filer_pb.Entry{Name: "o"}, nil, s3err.ErrNone}, - {"nil entry", nil, nil, s3err.ErrInvalidCopySource}, - {"directory entry", &filer_pb.Entry{IsDirectory: true}, nil, s3err.ErrInvalidCopySource}, + {"nil entry", nil, nil, s3err.ErrNoSuchKey}, + {"directory entry", &filer_pb.Entry{IsDirectory: true}, nil, s3err.ErrNoSuchKey}, {"delete marker entry", deleteMarker, nil, s3err.ErrNoSuchKey}, - {"not found sentinel", nil, filer_pb.ErrNotFound, s3err.ErrInvalidCopySource}, - {"wrapped not found", nil, fmt.Errorf("read %s: %w", "o", filer_pb.ErrNotFound), s3err.ErrInvalidCopySource}, - {"grpc not found", nil, status.Error(codes.NotFound, "no entry"), s3err.ErrInvalidCopySource}, - {"invalid version id", nil, errInvalidVersionID, s3err.ErrInvalidCopySource}, - {"delete marker error", nil, ErrDeleteMarker, s3err.ErrInvalidCopySource}, + {"not found sentinel", nil, filer_pb.ErrNotFound, s3err.ErrNoSuchKey}, + {"wrapped not found", nil, fmt.Errorf("read %s: %w", "o", filer_pb.ErrNotFound), s3err.ErrNoSuchKey}, + {"grpc not found", nil, status.Error(codes.NotFound, "no entry"), s3err.ErrNoSuchKey}, + {"invalid version id", nil, errInvalidVersionID, s3err.ErrNoSuchKey}, + {"delete marker error", nil, ErrDeleteMarker, s3err.ErrNoSuchKey}, {"transient unavailable", nil, status.Error(codes.Unavailable, "filer down"), s3err.ErrServiceUnavailable}, {"store error", nil, errors.New("connection reset"), s3err.ErrInternalError}, // A message merely mentioning the sentinel text must not downgrade a store error. diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index e15a3098b..8e9c1090e 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/weed/s3api/s3api_object_handlers_copy.go @@ -69,16 +69,14 @@ func hasPrefixFold(s, prefix string) bool { return len(s) >= len(prefix) && strings.EqualFold(s[:len(prefix)], prefix) } -// classifyCopySourceError maps a copy-source lookup to an S3 error: a missing, -// invalid, or directory source stays a client error, but a transient store -// error becomes a retryable 5xx so a resumable copy/commit survives a blip. +// classifyCopySourceError maps a copy-source lookup to an S3 error: a missing +// or directory source is NoSuchKey like AWS, but a transient store error +// becomes a retryable 5xx so a resumable copy/commit survives a blip. func classifyCopySourceError(entry *filer_pb.Entry, err error) s3err.ErrorCode { if err == nil { if entry == nil || entry.IsDirectory { - return s3err.ErrInvalidCopySource + return s3err.ErrNoSuchKey } - // The latest-version pointer tracks delete markers too; copying one - // must be NoSuchKey like every other handler, not a copy of the stub. if deleteMarker, ok := entry.Extended[s3_constants.ExtDeleteMarkerKey]; ok && string(deleteMarker) == "true" { return s3err.ErrNoSuchKey } @@ -86,7 +84,7 @@ func classifyCopySourceError(entry *filer_pb.Entry, err error) s3err.ErrorCode { } if errors.Is(err, filer_pb.ErrNotFound) || status.Code(err) == codes.NotFound || errors.Is(err, errInvalidVersionID) || errors.Is(err, ErrDeleteMarker) { - return s3err.ErrInvalidCopySource + return s3err.ErrNoSuchKey } if isTransientFilerError(err) { return s3err.ErrServiceUnavailable @@ -171,10 +169,10 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request srcVersioningState, err := s3a.getVersioningState(srcBucket) if err != nil { glog.Errorf("Error checking versioning state for source bucket %s: %v", srcBucket, err) - // Only a missing bucket is the client's fault; a store error must stay - // retryable, matching the destination-bucket lookup. + // A missing source bucket is NoSuchBucket like AWS; a store error must + // stay retryable, matching the destination-bucket lookup. if errors.Is(err, filer_pb.ErrNotFound) { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidCopySource) + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) return } s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) @@ -870,10 +868,10 @@ func (s3a *S3ApiServer) CopyObjectPartHandler(w http.ResponseWriter, r *http.Req srcVersioningState, err := s3a.getVersioningState(srcBucket) if err != nil { glog.Errorf("Error checking versioning state for source bucket %s: %v", srcBucket, err) - // Only a missing bucket is the client's fault; a store error must stay - // retryable, matching the destination-bucket lookup. + // A missing source bucket is NoSuchBucket like AWS; a store error must + // stay retryable, matching the destination-bucket lookup. if errors.Is(err, filer_pb.ErrNotFound) { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidCopySource) + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) return } s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)