summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrancis Lavoie <lavofr@gmail.com>2021-04-19 21:54:12 -0400
committerGitHub <noreply@github.com>2021-04-19 19:54:12 -0600
commitd789596bc0b014c99d75c00fe8e55c40ee3d58e3 (patch)
treeafb7c01838832af81d10df46b0e11ea26ece3a15
parent96bb3659299ae5ef28ffb3f9a23e16417c570924 (diff)
caddyhttp: Implement better logic for inserting the HTTP->HTTPS redirs (#4033)
* caddyhttp: Implement better logic for inserting the HTTP->HTTPS redirs * caddyhttp: Add integration test
-rw-r--r--caddytest/integration/autohttps_test.go23
-rw-r--r--modules/caddyhttp/autohttps.go29
-rw-r--r--modules/caddyhttp/server.go31
3 files changed, 69 insertions, 14 deletions
diff --git a/caddytest/integration/autohttps_test.go b/caddytest/integration/autohttps_test.go
index db6329a..72968e9 100644
--- a/caddytest/integration/autohttps_test.go
+++ b/caddytest/integration/autohttps_test.go
@@ -80,3 +80,26 @@ func TestAutoHTTPRedirectsWithHTTPListenerFirstInAddresses(t *testing.T) {
`, "json")
tester.AssertRedirect("http://localhost:9080/", "https://localhost/", http.StatusPermanentRedirect)
}
+
+func TestAutoHTTPRedirectsInsertedBeforeUserDefinedCatchAll(t *testing.T) {
+ tester := caddytest.NewTester(t)
+ tester.InitServer(`
+ {
+ http_port 9080
+ https_port 9443
+ local_certs
+ }
+ http://:9080 {
+ respond "Foo"
+ }
+ http://baz.localhost:9080 {
+ respond "Baz"
+ }
+ bar.localhost {
+ respond "Bar"
+ }
+ `, "caddyfile")
+ tester.AssertRedirect("http://bar.localhost:9080/", "https://bar.localhost/", http.StatusPermanentRedirect)
+ tester.AssertGetResponse("http://foo.localhost:9080/", 200, "Foo")
+ tester.AssertGetResponse("http://baz.localhost:9080/", 200, "Baz")
+}
diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go
index 5c83d8f..da4428d 100644
--- a/modules/caddyhttp/autohttps.go
+++ b/modules/caddyhttp/autohttps.go
@@ -342,21 +342,22 @@ redirServersLoop:
for redirServerAddr, routes := range redirServers {
// for each redirect listener, see if there's already a
// server configured to listen on that exact address; if so,
- // simply add the redirect route to the end of its route
- // list; otherwise, we'll create a new server for all the
- // listener addresses that are unused and serve the
- // remaining redirects from it
- for srvName, srv := range app.Servers {
+ // insert the redirect route to the end of its route list
+ // after any other routes with host matchers; otherwise,
+ // we'll create a new server for all the listener addresses
+ // that are unused and serve the remaining redirects from it
+ for _, srv := range app.Servers {
if srv.hasListenerAddress(redirServerAddr) {
- // user has configured a server for the same address
- // that the redirect runs from; simply append our
- // redirect route to the existing routes, with a
- // caveat that their config might override ours
- app.logger.Warn("user server is listening on same interface as automatic HTTP->HTTPS redirects; user-configured routes might override these redirects",
- zap.String("server_name", srvName),
- zap.String("interface", redirServerAddr),
- )
- srv.Routes = append(srv.Routes, appendCatchAll(routes)...)
+ // find the index of the route after the last route with a host
+ // matcher, then insert the redirects there, but before any
+ // user-defined catch-all routes
+ // see https://github.com/caddyserver/caddy/issues/3212
+ insertIndex := srv.findLastRouteWithHostMatcher()
+ srv.Routes = append(srv.Routes[:insertIndex], append(routes, srv.Routes[insertIndex:]...)...)
+
+ // append our catch-all route in case the user didn't define their own
+ srv.Routes = appendCatchAll(srv.Routes)
+
continue redirServersLoop
}
}
diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go
index 27a1ca8..294ee6a 100644
--- a/modules/caddyhttp/server.go
+++ b/modules/caddyhttp/server.go
@@ -373,6 +373,37 @@ func (s *Server) hasTLSClientAuth() bool {
return false
}
+// findLastRouteWithHostMatcher returns the index of the last route
+// in the server which has a host matcher. Used during Automatic HTTPS
+// to determine where to insert the HTTP->HTTPS redirect route, such
+// that it is after any other host matcher but before any "catch-all"
+// route without a host matcher.
+func (s *Server) findLastRouteWithHostMatcher() int {
+ lastIndex := len(s.Routes)
+ for i, route := range s.Routes {
+ // since we want to break out of an inner loop, use a closure
+ // to allow us to use 'return' when we found a host matcher
+ found := (func() bool {
+ for _, sets := range route.MatcherSets {
+ for _, matcher := range sets {
+ switch matcher.(type) {
+ case *MatchHost:
+ return true
+ }
+ }
+ }
+ return false
+ })()
+
+ // if we found the host matcher, change the lastIndex to
+ // just after the current route
+ if found {
+ lastIndex = i + 1
+ }
+ }
+ return lastIndex
+}
+
// HTTPErrorConfig determines how to handle errors
// from the HTTP handlers.
type HTTPErrorConfig struct {