From d058dee11d7cfcf0b711f0378d10c9e5cabc8982 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Sat, 5 Mar 2022 18:34:19 -0500 Subject: reverseproxy: Refactor dial address parsing, augment command parsing (#4616) --- modules/caddyhttp/reverseproxy/addresses.go | 112 ++++++++++++++++++++++++++++ modules/caddyhttp/reverseproxy/caddyfile.go | 108 ++++----------------------- modules/caddyhttp/reverseproxy/command.go | 27 +------ 3 files changed, 129 insertions(+), 118 deletions(-) create mode 100644 modules/caddyhttp/reverseproxy/addresses.go (limited to 'modules/caddyhttp') diff --git a/modules/caddyhttp/reverseproxy/addresses.go b/modules/caddyhttp/reverseproxy/addresses.go new file mode 100644 index 0000000..f15ed76 --- /dev/null +++ b/modules/caddyhttp/reverseproxy/addresses.go @@ -0,0 +1,112 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reverseproxy + +import ( + "fmt" + "net" + "net/url" + "strings" + + "github.com/caddyserver/caddy/v2" +) + +// 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. +// +// TODO: the logic in this function is kind of sensitive, we +// need to write tests before making any more changes to it +func parseUpstreamDialAddress(upstreamAddr string) (string, string, error) { + var network, scheme, host, port string + + if strings.Contains(upstreamAddr, "://") { + // we get a parsing error if a placeholder is specified + // 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") + } + + toURL, err := url.Parse(upstreamAddr) + if err != nil { + return "", "", fmt.Errorf("parsing upstream URL: %v", err) + } + + // there is currently no way to perform a URL rewrite between choosing + // 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") + } + + // ensure the port and scheme aren't in conflict + urlPort := toURL.Port() + if toURL.Scheme == "http" && urlPort == "443" { + return "", "", fmt.Errorf("upstream address has conflicting scheme (http://) and port (:443, the HTTPS port)") + } + if toURL.Scheme == "https" && urlPort == "80" { + return "", "", fmt.Errorf("upstream address has conflicting scheme (https://) and port (:80, the HTTP port)") + } + if toURL.Scheme == "h2c" && urlPort == "443" { + return "", "", fmt.Errorf("upstream address has conflicting scheme (h2c://) and port (:443, the HTTPS port)") + } + + // if port is missing, attempt to infer from scheme + if toURL.Port() == "" { + var toPort string + switch toURL.Scheme { + case "", "http", "h2c": + toPort = "80" + case "https": + toPort = "443" + } + toURL.Host = net.JoinHostPort(toURL.Hostname(), toPort) + } + + scheme, host, port = toURL.Scheme, toURL.Hostname(), toURL.Port() + } else { + // extract network manually, since caddy.ParseNetworkAddress() will always add one + if idx := strings.Index(upstreamAddr, "/"); idx >= 0 { + network = strings.ToLower(strings.TrimSpace(upstreamAddr[:idx])) + upstreamAddr = upstreamAddr[idx+1:] + } + var err error + host, port, err = net.SplitHostPort(upstreamAddr) + if err != nil { + host = upstreamAddr + } + // we can assume a port if only a hostname is specified, but use of a + // placeholder without a port likely means a port will be filled in + if port == "" && !strings.Contains(host, "{") { + port = "80" + } + } + + // 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 +} diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index d642d04..a1fbeb3 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -18,7 +18,6 @@ import ( "log" "net" "net/http" - "net/url" "reflect" "strconv" "strings" @@ -123,98 +122,6 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // prefixed with "@" for use with "handle_response" blocks h.responseMatchers = make(map[string]caddyhttp.ResponseMatcher) - // TODO: the logic in this function is kind of sensitive, we need - // to write tests before making any more changes to it - upstreamDialAddress := func(upstreamAddr string) (string, error) { - var network, scheme, host, port string - - if strings.Contains(upstreamAddr, "://") { - // we get a parsing error if a placeholder is specified - // so we return a more user-friendly error message instead - // to explain what to do instead - if strings.Contains(upstreamAddr, "{") { - return "", d.Err("due to parsing difficulties, placeholders are not allowed when an upstream address contains a scheme") - } - - toURL, err := url.Parse(upstreamAddr) - if err != nil { - return "", d.Errf("parsing upstream URL: %v", err) - } - - // there is currently no way to perform a URL rewrite between choosing - // a backend and proxying to it, so we cannot allow extra components - // in backend URLs - if toURL.Path != "" || toURL.RawQuery != "" || toURL.Fragment != "" { - return "", d.Err("for now, URLs for proxy upstreams only support scheme, host, and port components") - } - - // ensure the port and scheme aren't in conflict - urlPort := toURL.Port() - if toURL.Scheme == "http" && urlPort == "443" { - return "", d.Err("upstream address has conflicting scheme (http://) and port (:443, the HTTPS port)") - } - if toURL.Scheme == "https" && urlPort == "80" { - return "", d.Err("upstream address has conflicting scheme (https://) and port (:80, the HTTP port)") - } - if toURL.Scheme == "h2c" && urlPort == "443" { - return "", d.Err("upstream address has conflicting scheme (h2c://) and port (:443, the HTTPS port)") - } - - // if port is missing, attempt to infer from scheme - if toURL.Port() == "" { - var toPort string - switch toURL.Scheme { - case "", "http", "h2c": - toPort = "80" - case "https": - toPort = "443" - } - toURL.Host = net.JoinHostPort(toURL.Hostname(), toPort) - } - - scheme, host, port = toURL.Scheme, toURL.Hostname(), toURL.Port() - } else { - // extract network manually, since caddy.ParseNetworkAddress() will always add one - if idx := strings.Index(upstreamAddr, "/"); idx >= 0 { - network = strings.ToLower(strings.TrimSpace(upstreamAddr[:idx])) - upstreamAddr = upstreamAddr[idx+1:] - } - var err error - host, port, err = net.SplitHostPort(upstreamAddr) - if err != nil { - host = upstreamAddr - } - // we can assume a port if only a hostname is specified, but use of a - // placeholder without a port likely means a port will be filled in - if port == "" && !strings.Contains(host, "{") { - port = "80" - } - } - - // 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 { - return "", d.Errf("for now, all proxy upstreams must use the same scheme (transport protocol); expecting '%s://' but got '%s://'", - commonScheme, scheme) - } - commonScheme = scheme - - // 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), 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, nil - } - - return net.JoinHostPort(host, port), nil - } - // appendUpstream creates an upstream for address and adds // it to the list. If the address starts with "srv+" it is // treated as a SRV-based upstream, and any port will be @@ -224,10 +131,21 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if isSRV { address = strings.TrimPrefix(address, "srv+") } - dialAddr, err := upstreamDialAddress(address) + + dialAddr, scheme, err := parseUpstreamDialAddress(address) if err != nil { - return err + return d.WrapErr(err) } + + // 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 { + return d.Errf("for now, all proxy upstreams must use the same scheme (transport protocol); expecting '%s://' but got '%s://'", + commonScheme, scheme) + } + commonScheme = scheme + if isSRV { if host, _, err := net.SplitHostPort(dialAddr); err == nil { dialAddr = host diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index 9bbc681..121142c 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -18,7 +18,6 @@ import ( "encoding/json" "flag" "fmt" - "net" "net/http" "strconv" @@ -104,32 +103,14 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { } // set up the upstream address; assume missing information from given parts - toAddr, err := httpcaddyfile.ParseAddress(to) + toAddr, toScheme, err := parseUpstreamDialAddress(to) if err != nil { return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid upstream address %s: %v", to, err) } - if toAddr.Path != "" { - return caddy.ExitCodeFailedStartup, fmt.Errorf("paths are not allowed: %s", to) - } - if toAddr.Scheme == "" { - if toAddr.Port == httpsPort { - toAddr.Scheme = "https" - } else { - toAddr.Scheme = "http" - } - } - if toAddr.Port == "" { - if toAddr.Scheme == "http" { - toAddr.Port = httpPort - } else if toAddr.Scheme == "https" { - toAddr.Port = httpsPort - } - } // proceed to build the handler and server - ht := HTTPTransport{} - if toAddr.Scheme == "https" { + if toScheme == "https" { ht.TLS = new(TLSConfig) if insecure { ht.TLS.InsecureSkipVerify = true @@ -138,7 +119,7 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { handler := Handler{ TransportRaw: caddyconfig.JSONModuleObject(ht, "protocol", "http", nil), - Upstreams: UpstreamPool{{Dial: net.JoinHostPort(toAddr.Host, toAddr.Port)}}, + Upstreams: UpstreamPool{{Dial: toAddr}}, } if changeHost { @@ -185,7 +166,7 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { return caddy.ExitCodeFailedStartup, err } - fmt.Printf("Caddy proxying %s -> %s\n", fromAddr.String(), toAddr.String()) + fmt.Printf("Caddy proxying %s -> %s\n", fromAddr.String(), toAddr) select {} } -- cgit v1.2.3