Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions storage/http_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,21 @@ func newHTTPStorageClient(ctx context.Context, opts ...storageOption) (storageCl
if err != nil {
return nil, fmt.Errorf("dialing: %w", err)
}

if newEp := ensureEndpoint(ep); newEp != ep {
ep = newEp
if hc != nil {
hc.CloseIdleConnections()
}
// Redial with the fixed endpoint so that the HTTP transport is
// correctly configured for HTTPS (including ALPN for HTTP/2).
s.clientOption = append(s.clientOption, option.WithEndpoint(ep))
hc, ep, err = htransport.NewClient(ctx, s.clientOption...)
if err != nil {
return nil, fmt.Errorf("dialing with fixed endpoint: %w", err)
}
}

// RawService should be created with the chosen endpoint to take account of user override.
rawService, err := raw.NewService(ctx, option.WithEndpoint(ep), option.WithHTTPClient(hc))
if err != nil {
Expand Down
23 changes: 23 additions & 0 deletions storage/internal_utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package storage

import (
"strings"
)

// ensureEndpoint fixes schemeless endpoints by prepending "https://"
// and ensuring it has the "/storage/v1/" suffix.
func ensureEndpoint(ep string) string {
// If the resolved endpoint doesn't have a scheme, it is most likely a
// host-only string from WithEndpoint. Prepend https:// and ensure it has
// the storage path.
if !strings.Contains(ep, "://") {
ep = "https://" + ep
if !strings.Contains(ep, "/storage/v1") {
if !strings.HasSuffix(ep, "/") {
ep += "/"
}
ep += "storage/v1/"
}
}
Comment on lines +13 to +21
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.

high

The logic for ensuring the /storage/v1/ path suffix is currently nested inside the scheme check. This means that if a user provides an endpoint with a scheme but without the path (e.g., https://my-proxy.com), the path suffix will not be appended, which may cause the client to fail to construct valid API requests. Additionally, the trailing slash is not ensured if the path is already present but lacks it (e.g., my-proxy.com/storage/v1).

Consider moving the path check outside the scheme check and ensuring a trailing slash is always present to maintain consistency and correctness for the underlying REST client.

	if !strings.Contains(ep, "://") {
		ep = "https://" + ep
	}
	if !strings.Contains(ep, "/storage/v1") {
		if !strings.HasSuffix(ep, "/") {
			ep += "/"
		}
		ep += "storage/v1/"
	}
	if !strings.HasSuffix(ep, "/") {
		ep += "/"
	}

return ep
}
15 changes: 15 additions & 0 deletions storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,21 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error
if err != nil {
return nil, fmt.Errorf("dialing: %w", err)
}

if newEp := ensureEndpoint(ep); newEp != ep {
ep = newEp
if hc != nil {
hc.CloseIdleConnections()
}
// Redial with the fixed endpoint so that the HTTP transport is
// correctly configured for HTTPS (including ALPN for HTTP/2).
opts = append(opts, option.WithEndpoint(ep))
hc, ep, err = htransport.NewClient(ctx, opts...)
if err != nil {
return nil, fmt.Errorf("dialing with fixed endpoint: %w", err)
}
}

// RawService should be created with the chosen endpoint to take account of user override.
// Preserve other user-supplied options as well.
opts = append(opts, option.WithEndpoint(ep), option.WithHTTPClient(hc))
Expand Down
26 changes: 26 additions & 0 deletions storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2761,3 +2761,29 @@ func isZeroValue(v reflect.Value) (bool, error) {
return false, fmt.Errorf("unable to check kind %s", v.Kind())
}
}

func TestSignedURL_SchemelessEndpoint(t *testing.T) {
ctx := context.Background()
ep := "storage.europe-west3.rep.googleapis.com"
client, err := NewClient(ctx, option.WithEndpoint(ep), option.WithoutAuthentication())
if err != nil {
t.Fatalf("NewClient: %v", err)
}
defer client.Close()

u, err := client.Bucket("my-bucket").SignedURL("my-object", &SignedURLOptions{
Method: "GET",
Expires: time.Now().Add(time.Hour),
GoogleAccessID: "xxx@xxx.com",
SignBytes: func(b []byte) ([]byte, error) {
return []byte("signed"), nil
},
})
if err != nil {
t.Fatalf("SignedURL: %v", err)
}

if !strings.HasPrefix(u, "https://"+ep) {
t.Errorf("SignedURL %q does not start with expected endpoint %q", u, "https://"+ep)
}
}
Loading