From aad9f90cad23d709075073fd59214a51249de386 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sun, 19 Jan 2020 11:51:17 -0700 Subject: httpcaddyfile: Fix address parsing; don't infer port at parse-time Before, listener ports could be wrong because ParseAddress doesn't know about the user-configured HTTP/HTTPS ports, instead hard-coding port 80 or 443, which could be wrong if the user changed them to something else. Now we defer port and scheme validation/inference to a later part of building the output JSON. --- caddyconfig/httpcaddyfile/addresses.go | 48 +++++++++++++++++------------ caddyconfig/httpcaddyfile/addresses_test.go | 20 ++++++------ caddyconfig/httpcaddyfile/httptype.go | 2 +- 3 files changed, 39 insertions(+), 31 deletions(-) (limited to 'caddyconfig') diff --git a/caddyconfig/httpcaddyfile/addresses.go b/caddyconfig/httpcaddyfile/addresses.go index 431bb75..19ac197 100644 --- a/caddyconfig/httpcaddyfile/addresses.go +++ b/caddyconfig/httpcaddyfile/addresses.go @@ -72,7 +72,8 @@ import ( // repetition may be undesirable, so call consolidateAddrMappings() to map // multiple addresses to the same lists of server blocks (a many:many mapping). // (Doing this is essentially a map-reduce technique.) -func (st *ServerType) mapAddressToServerBlocks(originalServerBlocks []serverBlock) (map[string][]serverBlock, error) { +func (st *ServerType) mapAddressToServerBlocks(originalServerBlocks []serverBlock, + options map[string]interface{}) (map[string][]serverBlock, error) { sbmap := make(map[string][]serverBlock) for i, sblock := range originalServerBlocks { @@ -87,7 +88,7 @@ func (st *ServerType) mapAddressToServerBlocks(originalServerBlocks []serverBloc // arguments to the 'bind' directive (although they will all have // the same port, since the port is defined by the key or is implicit // through automatic HTTPS) - addrs, err := st.listenerAddrsForServerBlockKey(sblock, key) + addrs, err := st.listenerAddrsForServerBlockKey(sblock, key, options) if err != nil { return nil, fmt.Errorf("server block %d, key %d (%s): determining listener address: %v", i, j, key, err) } @@ -153,20 +154,43 @@ func (st *ServerType) consolidateAddrMappings(addrToServerBlocks map[string][]se return sbaddrs } -func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key string) ([]string, error) { +func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key string, + options map[string]interface{}) ([]string, error) { addr, err := ParseAddress(key) if err != nil { return nil, fmt.Errorf("parsing key: %v", err) } addr = addr.Normalize() + // figure out the HTTP and HTTPS ports; either + // use defaults, or override with user config + httpPort, httpsPort := strconv.Itoa(certmagic.HTTPPort), strconv.Itoa(certmagic.HTTPSPort) + if hport, ok := options["http_port"]; ok { + httpPort = strconv.Itoa(hport.(int)) + } + if hsport, ok := options["https_port"]; ok { + httpsPort = strconv.Itoa(hsport.(int)) + } + lnPort := DefaultPort if addr.Port != "" { // port explicitly defined lnPort = addr.Port + } else if addr.Scheme != "" { + // port inferred from scheme + if addr.Scheme == "http" { + lnPort = httpPort + } else if addr.Scheme == "https" { + lnPort = httpsPort + } } else if certmagic.HostQualifies(addr.Host) { // automatic HTTPS - lnPort = strconv.Itoa(certmagic.HTTPSPort) + lnPort = httpsPort + } + + // error if scheme and port combination violate convention + if (addr.Scheme == "http" && lnPort == httpsPort) || (addr.Scheme == "https" && lnPort == httpPort) { + return nil, fmt.Errorf("[%s] scheme and port violate convention", key) } // the bind directive specifies hosts, but is optional @@ -245,17 +269,6 @@ func ParseAddress(str string) (Address, error) { a.Path = "/" + hostSplit[1] } - httpPort, httpsPort := strconv.Itoa(certmagic.HTTPPort), strconv.Itoa(certmagic.HTTPSPort) - - // see if we can set port based off scheme - if a.Port == "" { - if a.Scheme == "http" { - a.Port = httpPort - } else if a.Scheme == "https" { - a.Port = httpsPort - } - } - // make sure port is valid if a.Port != "" { if portNum, err := strconv.Atoi(a.Port); err != nil { @@ -265,11 +278,6 @@ func ParseAddress(str string) (Address, error) { } } - // error if scheme and port combination violate convention - if (a.Scheme == "http" && a.Port == httpsPort) || (a.Scheme == "https" && a.Port == httpPort) { - return Address{}, fmt.Errorf("[%s] scheme and port violate convention", str) - } - return a, nil } diff --git a/caddyconfig/httpcaddyfile/addresses_test.go b/caddyconfig/httpcaddyfile/addresses_test.go index 70577f1..e22535c 100644 --- a/caddyconfig/httpcaddyfile/addresses_test.go +++ b/caddyconfig/httpcaddyfile/addresses_test.go @@ -28,17 +28,17 @@ func TestParseAddress(t *testing.T) { {`http://localhost:https`, "", "", "", "", true}, // conflict {`http://localhost:http`, "", "", "", "", true}, // repeated scheme {`host:https/path`, "", "", "", "", true}, - {`http://localhost:443`, "", "", "", "", true}, // not conventional - {`https://localhost:80`, "", "", "", "", true}, // not conventional - {`http://localhost`, "http", "localhost", "80", "", false}, - {`https://localhost`, "https", "localhost", "443", "", false}, - {`http://{env.APP_DOMAIN}`, "http", "{env.APP_DOMAIN}", "80", "", false}, + {`http://localhost:443`, "http", "localhost", "443", "", false}, // NOTE: not conventional + {`https://localhost:80`, "https", "localhost", "80", "", false}, // NOTE: not conventional + {`http://localhost`, "http", "localhost", "", "", false}, + {`https://localhost`, "https", "localhost", "", "", false}, + {`http://{env.APP_DOMAIN}`, "http", "{env.APP_DOMAIN}", "", "", false}, {`{env.APP_DOMAIN}:80`, "", "{env.APP_DOMAIN}", "80", "", false}, {`{env.APP_DOMAIN}/path`, "", "{env.APP_DOMAIN}", "", "/path", false}, {`example.com/{env.APP_PATH}`, "", "example.com", "", "/{env.APP_PATH}", false}, - {`http://127.0.0.1`, "http", "127.0.0.1", "80", "", false}, - {`https://127.0.0.1`, "https", "127.0.0.1", "443", "", false}, - {`http://[::1]`, "http", "::1", "80", "", false}, + {`http://127.0.0.1`, "http", "127.0.0.1", "", "", false}, + {`https://127.0.0.1`, "https", "127.0.0.1", "", "", false}, + {`http://[::1]`, "http", "::1", "", "", false}, {`http://localhost:1234`, "http", "localhost", "1234", "", false}, {`https://127.0.0.1:1234`, "https", "127.0.0.1", "1234", "", false}, {`http://[::1]:1234`, "http", "::1", "1234", "", false}, @@ -47,10 +47,10 @@ func TestParseAddress(t *testing.T) { {`localhost::`, "", "localhost::", "", "", false}, {`#$%@`, "", "#$%@", "", "", false}, // don't want to presume what the hostname could be {`host/path`, "", "host", "", "/path", false}, - {`http://host/`, "http", "host", "80", "/", false}, + {`http://host/`, "http", "host", "", "/", false}, {`//asdf`, "", "", "", "//asdf", false}, {`:1234/asdf`, "", "", "1234", "/asdf", false}, - {`http://host/path`, "http", "host", "80", "/path", false}, + {`http://host/path`, "http", "host", "", "/path", false}, {`https://host:443/path/foo`, "https", "host", "443", "/path/foo", false}, {`host:80/path`, "", "host", "80", "/path", false}, {`/path`, "", "", "", "/path", false}, diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index a57b6e9..20621bb 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -159,7 +159,7 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, } // map - sbmap, err := st.mapAddressToServerBlocks(serverBlocks) + sbmap, err := st.mapAddressToServerBlocks(serverBlocks, options) if err != nil { return nil, warnings, err } -- cgit v1.2.3