diff options
| -rw-r--r-- | caddytest/caddytest.go | 10 | ||||
| -rw-r--r-- | caddytest/integration/autohttps_test.go | 62 | ||||
| -rw-r--r-- | modules/caddyhttp/autohttps.go | 76 |
3 files changed, 104 insertions, 44 deletions
diff --git a/caddytest/caddytest.go b/caddytest/caddytest.go index d3c70b6..5387da7 100644 --- a/caddytest/caddytest.go +++ b/caddytest/caddytest.go @@ -314,9 +314,13 @@ func (tc *Tester) AssertRedirect(requestURI string, expectedToLocation string, e if err != nil { tc.t.Errorf("requesting \"%s\" expected location: \"%s\" but got error: %s", requestURI, expectedToLocation, err) } - - if expectedToLocation != loc.String() { - tc.t.Errorf("requesting \"%s\" expected location: \"%s\" but got \"%s\"", requestURI, expectedToLocation, loc.String()) + if loc == nil && expectedToLocation != "" { + tc.t.Errorf("requesting \"%s\" expected a Location header, but didn't get one", requestURI) + } + if loc != nil { + if expectedToLocation != loc.String() { + tc.t.Errorf("requesting \"%s\" expected location: \"%s\" but got \"%s\"", requestURI, expectedToLocation, loc.String()) + } } return resp diff --git a/caddytest/integration/autohttps_test.go b/caddytest/integration/autohttps_test.go index 62f172d..db6329a 100644 --- a/caddytest/integration/autohttps_test.go +++ b/caddytest/integration/autohttps_test.go @@ -7,7 +7,21 @@ import ( "github.com/caddyserver/caddy/v2/caddytest" ) -func TestAutoHTTPtoHTTPSRedirects(t *testing.T) { +func TestAutoHTTPtoHTTPSRedirectsImplicitPort(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + http_port 9080 + https_port 9443 + } + localhost + respond "Yahaha! You found me!" + `, "caddyfile") + + tester.AssertRedirect("http://localhost:9080/", "https://localhost/", http.StatusPermanentRedirect) +} + +func TestAutoHTTPtoHTTPSRedirectsExplicitPortSameAsHTTPSPort(t *testing.T) { tester := caddytest.NewTester(t) tester.InitServer(` { @@ -20,3 +34,49 @@ func TestAutoHTTPtoHTTPSRedirects(t *testing.T) { tester.AssertRedirect("http://localhost:9080/", "https://localhost/", http.StatusPermanentRedirect) } + +func TestAutoHTTPtoHTTPSRedirectsExplicitPortDifferentFromHTTPSPort(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + http_port 9080 + https_port 9443 + } + localhost:1234 + respond "Yahaha! You found me!" + `, "caddyfile") + + tester.AssertRedirect("http://localhost:9080/", "https://localhost:1234/", http.StatusPermanentRedirect) +} + +func TestAutoHTTPRedirectsWithHTTPListenerFirstInAddresses(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` +{ + "apps": { + "http": { + "http_port": 9080, + "https_port": 9443, + "servers": { + "ingress_server": { + "listen": [ + ":9080", + ":9443" + ], + "routes": [ + { + "match": [ + { + "host": ["localhost"] + } + ] + } + ] + } + } + } + } +} +`, "json") + tester.AssertRedirect("http://localhost:9080/", "https://localhost/", http.StatusPermanentRedirect) +} diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index fff4c46..9d6612f 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -303,31 +303,11 @@ uniqueDomainsLoop: matcherSet = append(matcherSet, MatchHost(domains)) } - // build the address to which to redirect addr, err := caddy.ParseNetworkAddress(addrStr) if err != nil { return err } - redirTo := "https://{http.request.host}" - if addr.StartPort != uint(app.httpsPort()) { - redirTo += ":" + strconv.Itoa(int(addr.StartPort)) - } - redirTo += "{http.request.uri}" - - // build the route - redirRoute := Route{ - MatcherSets: []MatcherSet{matcherSet}, - Handlers: []MiddlewareHandler{ - StaticResponse{ - StatusCode: WeakString(strconv.Itoa(http.StatusPermanentRedirect)), - Headers: http.Header{ - "Location": []string{redirTo}, - "Connection": []string{"close"}, - }, - Close: true, - }, - }, - } + redirRoute := app.makeRedirRoute(addr.StartPort, matcherSet) // use the network/host information from the address, // but change the port to the HTTP port then rebuild @@ -355,25 +335,7 @@ uniqueDomainsLoop: // it's not something that should be relied on. We can change this // if we want to. appendCatchAll := func(routes []Route) []Route { - redirTo := "https://{http.request.host}" - if app.httpsPort() != DefaultHTTPSPort { - redirTo += ":" + strconv.Itoa(app.httpsPort()) - } - redirTo += "{http.request.uri}" - routes = append(routes, Route{ - MatcherSets: []MatcherSet{{MatchProtocol("http")}}, - Handlers: []MiddlewareHandler{ - StaticResponse{ - StatusCode: WeakString(strconv.Itoa(http.StatusPermanentRedirect)), - Headers: http.Header{ - "Location": []string{redirTo}, - "Connection": []string{"close"}, - }, - Close: true, - }, - }, - }) - return routes + return append(routes, app.makeRedirRoute(uint(app.httpsPort()), MatcherSet{MatchProtocol("http")})) } redirServersLoop: @@ -422,6 +384,40 @@ redirServersLoop: return nil } +func (app *App) makeRedirRoute(redirToPort uint, matcherSet MatcherSet) Route { + redirTo := "https://{http.request.host}" + + // since this is an external redirect, we should only append an explicit + // port if we know it is not the officially standardized HTTPS port, and, + // notably, also not the port that Caddy thinks is the HTTPS port (the + // configurable HTTPSPort parameter) - we can't change the standard HTTPS + // port externally, so that config parameter is for internal use only; + // we also do not append the port if it happens to be the HTTP port as + // well, obviously (for example, user defines the HTTP port explicitly + // in the list of listen addresses for a server) + if redirToPort != uint(app.httpPort()) && + redirToPort != uint(app.httpsPort()) && + redirToPort != DefaultHTTPPort && + redirToPort != DefaultHTTPSPort { + redirTo += ":" + strconv.Itoa(int(redirToPort)) + } + + redirTo += "{http.request.uri}" + return Route{ + MatcherSets: []MatcherSet{matcherSet}, + Handlers: []MiddlewareHandler{ + StaticResponse{ + StatusCode: WeakString(strconv.Itoa(http.StatusPermanentRedirect)), + Headers: http.Header{ + "Location": []string{redirTo}, + "Connection": []string{"close"}, + }, + Close: true, + }, + }, + } +} + // createAutomationPolicy ensures that automated certificates for this // app are managed properly. This adds up to two automation policies: // one for the public names, and one for the internal names. If a catch-all |