From 02a59a04d171f10cc0e90699c1366de4b7e405c6 Mon Sep 17 00:00:00 2001
From: poornas <poornas@users.noreply.github.com>
Date: Tue, 18 Feb 2020 18:45:48 -0800
Subject: [PATCH] Fix error messages returned by (Put)GetObjectLegalHold
 (#9013)

fiixing some minor discrepancies between aws s3 responses
vs minio server
---
 cmd/api-errors.go              |  6 ++++++
 cmd/api-router.go              |  8 ++++----
 cmd/object-handlers.go         | 11 +++++++++--
 pkg/bucket/object/lock/lock.go |  5 +++++
 4 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/cmd/api-errors.go b/cmd/api-errors.go
index 14f3b2876..dbc856d33 100644
--- a/cmd/api-errors.go
+++ b/cmd/api-errors.go
@@ -151,6 +151,7 @@ const (
 	ErrKeyTooLongError
 	ErrInvalidBucketObjectLockConfiguration
 	ErrObjectLockConfigurationNotAllowed
+	ErrNoSuchObjectLockConfiguration
 	ErrObjectLocked
 	ErrInvalidRetentionDate
 	ErrPastObjectLockRetainDate
@@ -761,6 +762,11 @@ var errorCodes = errorCodeMap{
 		Description:    "Object Lock configuration cannot be enabled on existing buckets.",
 		HTTPStatusCode: http.StatusConflict,
 	},
+	ErrNoSuchObjectLockConfiguration: {
+		Code:           "NoSuchObjectLockConfiguration",
+		Description:    "The specified object does not have a ObjectLock configuration",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 	ErrObjectLocked: {
 		Code:           "InvalidRequest",
 		Description:    "Object is WORM protected and cannot be overwritten",
diff --git a/cmd/api-router.go b/cmd/api-router.go
index a74bd6a65..9266c9874 100644
--- a/cmd/api-router.go
+++ b/cmd/api-router.go
@@ -116,17 +116,17 @@ func registerAPIRouter(router *mux.Router, encryptionEnabled, allowSSEKMS bool)
 		// SelectObjectContent
 		bucket.Methods(http.MethodPost).Path("/{object:.+}").HandlerFunc(collectAPIStats("selectobjectcontent", httpTraceHdrs(api.SelectObjectContentHandler))).Queries("select", "").Queries("select-type", "2")
 		// GetObjectRetention
-		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(collectAPIStats("getobjectretention", httpTraceHdrs(api.GetObjectRetentionHandler))).Queries("retention", "")
+		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(collectAPIStats("getobjectretention", httpTraceAll(api.GetObjectRetentionHandler))).Queries("retention", "")
 		// GetObjectLegalHold
-		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(collectAPIStats("getobjectlegalhold", httpTraceHdrs(api.GetObjectLegalHoldHandler))).Queries("legal-hold", "")
+		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(collectAPIStats("getobjectlegalhold", httpTraceAll(api.GetObjectLegalHoldHandler))).Queries("legal-hold", "")
 		// GetObject
 		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(collectAPIStats("getobject", httpTraceHdrs(api.GetObjectHandler)))
 		// CopyObject
 		bucket.Methods(http.MethodPut).Path("/{object:.+}").HeadersRegexp(xhttp.AmzCopySource, ".*?(\\/|%2F).*?").HandlerFunc(collectAPIStats("copyobject", httpTraceAll(api.CopyObjectHandler)))
 		// PutObjectRetention
-		bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(collectAPIStats("putobjectretention", httpTraceHdrs(api.PutObjectRetentionHandler))).Queries("retention", "")
+		bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(collectAPIStats("putobjectretention", httpTraceAll(api.PutObjectRetentionHandler))).Queries("retention", "")
 		// PutObjectLegalHold
-		bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(collectAPIStats("putobjectlegalhold", httpTraceHdrs(api.PutObjectLegalHoldHandler))).Queries("legal-hold", "")
+		bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(collectAPIStats("putobjectlegalhold", httpTraceAll(api.PutObjectLegalHoldHandler))).Queries("legal-hold", "")
 
 		// PutObject
 		bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(collectAPIStats("putobject", httpTraceHdrs(api.PutObjectHandler)))
diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go
index b5652b312..dc0204b41 100644
--- a/cmd/object-handlers.go
+++ b/cmd/object-handlers.go
@@ -2658,7 +2658,7 @@ func (api objectAPIHandlers) PutObjectLegalHoldHandler(w http.ResponseWriter, r
 		return
 	}
 
-	writeSuccessNoContent(w)
+	writeSuccessResponseHeadersOnly(w)
 	// Notify object  event.
 	sendEvent(eventArgs{
 		EventName:    event.ObjectCreatedPutLegalHold,
@@ -2718,8 +2718,15 @@ func (api objectAPIHandlers) GetObjectLegalHoldHandler(w http.ResponseWriter, r
 		return
 	}
 
+	if _, ok := globalBucketObjectLockConfig.Get(bucket); !ok {
+		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidBucketObjectLockConfiguration), r.URL, guessIsBrowserReq(r))
+		return
+	}
 	legalHold := objectlock.GetObjectLegalHoldMeta(objInfo.UserDefined)
-
+	if legalHold.IsEmpty() {
+		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNoSuchObjectLockConfiguration), r.URL, guessIsBrowserReq(r))
+		return
+	}
 	writeSuccessResponseXML(w, encodeResponse(legalHold))
 	// Notify object legal hold accessed via a GET request.
 	sendEvent(eventArgs{
diff --git a/pkg/bucket/object/lock/lock.go b/pkg/bucket/object/lock/lock.go
index 24d38ef9a..18491c7b6 100644
--- a/pkg/bucket/object/lock/lock.go
+++ b/pkg/bucket/object/lock/lock.go
@@ -475,6 +475,11 @@ type ObjectLegalHold struct {
 	Status  LegalHoldStatus `xml:"Status,omitempty"`
 }
 
+// IsEmpty returns true if struct is empty
+func (l *ObjectLegalHold) IsEmpty() bool {
+	return l.Status != ON && l.Status != OFF
+}
+
 // ParseObjectLegalHold decodes the XML into ObjectLegalHold
 func ParseObjectLegalHold(reader io.Reader) (hold *ObjectLegalHold, err error) {
 	if err = xml.NewDecoder(reader).Decode(&hold); err != nil {