From 0c7b691852b3fe099961bd95a74fb957998e8eb7 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Wed, 5 Jun 2019 15:47:49 -0700
Subject: [PATCH] Handle regions returned in error responses (#1117)

---
 api-list.go     | 17 -----------------
 api.go          | 20 ++++++++++++--------
 bucket-cache.go | 12 ++++++++++--
 3 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/api-list.go b/api-list.go
index 2fb0b6233..b9c0f5d8d 100644
--- a/api-list.go
+++ b/api-list.go
@@ -41,23 +41,6 @@ import (
 func (c Client) ListBuckets() ([]BucketInfo, error) {
 	// Execute GET on service.
 	resp, err := c.executeMethod(context.Background(), "GET", requestMetadata{contentSHA256Hex: emptySHA256Hex})
-
-	if err != nil {
-		closeResponse(resp)
-		return nil, err
-	}
-	if resp != nil {
-		if resp.StatusCode != http.StatusOK {
-			errResponse := ToErrorResponse(httpRespToErrorResponse(resp, "", ""))
-			closeResponse(resp)
-			if errResponse.Code != "AuthorizationHeaderMalformed" && errResponse.Code != "InvalidRegion" {
-				return nil, httpRespToErrorResponse(resp, "", "")
-			}
-			// Retry with the region returned by the server.
-			resp, err = c.executeMethod(context.Background(), "GET", requestMetadata{contentSHA256Hex: emptySHA256Hex, bucketLocation: errResponse.Region})
-		}
-	}
-
 	defer closeResponse(resp)
 	if err != nil {
 		return nil, err
diff --git a/api.go b/api.go
index 67b76e4dc..0e83d8607 100644
--- a/api.go
+++ b/api.go
@@ -653,14 +653,23 @@ func (c Client) executeMethod(ctx context.Context, method string, metadata reque
 		//
 		// Additionally we should only retry if bucketLocation and custom
 		// region is empty.
-		if metadata.bucketLocation == "" && c.region == "" {
-			if errResponse.Code == "AuthorizationHeaderMalformed" || errResponse.Code == "InvalidRegion" {
+		if c.region == "" {
+			switch errResponse.Code {
+			case "AuthorizationHeaderMalformed":
+				fallthrough
+			case "InvalidRegion":
+				fallthrough
+			case "AccessDenied":
 				if metadata.bucketName != "" && errResponse.Region != "" {
 					// Gather Cached location only if bucketName is present.
 					if _, cachedOk := c.bucketLocCache.Get(metadata.bucketName); cachedOk {
 						c.bucketLocCache.Set(metadata.bucketName, errResponse.Region)
 						continue // Retry.
 					}
+				} else {
+					// Most probably for ListBuckets()
+					metadata.bucketLocation = errResponse.Region
+					continue // Retry
 				}
 			}
 		}
@@ -694,13 +703,8 @@ func (c Client) newRequest(method string, metadata requestMetadata) (req *http.R
 			// Gather location only if bucketName is present.
 			location, err = c.getBucketLocation(metadata.bucketName)
 			if err != nil {
-				if ToErrorResponse(err).Code != "AccessDenied" {
-					return nil, err
-				}
+				return nil, err
 			}
-			// Upon AccessDenied error on fetching bucket location, default
-			// to possible locations based on endpoint URL. This can usually
-			// happen when GetBucketLocation() is disabled using IAM policies.
 		}
 		if location == "" {
 			location = getDefaultLocation(*c.endpointURL, c.region)
diff --git a/bucket-cache.go b/bucket-cache.go
index 53a8a9c7f..4ea7e9181 100644
--- a/bucket-cache.go
+++ b/bucket-cache.go
@@ -124,8 +124,16 @@ func processBucketLocationResponse(resp *http.Response, bucketName string) (buck
 			// For access denied error, it could be an anonymous
 			// request. Move forward and let the top level callers
 			// succeed if possible based on their policy.
-			if errResp.Code == "AccessDenied" {
-				return "us-east-1", nil
+			switch errResp.Code {
+			case "AuthorizationHeaderMalformed":
+				fallthrough
+			case "InvalidRegion":
+				fallthrough
+			case "AccessDenied":
+				if errResp.Region == "" {
+					return "us-east-1", nil
+				}
+				return errResp.Region, nil
 			}
 			return "", err
 		}