Repository: caddyserver/caddy (language: go)

Issue:
Host in health_checks should not require capitalization
Consider this `reverse_proxy` handler with `health_checks`. It's not forwarding the right host header and therefore the check is constantly failing.

```yaml
- handler: reverse_proxy
  health_checks:
    active:
      uri: /health
      interval: 5s
      expect_body: "Healthy"
      headers:
        host: [ "something.test.com" ]
        blaat: [ "blaat2" ]
  upstreams:
    - dial: "localhost:9003"
```

After debugging I found that [this piece of code is responsible](https://github.com/caddyserver/caddy/blob/a7db0cfe55b524a34a310e852af6c9520134db10/modules/caddyhttp/reverseproxy/healthchecks.go#L284). Work arround is to supply the `host` header key in [it's canonical form](https://golangbyexample.com/canonical-http-header-key/). (so `Host` instead of `host`).

The [header Get function](https://cs.opensource.google/go/go/+/refs/tags/go1.20.2:src/net/http/header.go;l=49) actually describes nicely what happens:
``` 
Get assumes that all keys are stored in canonical form. To use non-canonical keys, access the map directly.
```

It's a small issue, which direction would be appreciated if I would create a PR (there is some opinion given #4330, but I find it hard to apply here)?
1. Just access the map directly and let the behaviour as much as it is (other headers are also just forwarded as is).
2. Have a 'better' parser that immediately converts the headers into their canonical form. Then the header.Get function can just be used as is (and the `strings.ToLower(key) == "host"` could also be simplified to something without the ToLower). The downside of this is that caddy would become more opinionated on the header casing (in theory not a problem since HTTP requires you to not depend on casing). 

I would like to do this to prevent somebody else from having to spend 2 hours finding out why the health check is not working. ⌛ 



Discussion / hints:

Fix patch:
diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go
index c27b24f7e0f..386d272bbba 100644
--- a/modules/caddyhttp/reverseproxy/healthchecks.go
+++ b/modules/caddyhttp/reverseproxy/healthchecks.go
@@ -24,7 +24,6 @@ import (
 	"regexp"
 	"runtime/debug"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/caddyserver/caddy/v2"
@@ -106,6 +105,76 @@ type ActiveHealthChecks struct {
 	logger     *zap.Logger
 }
 
+// Provision ensures that a is set up properly before use.
+func (a *ActiveHealthChecks) Provision(ctx caddy.Context, h *Handler) error {
+	if !a.IsEnabled() {
+		return nil
+	}
+
+	// Canonicalize the header keys ahead of time, since
+	// JSON unmarshaled headers may be incorrect
+	cleaned := http.Header{}
+	for key, hdrs := range a.Headers {
+		for _, val := range hdrs {
+			cleaned.Add(key, val)
+		}
+	}
+	a.Headers = cleaned
+
+	h.HealthChecks.Active.logger = h.logger.Named("health_checker.active")
+
+	timeout := time.Duration(a.Timeout)
+	if timeout == 0 {
+		timeout = 5 * time.Second
+	}
+
+	if a.Path != "" {
+		a.logger.Warn("the 'path' option is deprecated, please use 'uri' instead!")
+	}
+
+	// parse the URI string (supports path and query)
+	if a.URI != "" {
+		parsedURI, err := url.Parse(a.URI)
+		if err != nil {
+			return err
+		}
+		a.uri = parsedURI
+	}
+
+	a.httpClient = &http.Client{
+		Timeout:   timeout,
+		Transport: h.Transport,
+	}
+
+	for _, upstream := range h.Upstreams {
+		// if there's an alternative port for health-check provided in the config,
+		// then use it, otherwise use the port of upstream.
+		if a.Port != 0 {
+			upstream.activeHealthCheckPort = a.Port
+		}
+	}
+
+	if a.Interval == 0 {
+		a.Interval = caddy.Duration(30 * time.Second)
+	}
+
+	if a.ExpectBody != "" {
+		var err error
+		a.bodyRegexp, err = regexp.Compile(a.ExpectBody)
+		if err != nil {
+			return fmt.Errorf("expect_body: compiling regular expression: %v", err)
+		}
+	}
+
+	return nil
+}
+
+// IsEnabled checks if the active health checks have
+// the minimum config necessary to be enabled.
+func (a *ActiveHealthChecks) IsEnabled() bool {
+	return a.Path != "" || a.URI != "" || a.Port != 0
+}
+
 // PassiveHealthChecks holds configuration related to passive
 // health checks (that is, health checks which occur during
 // the normal flow of request proxying).
@@ -280,7 +349,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, upstre
 	ctx = context.WithValue(ctx, caddyhttp.OriginalRequestCtxKey, *req)
 	req = req.WithContext(ctx)
 	for key, hdrs := range h.HealthChecks.Active.Headers {
-		if strings.ToLower(key) == "host" {
+		if key == "Host" {
 			req.Host = h.HealthChecks.Active.Headers.Get(key)
 		} else {
 			req.Header[key] = hdrs
diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go
index 8fd24fe4e73..bf507c7ecd9 100644
--- a/modules/caddyhttp/reverseproxy/reverseproxy.go
+++ b/modules/caddyhttp/reverseproxy/reverseproxy.go
@@ -27,7 +27,6 @@ import (
 	"net/netip"
 	"net/textproto"
 	"net/url"
-	"regexp"
 	"runtime"
 	"strconv"
 	"strings"
@@ -369,56 +368,15 @@ func (h *Handler) Provision(ctx caddy.Context) error {
 		}
 
 		// if active health checks are enabled, configure them and start a worker
-		if h.HealthChecks.Active != nil && (h.HealthChecks.Active.Path != "" ||
-			h.HealthChecks.Active.URI != "" ||
-			h.HealthChecks.Active.Port != 0) {
-
-			h.HealthChecks.Active.logger = h.logger.Named("health_checker.active")
-
-			timeout := time.Duration(h.HealthChecks.Active.Timeout)
-			if timeout == 0 {
-				timeout = 5 * time.Second
-			}
-
-			if h.HealthChecks.Active.Path != "" {
-				h.HealthChecks.Active.logger.Warn("the 'path' option is deprecated, please use 'uri' instead!")
-			}
-
-			// parse the URI string (supports path and query)
-			if h.HealthChecks.Active.URI != "" {
-				parsedURI, err := url.Parse(h.HealthChecks.Active.URI)
-				if err != nil {
-					return err
-				}
-				h.HealthChecks.Active.uri = parsedURI
-			}
-
-			h.HealthChecks.Active.httpClient = &http.Client{
-				Timeout:   timeout,
-				Transport: h.Transport,
-			}
-
-			for _, upstream := range h.Upstreams {
-				// if there's an alternative port for health-check provided in the config,
-				// then use it, otherwise use the port of upstream.
-				if h.HealthChecks.Active.Port != 0 {
-					upstream.activeHealthCheckPort = h.HealthChecks.Active.Port
-				}
-			}
-
-			if h.HealthChecks.Active.Interval == 0 {
-				h.HealthChecks.Active.Interval = caddy.Duration(30 * time.Second)
+		if h.HealthChecks.Active != nil {
+			err := h.HealthChecks.Active.Provision(ctx, h)
+			if err != nil {
+				return err
 			}
 
-			if h.HealthChecks.Active.ExpectBody != "" {
-				var err error
-				h.HealthChecks.Active.bodyRegexp, err = regexp.Compile(h.HealthChecks.Active.ExpectBody)
-				if err != nil {
-					return fmt.Errorf("expect_body: compiling regular expression: %v", err)
-				}
+			if h.HealthChecks.Active.IsEnabled() {
+				go h.activeHealthChecker()
 			}
-
-			go h.activeHealthChecker()
 		}
 	}
 


Test patch: