From d8391d6fbd3d6cb256bb5ac67937a38bc9f493fc Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 23 Jun 2015 22:00:55 -0600 Subject: [PATCH 1/2] core: Handle address lookup and bind errors more gracefully (fixes #136 and #164) Addresses which fail to resolve are handled more gracefully in the two most common cases: the hostname doesn't resolve or the port is unknown (like "http" on a system that doesn't support that port name). If the hostname doesn't resolve, the host is served on the listener at host 0.0.0.0. If the port is unknown, we attempt to rewrite it as a number manually and try again. --- config/config.go | 68 +++++++++++++++++++++++++++++++++++++++---- config/config_test.go | 61 ++++++++++++++++++++++++++++++++++++++ server/config.go | 3 -- 3 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 config/config_test.go diff --git a/config/config.go b/config/config.go index 1d652c429..97c0c9b6d 100644 --- a/config/config.go +++ b/config/config.go @@ -1,7 +1,6 @@ package config import ( - "errors" "fmt" "io" "log" @@ -89,18 +88,22 @@ func Load(filename string, input io.Reader) ([]server.Config, error) { // ArrangeBindings groups configurations by their bind address. For example, // a server that should listen on localhost and another on 127.0.0.1 will // be grouped into the same address: 127.0.0.1. It will return an error -// if the address lookup fails or if a TLS listener is configured on the +// if an address is malformed or a TLS listener is configured on the // same address as a plaintext HTTP listener. The return value is a map of // bind address to list of configs that would become VirtualHosts on that -// server. +// server. Use the keys of the returned map to create listeners, and use +// the associated values to set up the virtualhosts. func ArrangeBindings(allConfigs []server.Config) (map[*net.TCPAddr][]server.Config, error) { addresses := make(map[*net.TCPAddr][]server.Config) // Group configs by bind address for _, conf := range allConfigs { - newAddr, err := net.ResolveTCPAddr("tcp", conf.Address()) - if err != nil { - return addresses, errors.New("could not serve " + conf.Address() + " - " + err.Error()) + newAddr, warnErr, fatalErr := resolveAddr(conf) + if fatalErr != nil { + return addresses, fatalErr + } + if warnErr != nil { + log.Println("[Warning]", warnErr) } // Make sure to compare the string representation of the address, @@ -139,6 +142,59 @@ func ArrangeBindings(allConfigs []server.Config) (map[*net.TCPAddr][]server.Conf return addresses, nil } +// resolveAddr determines the address (host and port) that a config will +// bind to. The returned address, resolvAddr, should be used to bind the +// listener or group the config with other configs using the same address. +// The first error, if not nil, is just a warning and should be reported +// but execution may continue. The second error, if not nil, is a real +// problem and the server should not be started. +// +// This function handles edge cases gracefully. If a port name like +// "http" or "https" is unknown to the system, this function will +// change them to 80 or 443 respectively. If a hostname fails to +// resolve, that host can still be served but will be listening on +// the wildcard host instead. This function takes care of this for you. +func resolveAddr(conf server.Config) (resolvAddr *net.TCPAddr, warnErr error, fatalErr error) { + // The host to bind to may be different from the (virtual)host to serve + bindHost := conf.BindHost + if bindHost == "" { + bindHost = conf.Host + } + + resolvAddr, warnErr = net.ResolveTCPAddr("tcp", net.JoinHostPort(bindHost, conf.Port)) + if warnErr != nil { + // Most likely the host lookup failed or the port is unknown + tryPort := conf.Port + + switch errVal := warnErr.(type) { + case *net.AddrError: + if errVal.Err == "unknown port" { + // some odd Linux machines don't support these port names; see issue #136 + switch conf.Port { + case "http": + tryPort = "80" + case "https": + tryPort = "443" + } + } + resolvAddr, fatalErr = net.ResolveTCPAddr("tcp", net.JoinHostPort(bindHost, tryPort)) + if fatalErr != nil { + return + } + default: + // the hostname probably couldn't be resolved, just bind to wildcard then + resolvAddr, fatalErr = net.ResolveTCPAddr("tcp", net.JoinHostPort("0.0.0.0", tryPort)) + if fatalErr != nil { + return + } + } + + return + } + + return +} + // validDirective returns true if d is a valid // directive; false otherwise. func validDirective(d string) bool { diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 000000000..e0e4db23e --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,61 @@ +package config + +import ( + "testing" + + "github.com/mholt/caddy/server" +) + +func TestReolveAddr(t *testing.T) { + // NOTE: If tests fail due to comparing to string "127.0.0.1", + // it's possible that system env resolves with IPv6, or ::1. + // If that happens, maybe we should use actualAddr.IP.IsLoopback() + // for the assertion, rather than a direct string comparison. + + for i, test := range []struct { + config server.Config + shouldWarnErr bool + shouldFatalErr bool + expectedIP string + expectedPort int + }{ + {server.Config{Host: "localhost", Port: "1234"}, false, false, "127.0.0.1", 1234}, + {server.Config{Host: "127.0.0.1", Port: "1234"}, false, false, "127.0.0.1", 1234}, + {server.Config{Host: "should-not-resolve", Port: "1234"}, true, false, "0.0.0.0", 1234}, + {server.Config{Host: "localhost", Port: "http"}, false, false, "127.0.0.1", 80}, + {server.Config{Host: "localhost", Port: "https"}, false, false, "127.0.0.1", 443}, + {server.Config{Host: "", Port: ""}, false, true, "", 0}, + {server.Config{Host: "localhost", Port: ""}, false, true, "127.0.0.1", 0}, + {server.Config{Host: "", Port: "1234"}, false, false, "", 1234}, + {server.Config{Host: "localhost", Port: "abcd"}, false, true, "", 0}, + {server.Config{BindHost: "127.0.0.1", Host: "should-not-be-used", Port: "1234"}, false, false, "127.0.0.1", 1234}, + {server.Config{BindHost: "localhost", Host: "should-not-be-used", Port: "1234"}, false, false, "127.0.0.1", 1234}, + {server.Config{BindHost: "should-not-resolve", Host: "localhost", Port: "1234"}, true, false, "0.0.0.0", 1234}, + } { + actualAddr, warnErr, fatalErr := resolveAddr(test.config) + + if test.shouldFatalErr && fatalErr == nil { + t.Errorf("Test %d: Expected error, but there wasn't any", i) + } + if !test.shouldFatalErr && fatalErr != nil { + t.Errorf("Test %d: Expected no error, but there was one: %v", i, fatalErr) + } + if fatalErr != nil { + continue + } + + if test.shouldWarnErr && warnErr == nil { + t.Errorf("Test %d: Expected warning, but there wasn't any", i) + } + if !test.shouldWarnErr && warnErr != nil { + t.Errorf("Test %d: Expected no warning, but there was one: %v", i, warnErr) + } + + if actual, expected := actualAddr.IP.String(), test.expectedIP; actual != expected { + t.Errorf("Test %d: IP was %s but expected %s", i, actual, expected) + } + if actual, expected := actualAddr.Port, test.expectedPort; actual != expected { + t.Errorf("Test %d: Port was %d but expected %d", i, actual, expected) + } + } +} diff --git a/server/config.go b/server/config.go index 6575b7634..4bcea8b22 100644 --- a/server/config.go +++ b/server/config.go @@ -47,9 +47,6 @@ type Config struct { // Address returns the host:port of c as a string. func (c Config) Address() string { - if c.BindHost != "" { - return net.JoinHostPort(c.BindHost, c.Port) - } return net.JoinHostPort(c.Host, c.Port) } From 47d1f5eecf60eb10de8a942dbee10a51c474737d Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 23 Jun 2015 22:13:29 -0600 Subject: [PATCH 2/2] Removed tests that are not cross-platform compatible --- config/config_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index e0e4db23e..a0cb6ec2a 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -12,6 +12,9 @@ func TestReolveAddr(t *testing.T) { // If that happens, maybe we should use actualAddr.IP.IsLoopback() // for the assertion, rather than a direct string comparison. + // NOTE: Tests with {Host: "", Port: ""} and {Host: "localhost", Port: ""} + // will not behave the same cross-platform, so they have bene omitted. + for i, test := range []struct { config server.Config shouldWarnErr bool @@ -24,8 +27,6 @@ func TestReolveAddr(t *testing.T) { {server.Config{Host: "should-not-resolve", Port: "1234"}, true, false, "0.0.0.0", 1234}, {server.Config{Host: "localhost", Port: "http"}, false, false, "127.0.0.1", 80}, {server.Config{Host: "localhost", Port: "https"}, false, false, "127.0.0.1", 443}, - {server.Config{Host: "", Port: ""}, false, true, "", 0}, - {server.Config{Host: "localhost", Port: ""}, false, true, "127.0.0.1", 0}, {server.Config{Host: "", Port: "1234"}, false, false, "", 1234}, {server.Config{Host: "localhost", Port: "abcd"}, false, true, "", 0}, {server.Config{BindHost: "127.0.0.1", Host: "should-not-be-used", Port: "1234"}, false, false, "127.0.0.1", 1234},