From 39ac62a1a11ade155cf33fb5cb8ad02723d78977 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 1 May 2022 13:45:45 -0700 Subject: [PATCH] fix: panic in browser redirect handler for unexpected r.Host (#14844) ``` panic: "GET /": invalid hostname goroutine 148 [running]: runtime/debug.Stack() runtime/debug/stack.go:24 +0x65 github.com/minio/minio/cmd.setCriticalErrorHandler.func1.1() github.com/minio/minio/cmd/generic-handlers.go:469 +0x8e panic({0x2201f00, 0xc001f1ddd0}) runtime/panic.go:1038 +0x215 github.com/minio/pkg/net.URL.String({{0x25aa417, 0x5}, {0x0, 0x0}, 0x0, {0xc000174380, 0xd7}, {0x0, 0x0}, {0x0, ...}, ...}) github.com/minio/pkg@v1.1.23/net/url.go:97 +0xfe github.com/minio/minio/cmd.setBrowserRedirectHandler.func1({0x49af080, 0xc0003c20e0}, 0xc00002ea00) github.com/minio/minio/cmd/generic-handlers.go:136 +0x118 net/http.HandlerFunc.ServeHTTP(0xc00002ea00, {0x49af080, 0xc0003c20e0}, 0xa) net/http/server.go:2047 +0x2f github.com/minio/minio/cmd.setAuthHandler.func1({0x49af080, 0xc0003c20e0}, 0xc00002ea00) github.com/minio/minio/cmd/auth-handler.go:525 +0x3d8 net/http.HandlerFunc.ServeHTTP(0xc00002e900, {0x49af080, 0xc0003c20e0}, 0xc001f33701) net/http/server.go:2047 +0x2f github.com/gorilla/mux.(*Router).ServeHTTP(0xc0025d0780, {0x49af080, 0xc0003c20e0}, 0xc00002e800) github.com/gorilla/mux@v1.8.0/mux.go:210 +0x1cf github.com/rs/cors.(*Cors).Handler.func1({0x49af080, 0xc0003c20e0}, 0xc00002e800) github.com/rs/cors@v1.7.0/cors.go:219 +0x1bd net/http.HandlerFunc.ServeHTTP(0x0, {0x49af080, 0xc0003c20e0}, 0xc00068d9f8) net/http/server.go:2047 +0x2f github.com/minio/minio/cmd.setCriticalErrorHandler.func1({0x49af080, 0xc0003c20e0}, 0x4a5cd3) github.com/minio/minio/cmd/generic-handlers.go:476 +0x83 net/http.HandlerFunc.ServeHTTP(0x72, {0x49af080, 0xc0003c20e0}, 0x0) net/http/server.go:2047 +0x2f github.com/minio/minio/internal/http.(*Server).Start.func1({0x49af080, 0xc0003c20e0}, 0x10000c001f1dda0) github.com/minio/minio/internal/http/server.go:105 +0x1b6 net/http.HandlerFunc.ServeHTTP(0x0, {0x49af080, 0xc0003c20e0}, 0x46982e) net/http/server.go:2047 +0x2f net/http.serverHandler.ServeHTTP({0xc003dc1950}, {0x49af080, 0xc0003c20e0}, 0xc00002e800) net/http/server.go:2879 +0x43b net/http.(*conn).serve(0xc000514d20, {0x49cfc38, 0xc0010c0e70}) net/http/server.go:1930 +0xb08 created by net/http.(*Server).Serve net/http/server.go:3034 +0x4e8 ``` --- cmd/generic-handlers.go | 53 ++++++++++++++++++++------------------- cmd/handler-utils.go | 27 ++++++++++---------- cmd/handler-utils_test.go | 3 +++ 3 files changed, 43 insertions(+), 40 deletions(-) diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go index fa4ba1666..0d6a12718 100644 --- a/cmd/generic-handlers.go +++ b/cmd/generic-handlers.go @@ -141,6 +141,14 @@ func setBrowserRedirectHandler(h http.Handler) http.Handler { }) } +var redirectPrefixes = map[string]struct{}{ + "favicon-16x16.png": {}, + "favicon-32x32.png": {}, + "favicon-96x96.png": {}, + "index.html": {}, + minioReservedBucket: {}, +} + // Fetch redirect location if urlPath satisfies certain // criteria. Some special names are considered to be // redirectable, this is purely internal function and @@ -151,32 +159,25 @@ func getRedirectLocation(r *http.Request) *xnet.URL { if err != nil { return nil } - for _, prefix := range []string{ - "favicon-16x16.png", - "favicon-32x32.png", - "favicon-96x96.png", - "index.html", - minioReservedBucket, - } { - bucket, _ := path2BucketObject(resource) - if path.Clean(bucket) == prefix || resource == slashSeparator { - if globalBrowserRedirectURL != nil { - return globalBrowserRedirectURL - } - hostname, _, _ := net.SplitHostPort(r.Host) - if hostname == "" { - hostname = r.Host - } - return &xnet.URL{ - Host: net.JoinHostPort(hostname, globalMinioConsolePort), - Scheme: func() string { - scheme := "http" - if r.TLS != nil { - scheme = "https" - } - return scheme - }(), - } + bucket, _ := path2BucketObject(resource) + _, redirect := redirectPrefixes[path.Clean(bucket)] + if redirect || resource == slashSeparator { + if globalBrowserRedirectURL != nil { + return globalBrowserRedirectURL + } + xhost, err := xnet.ParseHost(r.Host) + if err != nil { + return nil + } + return &xnet.URL{ + Host: net.JoinHostPort(xhost.Name, globalMinioConsolePort), + Scheme: func() string { + scheme := "http" + if r.TLS != nil { + scheme = "https" + } + return scheme + }(), } } return nil diff --git a/cmd/handler-utils.go b/cmd/handler-utils.go index 95fbbb439..793fb88ff 100644 --- a/cmd/handler-utils.go +++ b/cmd/handler-utils.go @@ -25,7 +25,6 @@ import ( "io" "io/ioutil" "mime/multipart" - "net" "net/http" "net/textproto" "net/url" @@ -37,6 +36,7 @@ import ( "github.com/minio/minio/internal/handlers" xhttp "github.com/minio/minio/internal/http" "github.com/minio/minio/internal/logger" + xnet "github.com/minio/pkg/net" ) const ( @@ -395,26 +395,25 @@ func getResource(path string, host string, domains []string) (string, error) { if len(domains) == 0 { return path, nil } + // If virtual-host-style is enabled construct the "resource" properly. - if strings.Contains(host, ":") { - // In bucket.mydomain.com:9000, strip out :9000 - var err error - if host, _, err = net.SplitHostPort(host); err != nil { - reqInfo := (&logger.ReqInfo{}).AppendTags("host", host) - reqInfo.AppendTags("path", path) - ctx := logger.SetReqInfo(GlobalContext, reqInfo) - logger.LogIf(ctx, err) - return "", err - } + xhost, err := xnet.ParseHost(host) + if err != nil { + reqInfo := (&logger.ReqInfo{}).AppendTags("host", host) + reqInfo.AppendTags("path", path) + ctx := logger.SetReqInfo(context.Background(), reqInfo) + logger.LogIf(ctx, err) + return "", err } + for _, domain := range domains { - if host == minioReservedBucket+"."+domain { + if xhost.Name == minioReservedBucket+"."+domain { continue } - if !strings.HasSuffix(host, "."+domain) { + if !strings.HasSuffix(xhost.Name, "."+domain) { continue } - bucket := strings.TrimSuffix(host, "."+domain) + bucket := strings.TrimSuffix(xhost.Name, "."+domain) return SlashSeparator + pathJoin(bucket, path), nil } return path, nil diff --git a/cmd/handler-utils_test.go b/cmd/handler-utils_test.go index 156566632..17c6bdfbc 100644 --- a/cmd/handler-utils_test.go +++ b/cmd/handler-utils_test.go @@ -221,6 +221,9 @@ func TestGetResource(t *testing.T) { expectedResource string }{ {"/a/b/c", "test.mydomain.com", []string{"mydomain.com"}, "/test/a/b/c"}, + {"/a/b/c", "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:17000", []string{"mydomain.com"}, "/a/b/c"}, + {"/a/b/c", "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", []string{"mydomain.com"}, "/a/b/c"}, + {"/a/b/c", "192.168.1.1:9000", []string{"mydomain.com"}, "/a/b/c"}, {"/a/b/c", "test.mydomain.com", []string{"notmydomain.com"}, "/a/b/c"}, {"/a/b/c", "test.mydomain.com", nil, "/a/b/c"}, }