Parser separate scheme/port, refactor config loading

By separating scheme and port at the parser, we are able to set the port appropriately and also keep the semantics of the scheme being specified by the user later on. The parser also stores an address' original input. Also, the config refactor makes it possible to partially load a config - valuable for determining which ones will need Let's Encrypt integration turned on during a restart.

caddy/config.go

@@ -21,25 +21,22 @@ const (
 	DefaultConfigFile = "Caddyfile"
 )
 
-// loadConfigs reads input (named filename) and parses it, returning the
-// server configurations in the order they appeared in the input. As part
-// of this, it activates Let's Encrypt for the configs that are produced.
-// Thus, the returned configs are already optimally configured optimally
-// for HTTPS.
-func loadConfigs(filename string, input io.Reader) ([]server.Config, error) {
+// loadConfigsUpToIncludingTLS loads the configs from input with name filename and returns them,
+// the parsed server blocks, the index of the last directive it processed, and an error (if any).
+func loadConfigsUpToIncludingTLS(filename string, input io.Reader) ([]server.Config, []parse.ServerBlock, int, error) {
 	var configs []server.Config
 
 	// Each server block represents similar hosts/addresses, since they
 	// were grouped together in the Caddyfile.
 	serverBlocks, err := parse.ServerBlocks(filename, input, true)
 	if err != nil {
-		return nil, err
+		return nil, nil, 0, err
 	}
 	if len(serverBlocks) == 0 {
 		newInput := DefaultInput()
 		serverBlocks, err = parse.ServerBlocks(newInput.Path(), bytes.NewReader(newInput.Body()), true)
 		if err != nil {
-			return nil, err
+			return nil, nil, 0, err
 		}
 	}
 
@@ -56,6 +53,7 @@ func loadConfigs(filename string, input io.Reader) ([]server.Config, error) {
 			config := server.Config{
 				Host:       addr.Host,
 				Port:       addr.Port,
+				Scheme:     addr.Scheme,
 				Root:       Root,
 				Middleware: make(map[string][]middleware.Middleware),
 				ConfigFile: filename,
@@ -88,7 +86,7 @@ func loadConfigs(filename string, input io.Reader) ([]server.Config, error) {
 					// execute setup function and append middleware handler, if any
 					midware, err := dir.setup(controller)
 					if err != nil {
-						return nil, err
+						return nil, nil, lastDirectiveIndex, err
 					}
 					if midware != nil {
 						// TODO: For now, we only support the default path scope /
@@ -109,22 +107,31 @@ func loadConfigs(filename string, input io.Reader) ([]server.Config, error) {
 		}
 	}
 
+	return configs, serverBlocks, lastDirectiveIndex, nil
+}
+
+// loadConfigs reads input (named filename) and parses it, returning the
+// server configurations in the order they appeared in the input. As part
+// of this, it activates Let's Encrypt for the configs that are produced.
+// Thus, the returned configs are already optimally configured for HTTPS.
+func loadConfigs(filename string, input io.Reader) ([]server.Config, error) {
+	configs, serverBlocks, lastDirectiveIndex, err := loadConfigsUpToIncludingTLS(filename, input)
+	if err != nil {
+		return nil, err
+	}
+
 	// Now we have all the configs, but they have only been set up to the
 	// point of tls. We need to activate Let's Encrypt before setting up
 	// the rest of the middlewares so they have correct information regarding
-	// TLS configuration, if necessary. (this call is append-only, so our
-	// iterations below shouldn't be affected)
+	// TLS configuration, if necessary. (this only appends, so our iterations
+	// over server blocks below shouldn't be affected)
 	if !IsRestart() && !Quiet {
 		fmt.Print("Activating privacy features...")
 	}
 	configs, err = letsencrypt.Activate(configs)
 	if err != nil {
-		if !Quiet {
-			fmt.Println()
-		}
 		return nil, err
-	}
-	if !IsRestart() && !Quiet {
+	} else if !IsRestart() && !Quiet {
 		fmt.Println(" done.")
 	}
 
@@ -277,46 +284,19 @@ func arrangeBindings(allConfigs []server.Config) (bindingGroup, error) {
 // 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.
+// This function does not handle edge cases like port "http" or "https" if
+// they are not known to the system. It does, however, serve on the wildcard
+// host if resolving the address of the specific hostname fails.
 func resolveAddr(conf server.Config) (resolvAddr *net.TCPAddr, warnErr, fatalErr error) {
-	bindHost := conf.BindHost
-
-	// TODO: Do we even need the port? Maybe we just need to look up the host.
-	resolvAddr, warnErr = net.ResolveTCPAddr("tcp", net.JoinHostPort(bindHost, conf.Port))
+	resolvAddr, warnErr = net.ResolveTCPAddr("tcp", net.JoinHostPort(conf.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))
+		resolvAddr, fatalErr = net.ResolveTCPAddr("tcp", net.JoinHostPort("", conf.Port))
 		if fatalErr != nil {
 			return
 		}
 	}
 
-		return
-	}
-
 	return
 }
 
@@ -334,12 +314,12 @@ func validDirective(d string) bool {
 // DefaultInput returns the default Caddyfile input
 // to use when it is otherwise empty or missing.
 // It uses the default host and port (depends on
-// host, e.g. localhost is 2015, otherwise https) and
+// host, e.g. localhost is 2015, otherwise 443) and
 // root.
 func DefaultInput() CaddyfileInput {
 	port := Port
-	if letsencrypt.HostQualifies(Host) {
-		port = "https"
+	if letsencrypt.HostQualifies(Host) && port == DefaultPort {
+		port = "443"
 	}
 	return CaddyfileInput{
 		Contents: []byte(fmt.Sprintf("%s:%s\nroot %s", Host, port, Root)),

caddy/config_test.go

@@ -13,10 +13,10 @@ func TestDefaultInput(t *testing.T) {
 		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
 	}
 
-	// next few tests simulate user providing -host flag
+	// next few tests simulate user providing -host and/or -port flags
 
 	Host = "not-localhost.com"
-	if actual, expected := string(DefaultInput().Body()), "not-localhost.com:https\nroot ."; actual != expected {
+	if actual, expected := string(DefaultInput().Body()), "not-localhost.com:443\nroot ."; actual != expected {
 		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
 	}
 
@@ -29,6 +29,18 @@ func TestDefaultInput(t *testing.T) {
 	if actual, expected := string(DefaultInput().Body()), "127.0.1.1:2015\nroot ."; actual != expected {
 		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
 	}
+
+	Host = "not-localhost.com"
+	Port = "1234"
+	if actual, expected := string(DefaultInput().Body()), "not-localhost.com:1234\nroot ."; actual != expected {
+		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
+	}
+
+	Host = DefaultHost
+	Port = "1234"
+	if actual, expected := string(DefaultInput().Body()), ":1234\nroot ."; actual != expected {
+		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
+	}
 }
 
 func TestResolveAddr(t *testing.T) {
@@ -51,14 +63,14 @@ func TestResolveAddr(t *testing.T) {
 		{server.Config{Host: "localhost", Port: "80"}, false, false, "<nil>", 80},
 		{server.Config{BindHost: "localhost", Port: "1234"}, false, false, "127.0.0.1", 1234},
 		{server.Config{BindHost: "127.0.0.1", Port: "1234"}, false, false, "127.0.0.1", 1234},
-		{server.Config{BindHost: "should-not-resolve", Port: "1234"}, true, false, "0.0.0.0", 1234},
+		{server.Config{BindHost: "should-not-resolve", Port: "1234"}, true, false, "<nil>", 1234},
 		{server.Config{BindHost: "localhost", Port: "http"}, false, false, "127.0.0.1", 80},
 		{server.Config{BindHost: "localhost", Port: "https"}, false, false, "127.0.0.1", 443},
 		{server.Config{BindHost: "", Port: "1234"}, false, false, "<nil>", 1234},
 		{server.Config{BindHost: "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},
+		{server.Config{BindHost: "should-not-resolve", Host: "localhost", Port: "1234"}, true, false, "<nil>", 1234},
 	} {
 		actualAddr, warnErr, fatalErr := resolveAddr(test.config)
 

caddy/parse/parse.go

@@ -8,7 +8,7 @@ import "io"
 // If checkDirectives is true, only valid directives will be allowed
 // otherwise we consider it a parse error. Server blocks are returned
 // in the order in which they appear.
-func ServerBlocks(filename string, input io.Reader, checkDirectives bool) ([]serverBlock, error) {
+func ServerBlocks(filename string, input io.Reader, checkDirectives bool) ([]ServerBlock, error) {
 	p := parser{Dispenser: NewDispenser(filename, input)}
 	p.checkDirectives = checkDirectives
 	blocks, err := p.parseAll()

caddy/parse/parsing.go

 package parse
 
 import (
+	"fmt"
 	"net"
 	"os"
 	"path/filepath"
@@ -9,13 +10,13 @@ import (
 
 type parser struct {
 	Dispenser
-	block           serverBlock // current server block being parsed
+	block           ServerBlock // current server block being parsed
 	eof             bool        // if we encounter a valid EOF in a hard place
 	checkDirectives bool        // if true, directives must be known
 }
 
-func (p *parser) parseAll() ([]serverBlock, error) {
-	var blocks []serverBlock
+func (p *parser) parseAll() ([]ServerBlock, error) {
+	var blocks []ServerBlock
 
 	for p.Next() {
 		err := p.parseOne()
@@ -31,7 +32,7 @@ func (p *parser) parseAll() ([]serverBlock, error) {
 }
 
 func (p *parser) parseOne() error {
-	p.block = serverBlock{Tokens: make(map[string][]token)}
+	p.block = ServerBlock{Tokens: make(map[string][]token)}
 
 	err := p.begin()
 	if err != nil {
@@ -99,11 +100,11 @@ func (p *parser) addresses() error {
 			}
 
 			// Parse and save this address
-			host, port, err := standardAddress(tkn)
+			addr, err := standardAddress(tkn)
 			if err != nil {
 				return err
 			}
-			p.block.Addresses = append(p.block.Addresses, address{host, port})
+			p.block.Addresses = append(p.block.Addresses, addr)
 		}
 
 		// Advance token and possibly break out of loop or return error
@@ -273,39 +274,57 @@ func (p *parser) closeCurlyBrace() error {
 	return nil
 }
 
-// standardAddress turns the accepted host and port patterns
-// into a format accepted by net.Dial.
-func standardAddress(str string) (host, port string, err error) {
-	var schemePort, splitPort string
+// standardAddress parses an address string into a structured format with separate
+// scheme, host, and port portions, as well as the original input string.
+func standardAddress(str string) (address, error) {
+	var scheme string
+	var err error
 
+	// first check for scheme and strip it off
+	input := str
 	if strings.HasPrefix(str, "https://") {
-		schemePort = "https"
+		scheme = "https"
 		str = str[8:]
 	} else if strings.HasPrefix(str, "http://") {
-		schemePort = "http"
+		scheme = "http"
 		str = str[7:]
 	}
 
-	host, splitPort, err = net.SplitHostPort(str)
+	// separate host and port
+	host, port, err := net.SplitHostPort(str)
 	if err != nil {
-		host, splitPort, err = net.SplitHostPort(str + ":") // tack on empty port
+		host, port, err = net.SplitHostPort(str + ":")
+		// no error check here; return err at end of function
+	}
+
+	// see if we can set port based off scheme
+	if port == "" {
+		if scheme == "http" {
+			port = "80"
+		} else if scheme == "https" {
+			port = "443"
 		}
-	if err != nil {
-		// ¯\_(ツ)_/¯
-		host = str
 	}
 
-	if splitPort != "" {
-		port = splitPort
-	} else {
-		port = schemePort
+	// repeated or conflicting scheme is confusing, so error
+	if scheme != "" && (port == "http" || port == "https") {
+		return address{}, fmt.Errorf("[%s] scheme specified twice in address", str)
+	}
+
+	// standardize http and https ports to their respective port numbers
+	if port == "http" {
+		scheme = "http"
+		port = "80"
+	} else if port == "https" {
+		scheme = "https"
+		port = "443"
 	}
 
-	return
+	return address{Original: input, Scheme: scheme, Host: host, Port: port}, err
 }
 
 // replaceEnvVars replaces environment variables that appear in the token
-// and understands both the Unix $SYNTAX and Windows %SYNTAX%.
+// and understands both the $UNIX and %WINDOWS% syntaxes.
 func replaceEnvVars(s string) string {
 	s = replaceEnvReferences(s, "{%", "%}")
 	s = replaceEnvReferences(s, "{$", "}")
@@ -330,26 +349,26 @@ func replaceEnvReferences(s, refStart, refEnd string) string {
 }
 
 type (
-	// serverBlock associates tokens with a list of addresses
+	// ServerBlock associates tokens with a list of addresses
 	// and groups tokens by directive name.
-	serverBlock struct {
+	ServerBlock struct {
 		Addresses []address
 		Tokens    map[string][]token
 	}
 
 	address struct {
-		Host, Port string
+		Original, Scheme, Host, Port string
 	}
 )
 
-// HostList converts the list of addresses (hosts)
-// that are associated with this server block into
-// a slice of strings. Each string is a host:port
-// combination.
-func (sb serverBlock) HostList() []string {
+// HostList converts the list of addresses that are
+// associated with this server block into a slice of
+// strings, where each address is as it was originally
+// read from the input.
+func (sb ServerBlock) HostList() []string {
 	sbHosts := make([]string, len(sb.Addresses))
 	for j, addr := range sb.Addresses {
-		sbHosts[j] = net.JoinHostPort(addr.Host, addr.Port)
+		sbHosts[j] = addr.Original
 	}
 	return sbHosts
 }

server/config.go

@@ -17,6 +17,9 @@ type Config struct {
 	// The port to listen on
 	Port string
 
+	// The protocol (http/https) to serve with this config; only set if user explicitly specifies it
+	Scheme string
+
 	// The directory from which to serve files
 	Root string
 
@@ -66,6 +69,7 @@ type TLSConfig struct {
 	Certificate      string
 	Key              string
 	LetsEncryptEmail string
+	//DisableHTTPRedir         bool // TODO: not a good idea - should we really allow it?
 	OCSPStaple               []byte
 	Ciphers                  []uint16
 	ProtocolMinVersion       uint16

server/config_test.go

@@ -18,8 +18,8 @@ func TestConfigAddress(t *testing.T) {
 		t.Errorf("Expected '%s' but got '%s'", expected, actual)
 	}
 
-	cfg = Config{Host: "::1", Port: "https"}
-	if actual, expected := cfg.Address(), "[::1]:https"; expected != actual {
+	cfg = Config{Host: "::1", Port: "443"}
+	if actual, expected := cfg.Address(), "[::1]:443"; expected != actual {
 		t.Errorf("Expected '%s' but got '%s'", expected, actual)
 	}
 }