fix(s3): default to https:// for cloud endpoints (UX improvement) (#949)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Cory LaNou
2026-01-06 11:58:00 -06:00
committed by GitHub
parent 62b25c13dc
commit a7d773e7a4
5 changed files with 149 additions and 11 deletions

View File

@@ -1276,11 +1276,8 @@ func NewS3ReplicaClientFromConfig(c *ReplicaConfig, _ *litestream.Replica) (_ *s
// Override with query parameters if provided
if qEndpoint := query.Get("endpoint"); qEndpoint != "" {
// Ensure endpoint has a scheme
if !strings.HasPrefix(qEndpoint, "http://") && !strings.HasPrefix(qEndpoint, "https://") {
// Default to http for non-TLS endpoints (common for local/dev)
qEndpoint = "http://" + qEndpoint
}
// Ensure endpoint has a scheme (defaults to https:// for cloud, http:// for local)
qEndpoint, _ = litestream.EnsureEndpointScheme(qEndpoint)
uendpoint = qEndpoint
// Default to path style for custom endpoints unless explicitly set to false
if query.Get("forcePathStyle") != "false" {

View File

@@ -2465,8 +2465,8 @@ func TestNewS3ReplicaClientFromConfig(t *testing.T) {
if client.Bucket != "my-tigris-bucket" {
t.Errorf("expected bucket 'my-tigris-bucket', got %q", client.Bucket)
}
if client.Endpoint != "http://fly.storage.tigris.dev" {
t.Errorf("expected Tigris endpoint, got %q", client.Endpoint)
if client.Endpoint != "https://fly.storage.tigris.dev" {
t.Errorf("expected Tigris endpoint with https scheme, got %q", client.Endpoint)
}
if client.Region != "auto" {
t.Errorf("expected region 'auto' for Tigris, got %q", client.Region)

View File

@@ -281,6 +281,52 @@ func IsMinIOEndpoint(endpoint string) bool {
return true
}
// IsLocalEndpoint returns true if the endpoint appears to be a local development
// endpoint (localhost, 127.0.0.1, or private network addresses).
// These endpoints typically use HTTP instead of HTTPS.
func IsLocalEndpoint(endpoint string) bool {
host := extractEndpointHost(endpoint)
if host == "" {
return false
}
// Remove port if present
if idx := strings.LastIndex(host, ":"); idx != -1 {
host = host[:idx]
}
// Check for common local/development hostnames
return host == "localhost" ||
host == "127.0.0.1" ||
strings.HasPrefix(host, "192.168.") ||
strings.HasPrefix(host, "10.") ||
strings.HasPrefix(host, "172.16.") ||
strings.HasPrefix(host, "172.17.") ||
strings.HasPrefix(host, "172.18.") ||
strings.HasPrefix(host, "172.19.") ||
strings.HasPrefix(host, "172.2") || // 172.20-172.29
strings.HasPrefix(host, "172.30.") ||
strings.HasPrefix(host, "172.31.") ||
strings.HasSuffix(host, ".local") ||
strings.HasSuffix(host, ".localhost")
}
// EnsureEndpointScheme ensures an endpoint has an HTTP(S) scheme.
// For local endpoints (localhost, private IPs), it defaults to http://.
// For all other endpoints (cloud providers), it defaults to https://.
// Returns the endpoint with scheme and a boolean indicating if a scheme was added.
func EnsureEndpointScheme(endpoint string) (string, bool) {
if endpoint == "" {
return "", false
}
if strings.HasPrefix(endpoint, "http://") || strings.HasPrefix(endpoint, "https://") {
return endpoint, false
}
// Default to HTTP for local development endpoints, HTTPS for everything else
if IsLocalEndpoint(endpoint) {
return "http://" + endpoint, true
}
return "https://" + endpoint, true
}
// extractEndpointHost extracts the host from an endpoint URL or returns the
// endpoint as-is if it's not a full URL.
func extractEndpointHost(endpoint string) string {

View File

@@ -895,3 +895,100 @@ func TestIsMinIOEndpoint(t *testing.T) {
})
}
}
func TestIsLocalEndpoint(t *testing.T) {
tests := []struct {
endpoint string
expected bool
}{
// Localhost variants
{"localhost", true},
{"localhost:9000", true},
{"http://localhost:9000", true},
{"https://localhost:9000", true},
// Loopback IP
{"127.0.0.1", true},
{"127.0.0.1:9000", true},
{"http://127.0.0.1:9000", true},
// Private network ranges (RFC1918)
{"192.168.1.100", true},
{"192.168.1.100:9000", true},
{"http://192.168.1.100:9000", true},
{"10.0.0.1", true},
{"10.0.0.1:9000", true},
{"172.16.0.1", true},
{"172.31.255.255", true},
// .local and .localhost TLDs
{"minio.local", true},
{"minio.local:9000", true},
{"http://minio.local:9000", true},
{"dev.localhost", true},
{"test.localhost:8080", true},
// Non-local endpoints (cloud providers)
{"s3.amazonaws.com", false},
{"https://s3.amazonaws.com", false},
{"abcdef.r2.cloudflarestorage.com", false},
{"https://abcdef.r2.cloudflarestorage.com", false},
{"fly.storage.tigris.dev", false},
{"s3.us-west-000.backblazeb2.com", false},
{"sfo3.digitaloceanspaces.com", false},
{"s3.filebase.com", false},
// Empty
{"", false},
}
for _, tt := range tests {
t.Run(tt.endpoint, func(t *testing.T) {
got := litestream.IsLocalEndpoint(tt.endpoint)
if got != tt.expected {
t.Errorf("IsLocalEndpoint(%q) = %v, want %v", tt.endpoint, got, tt.expected)
}
})
}
}
func TestEnsureEndpointScheme(t *testing.T) {
tests := []struct {
input string
expected string
schemeAdded bool
}{
// Already has scheme - no change
{"https://example.com", "https://example.com", false},
{"http://localhost:9000", "http://localhost:9000", false},
{"http://192.168.1.1:9000", "http://192.168.1.1:9000", false},
// Local endpoints get http://
{"localhost:9000", "http://localhost:9000", true},
{"127.0.0.1:9000", "http://127.0.0.1:9000", true},
{"192.168.1.100:9000", "http://192.168.1.100:9000", true},
{"10.0.0.1:9000", "http://10.0.0.1:9000", true},
{"minio.local:9000", "http://minio.local:9000", true},
// Cloud endpoints get https:// (THIS IS THE KEY FIX)
{"abcdef.r2.cloudflarestorage.com", "https://abcdef.r2.cloudflarestorage.com", true},
{"s3.us-west-000.backblazeb2.com", "https://s3.us-west-000.backblazeb2.com", true},
{"fly.storage.tigris.dev", "https://fly.storage.tigris.dev", true},
{"sfo3.digitaloceanspaces.com", "https://sfo3.digitaloceanspaces.com", true},
{"s3.filebase.com", "https://s3.filebase.com", true},
{"s3.fr-par.scw.cloud", "https://s3.fr-par.scw.cloud", true},
// Empty returns empty
{"", "", false},
}
for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
got, added := litestream.EnsureEndpointScheme(tt.input)
if got != tt.expected {
t.Errorf("EnsureEndpointScheme(%q) = %q, want %q", tt.input, got, tt.expected)
}
if added != tt.schemeAdded {
t.Errorf("EnsureEndpointScheme(%q) schemeAdded = %v, want %v", tt.input, added, tt.schemeAdded)
}
})
}
}

View File

@@ -144,10 +144,8 @@ func NewReplicaClientFromURL(scheme, host, urlPath string, query url.Values, use
// Override with query parameters if provided
if qEndpoint := query.Get("endpoint"); qEndpoint != "" {
// Ensure endpoint has a scheme
if !strings.HasPrefix(qEndpoint, "http://") && !strings.HasPrefix(qEndpoint, "https://") {
qEndpoint = "http://" + qEndpoint
}
// Ensure endpoint has a scheme (defaults to https:// for cloud, http:// for local)
qEndpoint, _ = litestream.EnsureEndpointScheme(qEndpoint)
endpoint = qEndpoint
// Default to path style for custom endpoints unless explicitly set to false
if query.Get("forcePathStyle") != "false" {