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
This commit is contained in:
Francis Lavoie 2021-04-19 21:54:12 -04:00 committed by GitHub
parent 96bb365929
commit d789596bc0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 69 additions and 14 deletions

View file

@ -80,3 +80,26 @@ func TestAutoHTTPRedirectsWithHTTPListenerFirstInAddresses(t *testing.T) {
`, "json") `, "json")
tester.AssertRedirect("http://localhost:9080/", "https://localhost/", http.StatusPermanentRedirect) 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")
}

View file

@ -342,21 +342,22 @@ redirServersLoop:
for redirServerAddr, routes := range redirServers { for redirServerAddr, routes := range redirServers {
// for each redirect listener, see if there's already a // for each redirect listener, see if there's already a
// server configured to listen on that exact address; if so, // server configured to listen on that exact address; if so,
// simply add the redirect route to the end of its route // insert the redirect route to the end of its route list
// list; otherwise, we'll create a new server for all the // after any other routes with host matchers; otherwise,
// listener addresses that are unused and serve the // we'll create a new server for all the listener addresses
// remaining redirects from it // that are unused and serve the remaining redirects from it
for srvName, srv := range app.Servers { for _, srv := range app.Servers {
if srv.hasListenerAddress(redirServerAddr) { if srv.hasListenerAddress(redirServerAddr) {
// user has configured a server for the same address // find the index of the route after the last route with a host
// that the redirect runs from; simply append our // matcher, then insert the redirects there, but before any
// redirect route to the existing routes, with a // user-defined catch-all routes
// caveat that their config might override ours // see https://github.com/caddyserver/caddy/issues/3212
app.logger.Warn("user server is listening on same interface as automatic HTTP->HTTPS redirects; user-configured routes might override these redirects", insertIndex := srv.findLastRouteWithHostMatcher()
zap.String("server_name", srvName), srv.Routes = append(srv.Routes[:insertIndex], append(routes, srv.Routes[insertIndex:]...)...)
zap.String("interface", redirServerAddr),
) // append our catch-all route in case the user didn't define their own
srv.Routes = append(srv.Routes, appendCatchAll(routes)...) srv.Routes = appendCatchAll(srv.Routes)
continue redirServersLoop continue redirServersLoop
} }
} }

View file

@ -373,6 +373,37 @@ func (s *Server) hasTLSClientAuth() bool {
return false 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 // HTTPErrorConfig determines how to handle errors
// from the HTTP handlers. // from the HTTP handlers.
type HTTPErrorConfig struct { type HTTPErrorConfig struct {