handler: simplify parsing valid location constraint. (#4040)

Separate out validating v/s parsing logic in
isValidLocationConstraint() into parseLocationConstraint()
and isValidLocation()

Additionally also set `X-Amz-Bucket-Region` as part of the
common headers for the clients to fallback on in-case of any
region related errors.

cmd/admin-handlers_test.go

@@ -1346,6 +1346,11 @@ func TestToAdminAPIErr(t *testing.T) {
 }
 
 func TestWriteSetConfigResponse(t *testing.T) {
+	rootPath, err := newTestConfig(globalMinioDefaultRegion)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer removeAll(rootPath)
 	testCases := []struct {
 		status bool
 		errs   []error

cmd/api-headers.go

@@ -36,6 +36,7 @@ func setCommonHeaders(w http.ResponseWriter) {
 	// Set unique request ID for each reply.
 	w.Header().Set(responseRequestIDKey, mustGetRequestID(UTCNow()))
 	w.Header().Set("Server", globalServerUserAgent)
+	w.Header().Set("X-Amz-Bucket-Region", serverConfig.GetRegion())
 	w.Header().Set("Accept-Ranges", "bytes")
 }
 

cmd/bucket-handlers.go

@@ -370,13 +370,20 @@ func (api objectAPIHandlers) PutBucketHandler(w http.ResponseWriter, r *http.Req
 	vars := mux.Vars(r)
 	bucket := vars["bucket"]
 
-	// Validate if incoming location constraint is valid, reject
+	// Parse incoming location constraint.
-	// requests which do not follow valid region requirements.
+	location, s3Error := parseLocationConstraint(r)
-	if s3Error := isValidLocationConstraint(r); s3Error != ErrNone {
+	if s3Error != ErrNone {
 		writeErrorResponse(w, s3Error, r.URL)
 		return
 	}
 
+	// Validate if location sent by the client is valid, reject
+	// requests which do not follow valid region requirements.
+	if !isValidLocation(location) {
+		writeErrorResponse(w, ErrInvalidRegion, r.URL)
+		return
+	}
+
 	bucketLock := globalNSMutex.NewNSLock(bucket, "")
 	bucketLock.Lock()
 	defer bucketLock.Unlock()

cmd/handler-utils.go

@@ -24,36 +24,29 @@ import (
 	"strings"
 )
 
-// Validates location constraint in PutBucket request body.
+// Parses location constraint from the incoming reader.
-// The location value in the request body should match the
+func parseLocationConstraint(r *http.Request) (location string, s3Error APIErrorCode) {
-// region configured at serverConfig, otherwise error is returned.
-func isValidLocationConstraint(r *http.Request) (s3Error APIErrorCode) {
-	serverRegion := serverConfig.GetRegion()
 	// If the request has no body with content-length set to 0,
 	// we do not have to validate location constraint. Bucket will
 	// be created at default region.
 	locationConstraint := createBucketLocationConfiguration{}
 	err := xmlDecoder(r.Body, &locationConstraint, r.ContentLength)
-	if err == nil || err == io.EOF {
+	if err != nil && err != io.EOF {
-		// Successfully decoded, proceed to verify the region.
+		errorIf(err, "Unable to xml decode location constraint")
-		// Once region has been obtained we proceed to verify it.
+		// Treat all other failures as XML parsing errors.
-		incomingRegion := locationConstraint.Location
+		return "", ErrMalformedXML
-		if incomingRegion == "" {
+	} // else for both err as nil or io.EOF
-			// Location constraint is empty for region globalMinioDefaultRegion,
+	location = locationConstraint.Location
-			// in accordance with protocol.
+	if location == "" {
-			incomingRegion = globalMinioDefaultRegion
+		location = globalMinioDefaultRegion
-		}
-		// Return errInvalidRegion if location constraint does not match
-		// with configured region.
-		s3Error = ErrNone
-		if serverRegion != incomingRegion {
-			s3Error = ErrInvalidRegion
-		}
-		return s3Error
 	}
-	errorIf(err, "Unable to xml decode location constraint")
+	return location, ErrNone
-	// Treat all other failures as XML parsing errors.
+}
-	return ErrMalformedXML
+
+// Validates input location is same as configured region
+// of Minio server.
+func isValidLocation(location string) bool {
+	return serverConfig.GetRegion() == location
 }
 
 // Supported headers that needs to be extracted.

cmd/handler-utils_test.go

@@ -39,7 +39,7 @@ func TestIsValidLocationContraint(t *testing.T) {
 		Body:          ioutil.NopCloser(bytes.NewBuffer([]byte("<>"))),
 		ContentLength: int64(len("<>")),
 	}
-	if err := isValidLocationConstraint(malformedReq); err != ErrMalformedXML {
+	if _, err := parseLocationConstraint(malformedReq); err != ErrMalformedXML {
 		t.Fatal("Unexpected error: ", err)
 	}
 
@@ -68,8 +68,6 @@ func TestIsValidLocationContraint(t *testing.T) {
 		// Test case - 2.
 		// In case of empty request body ErrNone is returned.
 		{"", globalMinioDefaultRegion, ErrNone},
-		// Test case - 3.
-		{"eu-central-1", globalMinioDefaultRegion, ErrInvalidRegion},
 	}
 	for i, testCase := range testCases {
 		inputRequest, e := createExpectedRequest(&http.Request{}, testCase.locationForInputRequest)
@@ -77,7 +75,7 @@ func TestIsValidLocationContraint(t *testing.T) {
 			t.Fatalf("Test %d: Failed to Marshal bucket configuration", i+1)
 		}
 		serverConfig.SetRegion(testCase.serverConfigRegion)
-		actualCode := isValidLocationConstraint(inputRequest)
+		_, actualCode := parseLocationConstraint(inputRequest)
 		if testCase.expectedCode != actualCode {
 			t.Errorf("Test %d: Expected the APIErrCode to be %d, but instead found %d", i+1, testCase.expectedCode, actualCode)
 		}