From a58f240d3ecbb59285303746406cab50217f8d24 Mon Sep 17 00:00:00 2001
From: Francis Lavoie <lavofr@gmail.com>
Date: Fri, 25 Mar 2022 00:54:03 -0400
Subject: httpcaddyfile: Fix #4640 (auto-HTTPS edgecase) (#4661)

Guh, this is complicated.

Fixes #4640

This also follows up on #4398 (reverting it) which made a change that technically worked, but was incorrect. It changed the condition in `hostsFromKeysNotHTTP` from `&&` to `||`, but then the function no longer did what its name said it would do, and it would return hosts even if they were marked with `http://`, if they used a non-HTTP port. That wasn't the intent of it. The test added in there was kept though, because it is a valid usecase.

The actual fix is to check _earlier_ whether all the addresses explicitly have `http://`, and if so we can short circuit and skip considering the rest.
---
 caddyconfig/httpcaddyfile/directives.go | 13 ++++++++++++-
 caddyconfig/httpcaddyfile/httptype.go   |  2 +-
 caddyconfig/httpcaddyfile/tlsapp.go     |  6 ++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

(limited to 'caddyconfig')

diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go
index 425bf19..6b80e34 100644
--- a/caddyconfig/httpcaddyfile/directives.go
+++ b/caddyconfig/httpcaddyfile/directives.go
@@ -494,7 +494,7 @@ func (sb serverBlock) hostsFromKeysNotHTTP(httpPort string) []string {
 		if addr.Host == "" {
 			continue
 		}
-		if addr.Scheme != "http" || addr.Port != httpPort {
+		if addr.Scheme != "http" && addr.Port != httpPort {
 			hostMap[addr.Host] = struct{}{}
 		}
 	}
@@ -519,6 +519,17 @@ func (sb serverBlock) hasHostCatchAllKey() bool {
 	return false
 }
 
+// isAllHTTP returns true if all sb keys explicitly specify
+// the http:// scheme
+func (sb serverBlock) isAllHTTP() bool {
+	for _, addr := range sb.keys {
+		if addr.Scheme != "http" {
+			return false
+		}
+	}
+	return true
+}
+
 type (
 	// UnmarshalFunc is a function which can unmarshal Caddyfile
 	// tokens into zero or more config values using a Helper type.
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index d7716a4..f5dd68a 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -581,7 +581,7 @@ func (st *ServerType) serversFromPairings(
 			}
 
 			for _, addr := range sblock.keys {
-				// if server only uses HTTPS port, auto-HTTPS will not apply
+				// if server only uses HTTP port, auto-HTTPS will not apply
 				if listenersUseAnyPortOtherThan(srv.Listen, httpPort) {
 					// exclude any hosts that were defined explicitly with "http://"
 					// in the key from automated cert management (issue #2998)
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
index daaec95..76d7ebf 100644
--- a/caddyconfig/httpcaddyfile/tlsapp.go
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -101,6 +101,12 @@ func (st ServerType) buildTLSApp(
 		}
 
 		for _, sblock := range p.serverBlocks {
+			// check the scheme of all the site addresses,
+			// skip building AP if they all had http://
+			if sblock.isAllHTTP() {
+				continue
+			}
+
 			// get values that populate an automation policy for this block
 			ap, err := newBaseAutomationPolicy(options, warnings, true)
 			if err != nil {
-- 
cgit v1.2.3