api: ListBuckets() should have a default region for AWS S3. (#720)

Fixes #717

api.go

@@ -591,15 +591,23 @@ func (c Client) newRequest(method string, metadata requestMetadata) (req *http.R
 		method = "POST"
 	}
 
-	var location string
-	// Gather location only if bucketName is present.
-	if metadata.bucketName != "" && metadata.bucketLocation == "" {
-		location, err = c.getBucketLocation(metadata.bucketName)
-		if err != nil {
-			return nil, err
+	location := metadata.bucketLocation
+	if location == "" {
+		if metadata.bucketName != "" {
+			// Gather location only if bucketName is present.
+			location, err = c.getBucketLocation(metadata.bucketName)
+			if err != nil {
+				if ToErrorResponse(err).Code != "AccessDenied" {
+					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)
 		}
-	} else {
-		location = metadata.bucketLocation
 	}
 
 	// Construct a new target URL.

bucket-cache.go

@@ -86,6 +86,11 @@ func (c Client) getBucketLocation(bucketName string) (string, error) {
 		return "", err
 	}
 
+	// Region set then no need to fetch bucket location.
+	if c.region != "" {
+		return c.region, nil
+	}
+
 	if s3utils.IsAmazonChinaEndpoint(c.endpointURL) {
 		// For china specifically we need to set everything to
 		// cn-north-1 for now, there is no easier way until AWS S3
@@ -100,11 +105,6 @@ func (c Client) getBucketLocation(bucketName string) (string, error) {
 		return "us-gov-west-1", nil
 	}
 
-	// Region set then no need to fetch bucket location.
-	if c.region != "" {
-		return c.region, nil
-	}
-
 	if location, ok := c.bucketLocCache.Get(bucketName); ok {
 		return location, nil
 	}

utils.go

@@ -192,3 +192,23 @@ func redactSignature(origAuth string) string {
 	// Strip out 256-bit signature from: Signature=<256-bit signature>
 	return regSign.ReplaceAllString(newAuth, "Signature=**REDACTED**")
 }
+
+// Get default location returns the location based on the input
+// URL `u`, if region override is provided then all location
+// defaults to regionOverride.
+//
+// If no other cases match then the location is set to `us-east-1`
+// as a last resort.
+func getDefaultLocation(u url.URL, regionOverride string) (location string) {
+	if regionOverride != "" {
+		return regionOverride
+	}
+	if s3utils.IsAmazonChinaEndpoint(u) {
+		return "cn-north-1"
+	}
+	if s3utils.IsAmazonGovCloudEndpoint(u) {
+		return "us-gov-west-1"
+	}
+	// Default to location to 'us-east-1'.
+	return "us-east-1"
+}

utils_test.go

@@ -126,8 +126,10 @@ func TestIsValidEndpointURL(t *testing.T) {
 	}{
 		{"", ErrInvalidArgument("Endpoint url cannot be empty."), false},
 		{"/", nil, true},
-		{"https://s3.am1;4205;0cazonaws.com", nil, true},
+		{"https://s3.amazonaws.com", nil, true},
 		{"https://s3.cn-north-1.amazonaws.com.cn", nil, true},
+		{"https://s3-us-gov-west-1.amazonaws.com", nil, true},
+		{"https://s3-fips-us-gov-west-1.amazonaws.com", nil, true},
 		{"https://s3.amazonaws.com/", nil, true},
 		{"https://storage.googleapis.com/", nil, true},
 		{"192.168.1.1", ErrInvalidArgument("Endpoint url cannot have fully qualified paths."), false},
@@ -165,6 +167,52 @@ func TestIsValidEndpointURL(t *testing.T) {
 	}
 }
 
+func TestDefaultBucketLocation(t *testing.T) {
+	testCases := []struct {
+		endpointURL      url.URL
+		regionOverride   string
+		expectedLocation string
+	}{
+		// Region override is set URL is ignored. - Test 1.
+		{
+			endpointURL:      url.URL{Host: "s3-fips-us-gov-west-1.amazonaws.com"},
+			regionOverride:   "us-west-1",
+			expectedLocation: "us-west-1",
+		},
+		// No region override, url based preferenced is honored - Test 2.
+		{
+			endpointURL:      url.URL{Host: "s3-fips-us-gov-west-1.amazonaws.com"},
+			regionOverride:   "",
+			expectedLocation: "us-gov-west-1",
+		},
+		// Region override is honored - Test 3.
+		{
+			endpointURL:      url.URL{Host: "s3.amazonaws.com"},
+			regionOverride:   "us-west-1",
+			expectedLocation: "us-west-1",
+		},
+		// China region should be honored, region override not provided. - Test 4.
+		{
+			endpointURL:      url.URL{Host: "s3.cn-north-1.amazonaws.com.cn"},
+			regionOverride:   "",
+			expectedLocation: "cn-north-1",
+		},
+		// No region provided, no standard region strings provided as well. - Test 5.
+		{
+			endpointURL:      url.URL{Host: "s3.amazonaws.com"},
+			regionOverride:   "",
+			expectedLocation: "us-east-1",
+		},
+	}
+
+	for i, testCase := range testCases {
+		retLocation := getDefaultLocation(testCase.endpointURL, testCase.regionOverride)
+		if testCase.expectedLocation != retLocation {
+			t.Errorf("Test %d: Expected location %s, got %s", i+1, testCase.expectedLocation, retLocation)
+		}
+	}
+}
+
 // Tests validate the expiry time validator.
 func TestIsValidExpiry(t *testing.T) {
 	testCases := []struct {