Return correct error on lock configuration check (#888)

Closes #869.
nspcc-dev · Nov 20, 2023 · 6541c61 · 6541c61
2 parents 014a5bf + a6a9094
commit 6541c61
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 7 deletions.
diff --git a/api/handler/locking.go b/api/handler/locking.go
@@ -3,6 +3,7 @@ package handler
 import (
 	"context"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -25,6 +26,11 @@ const (
 	legalHoldOff   = "OFF"
 )
 
+var (
+	errEmptyDaysErrors    = errors.New("you must specify Days or Years")
+	errNonEmptyDaysErrors = errors.New("you cannot specify Days and Years at the same time")
+)
+
 func (h *handler) PutBucketObjectLockConfigHandler(w http.ResponseWriter, r *http.Request) {
 	reqInfo := api.GetReqInfo(r.Context())
 
@@ -278,7 +284,7 @@ func (h *handler) GetObjectRetentionHandler(w http.ResponseWriter, r *http.Reque
 
 func checkLockConfiguration(conf *data.ObjectLockConfiguration) error {
 	if conf.ObjectLockEnabled != "" && conf.ObjectLockEnabled != enabledValue {
-		return fmt.Errorf("invalid ObjectLockEnabled value: %s", conf.ObjectLockEnabled)
+		return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid ObjectLockEnabled value: %s", conf.ObjectLockEnabled))
 	}
 
 	if conf.Rule == nil || conf.Rule.DefaultRetention == nil {
@@ -287,15 +293,15 @@ func checkLockConfiguration(conf *data.ObjectLockConfiguration) error {
 
 	retention := conf.Rule.DefaultRetention
 	if retention.Mode != governanceMode && retention.Mode != complianceMode {
-		return fmt.Errorf("invalid Mode value: %s", retention.Mode)
+		return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid Mode value: %s", retention.Mode))
 	}
 
-	if retention.Days == 0 && retention.Years == 0 {
-		return fmt.Errorf("you must specify Days or Years")
+	if retention.Days <= 0 && retention.Years <= 0 {
+		return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, errEmptyDaysErrors)
 	}
 
 	if retention.Days != 0 && retention.Years != 0 {
-		return fmt.Errorf("you cannot specify Days and Years at the same time")
+		return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, errNonEmptyDaysErrors)
 	}
 
 	return nil

diff --git a/api/handler/locking_test.go b/api/handler/locking_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/xml"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strconv"
@@ -282,11 +283,29 @@ func TestPutBucketLockConfigurationHandler(t *testing.T) {
 			expectedError: s3errors.GetAPIError(s3errors.ErrObjectLockConfigurationNotAllowed),
 		},
 		{
-			name:          "invalid configuration",
+			name:          "invalid ObjectLockEnabled",
 			bucket:        bktLockEnabled,
-			expectedError: s3errors.GetAPIError(s3errors.ErrInternalError),
+			expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid ObjectLockEnabled value: %s", "dummy")),
 			configuration: &data.ObjectLockConfiguration{ObjectLockEnabled: "dummy"},
 		},
+		{
+			name:          "invalid retention mode",
+			bucket:        bktLockEnabled,
+			expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid Mode value: %s", "dummy")),
+			configuration: &data.ObjectLockConfiguration{Rule: &data.ObjectLockRule{DefaultRetention: &data.DefaultRetention{Mode: "dummy"}}},
+		},
+		{
+			name:          "empty retention days and years",
+			bucket:        bktLockEnabled,
+			expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, errEmptyDaysErrors),
+			configuration: &data.ObjectLockConfiguration{Rule: &data.ObjectLockRule{DefaultRetention: &data.DefaultRetention{Mode: complianceMode}}},
+		},
+		{
+			name:          "non empty retention days and years",
+			bucket:        bktLockEnabled,
+			expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, errNonEmptyDaysErrors),
+			configuration: &data.ObjectLockConfiguration{Rule: &data.ObjectLockRule{DefaultRetention: &data.DefaultRetention{Mode: complianceMode, Days: 1, Years: 1}}},
+		},
 		{
 			name:          "basic",
 			bucket:        bktLockEnabled,