Fix #7305: Return 400 BadDigest instead of 500 InternalError for MD5 mismatch (#7306)

When an S3 upload has a mismatched Content-MD5 header, SeaweedFS was
incorrectly returning a 500 Internal Server Error instead of the proper
400 Bad Request with error code BadDigest (per AWS S3 specification).

Changes:
- Created weed/util/constants/filer.go with error message constants
- Added ErrMsgBadDigest constant for MD5 mismatch errors
- Added ErrMsgOperationNotPermitted constant for WORM permission errors
- Added ErrBadDigest error code with proper 400 status code mapping
- Updated filerErrorToS3Error() to detect MD5 mismatch and return ErrBadDigest
- Updated filer autoChunk() to return 400 Bad Request for MD5 mismatch
- Refactored error handling to use switch statement for better readability
- Ordered error checks with exact matches first for better maintainability
- Updated all error handling to use centralized constants
- Added comprehensive unit tests

All error messages now use constants from a single location for better
maintainability and consistency. Constants placed in util package to avoid
architectural dependency issues.

Fixes #7305

7 files changed, 82 insertions, 10 deletions

diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go
index 17fceb8d2..cbd8da54f 100644
--- a/weed/s3api/s3api_object_handlers_put.go
+++ b/weed/s3api/s3api_object_handlers_put.go
@@ -20,6 +20,7 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
 	"github.com/seaweedfs/seaweedfs/weed/security"
 	weed_server "github.com/seaweedfs/seaweedfs/weed/server"
+	"github.com/seaweedfs/seaweedfs/weed/util/constants"
 	stats_collect "github.com/seaweedfs/seaweedfs/weed/stats"
 )
 
