diff --git a/weed/s3api/s3api_object_handlers_copy_unified.go b/weed/s3api/s3api_object_handlers_copy_unified.go index d11594420..255c3eb2d 100644 --- a/weed/s3api/s3api_object_handlers_copy_unified.go +++ b/weed/s3api/s3api_object_handlers_copy_unified.go @@ -2,12 +2,14 @@ package s3api import ( "context" + "errors" "fmt" "net/http" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" + weed_server "github.com/seaweedfs/seaweedfs/weed/server" ) // executeUnifiedCopyStrategy executes the appropriate copy strategy based on encryption state @@ -76,6 +78,14 @@ func (s3a *S3ApiServer) mapCopyErrorToS3Error(err error) s3err.ErrorCode { return s3err.ErrNone } + // Check for read-only errors (quota enforcement) + // Uses errors.Is() to properly detect wrapped errors + if errors.Is(err, weed_server.ErrReadOnly) { + // Bucket is read-only due to quota enforcement or other configuration + // Return 403 Forbidden per S3 semantics (similar to MinIO's quota enforcement) + return s3err.ErrAccessDenied + } + // Check for KMS errors first if kmsErr := MapKMSErrorToS3Error(err); kmsErr != s3err.ErrInvalidRequest { return kmsErr diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index f7105052e..c618405ca 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -23,6 +23,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/seaweedfs/seaweedfs/weed/security" + weed_server "github.com/seaweedfs/seaweedfs/weed/server" stats_collect "github.com/seaweedfs/seaweedfs/weed/stats" "github.com/seaweedfs/seaweedfs/weed/util/constants" ) @@ -605,7 +606,7 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, uploadUrl string, dataReader s3a.deleteOrphanedChunks(chunkResult.FileChunks) } - return "", filerErrorToS3Error(createErr.Error()), SSEResponseMetadata{} + return "", filerErrorToS3Error(createErr), SSEResponseMetadata{} } glog.V(3).Infof("putToFiler: CreateEntry SUCCESS for %s", filePath) @@ -677,10 +678,21 @@ func (s3a *S3ApiServer) setSSEResponseHeaders(w http.ResponseWriter, r *http.Req } } -func filerErrorToS3Error(errString string) s3err.ErrorCode { +func filerErrorToS3Error(err error) s3err.ErrorCode { + if err == nil { + return s3err.ErrNone + } + + errString := err.Error() + switch { case errString == constants.ErrMsgBadDigest: return s3err.ErrBadDigest + case errors.Is(err, weed_server.ErrReadOnly): + // Bucket is read-only due to quota enforcement or other configuration + // Return 403 Forbidden per S3 semantics (similar to MinIO's quota enforcement) + // Uses errors.Is() to properly detect wrapped errors + return s3err.ErrAccessDenied case strings.Contains(errString, "context canceled") || strings.Contains(errString, "code = Canceled"): // Client canceled the request, return client error not server error return s3err.ErrInvalidRequest diff --git a/weed/s3api/s3api_object_handlers_put_test.go b/weed/s3api/s3api_object_handlers_put_test.go index 9144e2cee..011b73578 100644 --- a/weed/s3api/s3api_object_handlers_put_test.go +++ b/weed/s3api/s3api_object_handlers_put_test.go @@ -1,55 +1,73 @@ package s3api import ( + "errors" + "fmt" "testing" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" + weed_server "github.com/seaweedfs/seaweedfs/weed/server" "github.com/seaweedfs/seaweedfs/weed/util/constants" ) func TestFilerErrorToS3Error(t *testing.T) { tests := []struct { name string - errString string + err error expectedErr s3err.ErrorCode }{ + { + name: "nil error", + err: nil, + expectedErr: s3err.ErrNone, + }, { name: "MD5 mismatch error", - errString: constants.ErrMsgBadDigest, + err: errors.New(constants.ErrMsgBadDigest), expectedErr: s3err.ErrBadDigest, }, + { + name: "Read only error (direct)", + err: weed_server.ErrReadOnly, + expectedErr: s3err.ErrAccessDenied, + }, + { + name: "Read only error (wrapped)", + err: fmt.Errorf("create file /buckets/test/file.txt: %w", weed_server.ErrReadOnly), + expectedErr: s3err.ErrAccessDenied, + }, { name: "Context canceled error", - errString: "rpc error: code = Canceled desc = context canceled", + err: errors.New("rpc error: code = Canceled desc = context canceled"), expectedErr: s3err.ErrInvalidRequest, }, { name: "Context canceled error (simple)", - errString: "context canceled", + err: errors.New("context canceled"), expectedErr: s3err.ErrInvalidRequest, }, { name: "Directory exists error", - errString: "existing /path/to/file is a directory", + err: errors.New("existing /path/to/file is a directory"), expectedErr: s3err.ErrExistingObjectIsDirectory, }, { name: "File exists error", - errString: "/path/to/file is a file", + err: errors.New("/path/to/file is a file"), expectedErr: s3err.ErrExistingObjectIsFile, }, { name: "Unknown error", - errString: "some random error", + err: errors.New("some random error"), expectedErr: s3err.ErrInternalError, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := filerErrorToS3Error(tt.errString) + result := filerErrorToS3Error(tt.err) if result != tt.expectedErr { - t.Errorf("filerErrorToS3Error(%q) = %v, want %v", tt.errString, result, tt.expectedErr) + t.Errorf("filerErrorToS3Error(%v) = %v, want %v", tt.err, result, tt.expectedErr) } }) }