Merge pull request #166 from mholt/graceful-resolve

core: Handle address lookup and bind errors more gracefully (fixes #136 and #164)

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())
+		newAddr, warnErr, fatalErr := resolveAddr(conf)
-		if err != nil {
+		if fatalErr != nil {
-			return addresses, errors.New("could not serve " + conf.Address() + " - " + err.Error())
+			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 {

config/config_test.go

@@ -0,0 +1,62 @@
+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.
+
+	// 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
+		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: "1234"}, false, false, "<nil>", 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)
+		}
+	}
+}

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