@@ -380,6 +381,8 @@ func setEtag(w http.ResponseWriter, etag string) {
 
 func filerErrorToS3Error(errString string) s3err.ErrorCode {
 	switch {
+	case errString == constants.ErrMsgBadDigest:
+		return s3err.ErrBadDigest
 	case strings.HasPrefix(errString, "existing ") && strings.HasSuffix(errString, "is a directory"):
 		return s3err.ErrExistingObjectIsDirectory
 	case strings.HasSuffix(errString, "is a file"):
diff --git a/weed/s3api/s3api_object_handlers_put_test.go b/weed/s3api/s3api_object_handlers_put_test.go
new file mode 100644
index 000000000..87b874e1f
--- /dev/null
+++ b/weed/s3api/s3api_object_handlers_put_test.go
@@ -0,0 +1,46 @@
+package s3api
+
+import (
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
+	"github.com/seaweedfs/seaweedfs/weed/util/constants"
+)
+
+func TestFilerErrorToS3Error(t *testing.T) {
+	tests := []struct {
+		name        string
+		errString   string
+		expectedErr s3err.ErrorCode
+	}{
+		{
+			name:        "MD5 mismatch error",
+			errString:   constants.ErrMsgBadDigest,
+			expectedErr: s3err.ErrBadDigest,
+		},
+		{
+			name:        "Directory exists error",
+			errString:   "existing /path/to/file is a directory",
+			expectedErr: s3err.ErrExistingObjectIsDirectory,
+		},
+		{
+			name:        "File exists error",
+			errString:   "/path/to/file is a file",
+			expectedErr: s3err.ErrExistingObjectIsFile,
+		},
+		{
+			name:        "Unknown error",
+			errString:   "some random error",
+			expectedErr: s3err.ErrInternalError,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := filerErrorToS3Error(tt.errString)
+			if result != tt.expectedErr {
+				t.Errorf("filerErrorToS3Error(%q) = %v, want %v", tt.errString, result, tt.expectedErr)
+			}
+		})
+	}
+}
diff --git a/weed/s3api/s3err/s3-error.go b/weed/s3api/s3err/s3-error.go
index b87764742..c5e515abd 100644
--- a/weed/s3api/s3err/s3-error.go
+++ b/weed/s3api/s3err/s3-error.go
@@ -1,5 +1,7 @@
 package s3err
 
+import "github.com/seaweedfs/seaweedfs/weed/util/constants"
+
 /*
  * MinIO Go Library for Amazon S3 Compatible Cloud Storage
  * Copyright 2015-2017 MinIO, Inc.
@@ -21,7 +23,7 @@ package s3err
 // http://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html
 var s3ErrorResponseMap = map[string]string{
 	"AccessDenied":                      "Access Denied.",
-	"BadDigest":                         "The Content-Md5 you specified did not match what we received.",
+	"BadDigest":                         constants.ErrMsgBadDigest,
 	"EntityTooSmall":                    "Your proposed upload is smaller than the minimum allowed object size.",
 	"EntityTooLarge":                    "Your proposed upload exceeds the maximum allowed object size.",
 	"IncompleteBody":                    "You did not provide the number of bytes specified by the Content-Length HTTP header.",
diff --git a/weed/s3api/s3err/s3api_errors.go b/weed/s3api/s3err/s3api_errors.go
index 3da79e817..24f8e1b56 100644
--- a/weed/s3api/s3err/s3api_errors.go
+++ b/weed/s3api/s3err/s3api_errors.go
@@ -4,6 +4,8 @@ import (
 	"encoding/xml"
 	"fmt"
 	"net/http"
+
+	"github.com/seaweedfs/seaweedfs/weed/util/constants"
 )
 
 // APIError structure
@@ -59,6 +61,7 @@ const (
 	ErrInvalidBucketName
 	ErrInvalidBucketState
 	ErrInvalidDigest
+	ErrBadDigest
 	ErrInvalidMaxKeys
 	ErrInvalidMaxUploads
 	ErrInvalidMaxParts
@@ -187,6 +190,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
 		Description:    "The Content-Md5 you specified is not valid.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
+	ErrBadDigest: {
+		Code:           "BadDigest",
+		Description:    constants.ErrMsgBadDigest,
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 	ErrInvalidMaxUploads: {
 		Code:           "InvalidArgument",
 		Description:    "Argument max-uploads must be an integer between 0 and 2147483647",
diff --git a/weed/server/filer_server_handlers_write.go b/weed/server/filer_server_handlers_write.go
index 923f2c0eb..1535ba207 100644
--- a/weed/server/filer_server_handlers_write.go
+++ b/weed/server/filer_server_handlers_write.go
@@ -15,6 +15,7 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/operation"
 	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
 	"github.com/seaweedfs/seaweedfs/weed/security"
+	"github.com/seaweedfs/seaweedfs/weed/util/constants"
 	"github.com/seaweedfs/seaweedfs/weed/stats"
 	"github.com/seaweedfs/seaweedfs/weed/storage/needle"
 	"github.com/seaweedfs/seaweedfs/weed/util"
@@ -168,7 +169,7 @@ func (fs *FilerServer) move(ctx context.Context, w http.ResponseWriter, r *http.
 		return
 	} else if wormEnforced {
 		// you cannot move a worm file or directory
-		err = fmt.Errorf("cannot move write-once entry from '%s' to '%s': operation not permitted", src, dst)
+		err = fmt.Errorf("cannot move write-once entry from '%s' to '%s': %s", src, dst, constants.ErrMsgOperationNotPermitted)
 		writeJsonError(w, r, http.StatusForbidden, err)
 		return
 	}
@@ -228,7 +229,7 @@ func (fs *FilerServer) DeleteHandler(w http.ResponseWriter, r *http.Request) {
 		writeJsonError(w, r, http.StatusInternalServerError, err)
 		return
 	} else if wormEnforced {
-		writeJsonError(w, r, http.StatusForbidden, errors.New("operation not permitted"))
+		writeJsonError(w, r, http.StatusForbidden, errors.New(constants.ErrMsgOperationNotPermitted))
 		return
 	}
 
diff --git a/weed/server/filer_server_handlers_write_autochunk.go b/weed/server/filer_server_handlers_write_autochunk.go
index 0d6462c11..46fa2519d 100644
--- a/weed/server/filer_server_handlers_write_autochunk.go
+++ b/weed/server/filer_server_handlers_write_autochunk.go
@@ -20,6 +20,7 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/operation"
 	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
+	"github.com/seaweedfs/seaweedfs/weed/util/constants"
 	"github.com/seaweedfs/seaweedfs/weed/storage/needle"
 	"github.com/seaweedfs/seaweedfs/weed/util"
 )
@@ -50,13 +51,17 @@ func (fs *FilerServer) autoChunk(ctx context.Context, w http.ResponseWriter, r *
 		reply, md5bytes, err = fs.doPutAutoChunk(ctx, w, r, chunkSize, contentLength, so)
 	}
 	if err != nil {
-		if err.Error() == "operation not permitted" {
+		errStr := err.Error()
+		switch {
+		case errStr == constants.ErrMsgOperationNotPermitted:
 			writeJsonError(w, r, http.StatusForbidden, err)
-		} else if strings.HasPrefix(err.Error(), "read input:") || err.Error() == io.ErrUnexpectedEOF.Error() {
+		case strings.HasPrefix(errStr, "read input:") || errStr == io.ErrUnexpectedEOF.Error():
 			writeJsonError(w, r, util.HttpStatusCancelled, err)
-		} else if strings.HasSuffix(err.Error(), "is a file") || strings.HasSuffix(err.Error(), "already exists") {
+		case strings.HasSuffix(errStr, "is a file") || strings.HasSuffix(errStr, "already exists"):
 			writeJsonError(w, r, http.StatusConflict, err)
-		} else {
+		case errStr == constants.ErrMsgBadDigest:
+			writeJsonError(w, r, http.StatusBadRequest, err)
+		default:
 			writeJsonError(w, r, http.StatusInternalServerError, err)
 		}
 	} else if reply != nil {
@@ -110,7 +115,7 @@ func (fs *FilerServer) doPostAutoChunk(ctx context.Context, w http.ResponseWrite
 	headerMd5 := r.Header.Get("Content-Md5")
 	if headerMd5 != "" && !(util.Base64Encode(md5bytes) == headerMd5 || fmt.Sprintf("%x", md5bytes) == headerMd5) {
 		fs.filer.DeleteUncommittedChunks(ctx, fileChunks)
-		return nil, nil, errors.New("The Content-Md5 you specified did not match what we received.")
+		return nil, nil, errors.New(constants.ErrMsgBadDigest)
 	}
 	filerResult, replyerr = fs.saveMetaData(ctx, r, fileName, contentType, so, md5bytes, fileChunks, chunkOffset, smallContent)
 	if replyerr != nil {
@@ -142,7 +147,7 @@ func (fs *FilerServer) doPutAutoChunk(ctx context.Context, w http.ResponseWriter
 	headerMd5 := r.Header.Get("Content-Md5")
 	if headerMd5 != "" && !(util.Base64Encode(md5bytes) == headerMd5 || fmt.Sprintf("%x", md5bytes) == headerMd5) {
 		fs.filer.DeleteUncommittedChunks(ctx, fileChunks)
-		return nil, nil, errors.New("The Content-Md5 you specified did not match what we received.")
+		return nil, nil, errors.New(constants.ErrMsgBadDigest)
 	}
 	filerResult, replyerr = fs.saveMetaData(ctx, r, fileName, contentType, so, md5bytes, fileChunks, chunkOffset, smallContent)
 	if replyerr != nil {
@@ -171,7 +176,7 @@ func (fs *FilerServer) checkPermissions(ctx context.Context, r *http.Request, fi
 		return err
 	} else if enforced {
 		// you cannot change a worm file
-		return errors.New("operation not permitted")
+		return errors.New(constants.ErrMsgOperationNotPermitted)
 	}
 
 	return nil
diff --git a/weed/util/constants/filer.go b/weed/util/constants/filer.go
new file mode 100644
index 000000000..f5f240e76
--- /dev/null
+++ b/weed/util/constants/filer.go
@@ -0,0 +1,7 @@
+package constants
+
+// Filer error messages
+const (
+	ErrMsgOperationNotPermitted = "operation not permitted"
+	ErrMsgBadDigest             = "The Content-Md5 you specified did not match what we received."
+)