From d789596bc0b014c99d75c00fe8e55c40ee3d58e3 Mon Sep 17 00:00:00 2001
From: Francis Lavoie <lavofr@gmail.com>
Date: Mon, 19 Apr 2021 21:54:12 -0400
Subject: 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
---
 modules/caddyhttp/autohttps.go | 29 +++++++++++++++--------------
 modules/caddyhttp/server.go    | 31 +++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 14 deletions(-)

(limited to 'modules/caddyhttp')

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 {
-- 
cgit v1.2.3