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.
This commit is contained in:
Matthew Holt 2020-01-19 11:51:17 -07:00
parent 07ef4b0c7d
commit aad9f90cad
No known key found for this signature in database
GPG key ID: 2A349DD577D586A5
3 changed files with 39 additions and 31 deletions

View file

@ -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
}

View file

@ -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},

View file

@ -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
}