Repository: caddyserver/caddy (language: go)

Issue:
reverse_proxy upstream directive interprolation throwing `strconv.ParseUint`
Hey team,

We are currently facing an issue with the Caddy in our docker compose stacks, currently using the Caddy Docker Image pinned on version '2'.

We see the following in our logs: `Error during parsing: parsing caddyfile tokens for 'reverse_proxy': /etc/caddy/Caddyfile:26 - Error during parsing: invalid start port: strconv.ParseUint: parsing "6{http.regexp.port.1}"`

It appears that the Interpolation for the `Matcher` / `Upstream` is not functioning as intended.

Caddyfile:
```
*.sandbox.application.localhost {
	@sandboxPort {
		header_regexp port Host ^([0-9]{3})\.sandbox\.
	}

	handle @sandboxPort {
		reverse_proxy app:6{re.port.1}
	}

	handle {
		redir {scheme}://application.localhost
	}
}
```

As a quick fix, we have pinned our docker compose files to version 2.6.4 (where is issue is not present).

Regards


Discussion / hints:
This is probably related to #4019. Can you download the CI build artifact and test it out? You can download it from [here](https://github.com/caddyserver/caddy/actions/runs/5760117333) or run `xcaddy build fix-args-replacer`.

Edit: Just tested and the PR isn't related. We have to `git bisect` it to figure it out.
potentially facing the same issue in our server, the logs point to a similar issue:
```
parsing caddyfile tokens for 'reverse_proxy': /opt/devbox/caddy/Caddyfile:31 - Error during parsing: invalid start port: strconv.ParseUint: parsing "{http.request.header.devbox": invalid syntax, import chain: ['']
```

Update: can confirm pinning Caddy version to 2.6.4 does make the problem go away.
The culprit is PR #5494 (75b690d248c7681dd974f6179c98a363af417a25). Now onto finding a fix.

Fix patch:
diff --git a/modules/caddyhttp/reverseproxy/addresses.go b/modules/caddyhttp/reverseproxy/addresses.go
index 6078f1194f7..09394973924 100644
--- a/modules/caddyhttp/reverseproxy/addresses.go
+++ b/modules/caddyhttp/reverseproxy/addresses.go
@@ -23,11 +23,43 @@ import (
 	"github.com/caddyserver/caddy/v2"
 )
 
+type parsedAddr struct {
+	network, scheme, host, port string
+	valid                       bool
+}
+
+func (p parsedAddr) dialAddr() string {
+	if !p.valid {
+		return ""
+	}
+	// for simplest possible config, we only need to include
+	// the network portion if the user specified one
+	if p.network != "" {
+		return caddy.JoinNetworkAddress(p.network, p.host, p.port)
+	}
+
+	// if the host is a placeholder, then we don't want to join with an empty port,
+	// because that would just append an extra ':' at the end of the address.
+	if p.port == "" && strings.Contains(p.host, "{") {
+		return p.host
+	}
+	return net.JoinHostPort(p.host, p.port)
+}
+func (p parsedAddr) rangedPort() bool {
+	return strings.Contains(p.port, "-")
+}
+func (p parsedAddr) replaceablePort() bool {
+	return strings.Contains(p.port, "{") && strings.Contains(p.port, "}")
+}
+func (p parsedAddr) isUnix() bool {
+	return caddy.IsUnixNetwork(p.network)
+}
+
 // parseUpstreamDialAddress parses configuration inputs for
 // the dial address, including support for a scheme in front
 // as a shortcut for the port number, and a network type,
 // for example 'unix' to dial a unix socket.
