Skip to content

fix: trim whitespace from bucket location #2274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vigneshakaviki wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: trim whitespace from bucket location #2274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bucket-cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"net/http"
"net/url"
"path"
"strings"

"github.com/minio/minio-go/v7/pkg/credentials"
"github.com/minio/minio-go/v7/pkg/s3utils"
Expand Down Expand Up @@ -114,7 +115,7 @@ func processBucketLocationResponse(resp *http.Response, bucketName string) (buck
return "", err
}

location := locationConstraint
location := strings.TrimSpace(locationConstraint)

@allanrogerr allanrogerr Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One branch up, the error-response path returns errResp.Region without the same treatment, and the errResp.Region == "" check guarding it has the identical gap this line just closed below: a whitespace-only region is not "", so it is returned as-is, cached by getBucketLocation, and ends up as the SigV4 region — the same newline-in-the-Authorization-header failure described in #2271.

The same value also reaches that cache and the signer from executeMethod's retry path (api.go:805, 814, 822) without passing through this function at all, so trimming only here would not cover it. Trimming once where errResp.Region is populated covers all three callers, and has the nicer property that a whitespace-only region normalizes to "" and falls through to the x-amz-bucket-region header instead of defaulting to us-east-1. strings is already imported in that file.

In api-error-response.go, immediately before the existing header fallback:

	errResp.Region = strings.TrimSpace(errResp.Region)
	if errResp.Region == "" {
		errResp.Region = resp.Header.Get("x-amz-bucket-region")
	}

I have not seen a server actually emit a padded <Region> in an error body, so treat this as hardening rather than something you are hitting today — likely its own PR, and entirely your call.

// Location is empty will be 'us-east-1'.
if location == "" {
location = "us-east-1"
Expand Down
2 changes: 2 additions & 0 deletions bucket-cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ func TestProcessBucketLocationResponse(t *testing.T) {
}{
{"my-bucket", "", true, APIErrors[0], "us-east-1", nil, true},
{"my-bucket", "", false, APIError{}, "us-east-1", nil, true},
{"my-bucket", "\n \n", false, APIError{}, "us-east-1", nil, true},
{"my-bucket", " \neu-central-1\t", false, APIError{}, "eu-central-1", nil, true},

@allanrogerr allanrogerr Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description says the trim happens before the EU normalization, but neither new row exercises that ordering. The padded eu-central-1 row trims to a value that never reaches the EU branch, and the plain "EU" row passes with or without trimming, so nothing here would notice if the two steps were ever reordered.

This row covers the interaction directly: it resolves to eu-west-1 with the trim in place, and comes back as " EU " unchanged without it.

Suggested change
{"my-bucket", " \neu-central-1\t", false, APIError{}, "eu-central-1", nil, true},
{"my-bucket", " \neu-central-1\t", false, APIError{}, "eu-central-1", nil, true},
{"my-bucket", " EU ", false, APIError{}, "eu-west-1", nil, true},

{"my-bucket", "EU", false, APIError{}, "eu-west-1", nil, true},
{"my-bucket", "eu-central-1", false, APIError{}, "eu-central-1", nil, true},
{"my-bucket", "us-east-1", false, APIError{}, "us-east-1", nil, true},
Expand Down
Loading