-func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) {
+func parseUpstreamDialAddress(upstreamAddr string) (parsedAddr, error) {
 	var network, scheme, host, port string
 
 	if strings.Contains(upstreamAddr, "://") {
@@ -35,7 +67,7 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) {
 		// so we return a more user-friendly error message instead
 		// to explain what to do instead
 		if strings.Contains(upstreamAddr, "{") {
-			return "", "", fmt.Errorf("due to parsing difficulties, placeholders are not allowed when an upstream address contains a scheme")
+			return parsedAddr{}, fmt.Errorf("due to parsing difficulties, placeholders are not allowed when an upstream address contains a scheme")
 		}
 
 		toURL, err := url.Parse(upstreamAddr)
@@ -46,19 +78,19 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) {
 			if strings.Contains(err.Error(), "invalid port") && strings.Contains(err.Error(), "-") {
 				index := strings.LastIndex(upstreamAddr, ":")
 				if index == -1 {
-					return "", "", fmt.Errorf("parsing upstream URL: %v", err)
+					return parsedAddr{}, fmt.Errorf("parsing upstream URL: %v", err)
 				}
 				portRange := upstreamAddr[index+1:]
 				if strings.Count(portRange, "-") != 1 {
-					return "", "", fmt.Errorf("parsing upstream URL: parse \"%v\": port range invalid: %v", upstreamAddr, portRange)
+					return parsedAddr{}, fmt.Errorf("parsing upstream URL: parse \"%v\": port range invalid: %v", upstreamAddr, portRange)
 				}
 				toURL, err = url.Parse(strings.ReplaceAll(upstreamAddr, portRange, "0"))
 				if err != nil {
-					return "", "", fmt.Errorf("parsing upstream URL: %v", err)
+					return parsedAddr{}, fmt.Errorf("parsing upstream URL: %v", err)
 				}
 				port = portRange
 			} else {
-				return "", "", fmt.Errorf("parsing upstream URL: %v", err)
+				return parsedAddr{}, fmt.Errorf("parsing upstream URL: %v", err)
 			}
 		}
 		if port == "" {
@@ -69,18 +101,18 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) {
 		// a backend and proxying to it, so we cannot allow extra components
 		// in backend URLs
 		if toURL.Path != "" || toURL.RawQuery != "" || toURL.Fragment != "" {
-			return "", "", fmt.Errorf("for now, URLs for proxy upstreams only support scheme, host, and port components")
+			return parsedAddr{}, fmt.Errorf("for now, URLs for proxy upstreams only support scheme, host, and port components")
 		}
 
 		// ensure the port and scheme aren't in conflict
 		if toURL.Scheme == "http" && port == "443" {
-			return "", "", fmt.Errorf("upstream address has conflicting scheme (http://) and port (:443, the HTTPS port)")
+			return parsedAddr{}, fmt.Errorf("upstream address has conflicting scheme (http://) and port (:443, the HTTPS port)")
 		}
 		if toURL.Scheme == "https" && port == "80" {
-			return "", "", fmt.Errorf("upstream address has conflicting scheme (https://) and port (:80, the HTTP port)")
+			return parsedAddr{}, fmt.Errorf("upstream address has conflicting scheme (https://) and port (:80, the HTTP port)")
 		}
 		if toURL.Scheme == "h2c" && port == "443" {
-			return "", "", fmt.Errorf("upstream address has conflicting scheme (h2c://) and port (:443, the HTTPS port)")
+			return parsedAddr{}, fmt.Errorf("upstream address has conflicting scheme (h2c://) and port (:443, the HTTPS port)")
 		}
 
 		// if port is missing, attempt to infer from scheme
@@ -112,18 +144,5 @@ func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) {
 		network = "unix"
 		scheme = "h2c"
 	}
-
-	// for simplest possible config, we only need to include
-	// the network portion if the user specified one
-	if network != "" {
-		return caddy.JoinNetworkAddress(network, host, port), scheme, nil
-	}
-
-	// if the host is a placeholder, then we don't want to join with an empty port,
-	// because that would just append an extra ':' at the end of the address.
-	if port == "" && strings.Contains(host, "{") {
-		return host, scheme, nil
-	}
-
-	return net.JoinHostPort(host, port), scheme, nil
+	return parsedAddr{network, scheme, host, port, true}, nil
 }
diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go
index 1d86bebd7e1..f5eb7a5d01d 100644
--- a/modules/caddyhttp/reverseproxy/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/caddyfile.go
@@ -146,7 +146,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	// appendUpstream creates an upstream for address and adds
 	// it to the list.
 	appendUpstream := func(address string) error {
-		dialAddr, scheme, err := parseUpstreamDialAddress(address)
+		pa, err := parseUpstreamDialAddress(address)
 		if err != nil {
 			return d.WrapErr(err)
 		}
@@ -154,21 +154,27 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 		// the underlying JSON does not yet support different
 		// transports (protocols or schemes) to each backend,
 		// so we remember the last one we see and compare them
-		if commonScheme != "" && scheme != commonScheme {
+		if commonScheme != "" && pa.scheme != commonScheme {
 			return d.Errf("for now, all proxy upstreams must use the same scheme (transport protocol); expecting '%s://' but got '%s://'",
-				commonScheme, scheme)
+				commonScheme, pa.scheme)
 		}
-		commonScheme = scheme
+		commonScheme = pa.scheme
 
-		parsedAddr, err := caddy.ParseNetworkAddress(dialAddr)
+		// if the port of upstream address contains a placeholder, only wrap it with the `Upstream` struct,
+		// delaying actual resolution of the address until request time.
+		if pa.replaceablePort() {
+			h.Upstreams = append(h.Upstreams, &Upstream{Dial: pa.dialAddr()})
+			return nil
+		}
+		parsedAddr, err := caddy.ParseNetworkAddress(pa.dialAddr())
 		if err != nil {
 			return d.WrapErr(err)
 		}
 
-		if parsedAddr.StartPort == 0 && parsedAddr.EndPort == 0 {
+		if pa.isUnix() || !pa.rangedPort() {
 			// unix networks don't have ports
 			h.Upstreams = append(h.Upstreams, &Upstream{
-				Dial: dialAddr,
+				Dial: pa.dialAddr(),
 			})
 		} else {
 			// expand a port range into multiple upstreams
diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go
index 48fabd56de1..8438b727830 100644
--- a/modules/caddyhttp/reverseproxy/command.go
+++ b/modules/caddyhttp/reverseproxy/command.go
@@ -132,14 +132,14 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) {
 	toAddresses := make([]string, len(to))
 	var toScheme string
 	for i, toLoc := range to {
-		addr, scheme, err := parseUpstreamDialAddress(toLoc)
+		addr, err := parseUpstreamDialAddress(toLoc)
 		if err != nil {
 			return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid upstream address %s: %v", toLoc, err)
 		}
-		if scheme != "" && toScheme == "" {
-			toScheme = scheme
+		if addr.scheme != "" && toScheme == "" {
+			toScheme = addr.scheme
 		}
-		toAddresses[i] = addr
+		toAddresses[i] = addr.dialAddr()
 	}
 
 	// proceed to build the handler and server


Test patch:
diff --git a/caddytest/integration/caddyfile_adapt/replaceable_upstream.txt b/caddytest/integration/caddyfile_adapt/replaceable_upstream.txt
new file mode 100644
index 00000000000..202e3304368
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/replaceable_upstream.txt
@@ -0,0 +1,100 @@
+*.sandbox.localhost {
+	@sandboxPort {
+		header_regexp first_label Host ^([0-9]{3})\.sandbox\.
+	}
+	handle @sandboxPort {
+		reverse_proxy {re.first_label.1}
+	}
+	handle {
+		redir {scheme}://application.localhost
+	}
+}
+
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"*.sandbox.localhost"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"group": "group2",
+											"handle": [
+												{
+													"handler": "subroute",
+													"routes": [
+														{
+															"handle": [
+																{
+																	"handler": "reverse_proxy",
+																	"upstreams": [
+																		{
+																			"dial": "{http.regexp.first_label.1}"
+																		}
+																	]
+																}
+															]
+														}
+													]
+												}
+											],
+											"match": [
+												{
+													"header_regexp": {
+														"Host": {
+															"name": "first_label",
+															"pattern": "^([0-9]{3})\\.sandbox\\."
+														}
+													}
+												}
+											]
+										},
+										{
+											"group": "group2",
+											"handle": [
+												{
+													"handler": "subroute",
+													"routes": [
+														{
+															"handle": [
+																{
+																	"handler": "static_response",
+																	"headers": {
+																		"Location": [
+																			"{http.request.scheme}://application.localhost"
+																		]
+																	},
+																	"status_code": 302
+																}
+															]
+														}
+													]
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/caddytest/integration/caddyfile_adapt/replaceable_upstream_partial_port.txt b/caddytest/integration/caddyfile_adapt/replaceable_upstream_partial_port.txt
new file mode 100644
index 00000000000..7fbcb5c78eb
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/replaceable_upstream_partial_port.txt
@@ -0,0 +1,100 @@
+*.sandbox.localhost {
+	@sandboxPort {
+		header_regexp port Host ^([0-9]{3})\.sandbox\.
+	}
+	handle @sandboxPort {
+		reverse_proxy app:6{re.port.1}
+	}
+	handle {
+		redir {scheme}://application.localhost
+	}
+}
+
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"*.sandbox.localhost"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"group": "group2",
+											"handle": [
+												{
+													"handler": "subroute",
+													"routes": [
+														{
+															"handle": [
+																{
+																	"handler": "reverse_proxy",
+																	"upstreams": [
+																		{
+																			"dial": "app:6{http.regexp.port.1}"
+																		}
+																	]
+																}
+															]
+														}
+													]
+												}
+											],
+											"match": [
+												{
+													"header_regexp": {
+														"Host": {
+															"name": "port",
+															"pattern": "^([0-9]{3})\\.sandbox\\."
+														}
+													}
+												}
+											]
+										},
+										{
+											"group": "group2",
+											"handle": [
+												{
+													"handler": "subroute",
+													"routes": [
+														{
+															"handle": [
+																{
+																	"handler": "static_response",
+																	"headers": {
+																		"Location": [
+																			"{http.request.scheme}://application.localhost"
+																		]
+																	},
+																	"status_code": 302
+																}
+															]
+														}
+													]
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/caddytest/integration/caddyfile_adapt/replaceable_upstream_port.txt b/caddytest/integration/caddyfile_adapt/replaceable_upstream_port.txt
new file mode 100644
index 00000000000..8f75c5bd067
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/replaceable_upstream_port.txt
@@ -0,0 +1,100 @@
+*.sandbox.localhost {
+	@sandboxPort {
+		header_regexp port Host ^([0-9]{3})\.sandbox\.
+	}
+	handle @sandboxPort {
+		reverse_proxy app:{re.port.1}
+	}
+	handle {
+		redir {scheme}://application.localhost
+	}
+}
+
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"*.sandbox.localhost"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"group": "group2",
+											"handle": [
+												{
+													"handler": "subroute",
+													"routes": [
+														{
+															"handle": [
+																{
+																	"handler": "reverse_proxy",
+																	"upstreams": [
+																		{
+																			"dial": "app:{http.regexp.port.1}"
+																		}
+																	]
+																}
+															]
+														}
+													]
+												}
+											],
+											"match": [
+												{
+													"header_regexp": {
+														"Host": {
+															"name": "port",
+															"pattern": "^([0-9]{3})\\.sandbox\\."
+														}
+													}
+												}
+											]
+										},
+										{
+											"group": "group2",
+											"handle": [
+												{
+													"handler": "subroute",
+													"routes": [
+														{
+															"handle": [
+																{
+																	"handler": "static_response",
+																	"headers": {
+																		"Location": [
+																			"{http.request.scheme}://application.localhost"
+																		]
+																	},
+																	"status_code": 302
+																}
+															]
+														}
+													]
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/modules/caddyhttp/reverseproxy/addresses_test.go b/modules/caddyhttp/reverseproxy/addresses_test.go
index 0c7ad7b317e..0c514194290 100644
--- a/modules/caddyhttp/reverseproxy/addresses_test.go
+++ b/modules/caddyhttp/reverseproxy/addresses_test.go
@@ -265,18 +265,18 @@ func TestParseUpstreamDialAddress(t *testing.T) {
 			expectScheme:   "h2c",
 		},
 	} {
-		actualHostPort, actualScheme, err := parseUpstreamDialAddress(tc.input)
+		actualAddr, err := parseUpstreamDialAddress(tc.input)
 		if tc.expectErr && err == nil {
 			t.Errorf("Test %d: Expected error but got %v", i, err)
 		}
 		if !tc.expectErr && err != nil {
 			t.Errorf("Test %d: Expected no error but got %v", i, err)
 		}
-		if actualHostPort != tc.expectHostPort {
-			t.Errorf("Test %d: Expected host and port '%s' but got '%s'", i, tc.expectHostPort, actualHostPort)
+		if actualAddr.dialAddr() != tc.expectHostPort {
+			t.Errorf("Test %d: input %s: Expected host and port '%s' but got '%s'", i, tc.input, tc.expectHostPort, actualAddr.dialAddr())
 		}
-		if actualScheme != tc.expectScheme {
-			t.Errorf("Test %d: Expected scheme '%s' but got '%s'", i, tc.expectScheme, actualScheme)
+		if actualAddr.scheme != tc.expectScheme {
+			t.Errorf("Test %d: Expected scheme '%s' but got '%s'", i, tc.expectScheme, actualAddr.scheme)
 		}
 	}
 }