Merge branch 'master' into http3-default

.github/workflows/ci.yml

@@ -30,11 +30,6 @@ jobs:
         - go: '1.18'
           GO_SEMVER: '~1.18.1'
 
-        # Go 1.18.1 isn't released yet for Mac as of Apr 13 2022
-        - go: '1.18'
-          os: 'macos-latest'
-          GO_SEMVER: '1.18.0'
-
         # Set some variables per OS, usable via ${{ matrix.VAR }}
         # CADDY_BIN_PATH: the path to the compiled Caddy binary, for artifact publishing
         # SUCCESS: the typical value for $? per OS (Windows/pwsh returns 'True')
@@ -83,12 +78,20 @@ jobs:
         printf "Git version: $(git version)\n\n"
         # Calculate the short SHA1 hash of the git commit
         echo "::set-output name=short_sha::$(git rev-parse --short HEAD)"
-        echo "::set-output name=go_cache::$(go env GOCACHE)"
 
     - name: Cache the build cache
       uses: actions/cache@v2
       with:
-        path: ${{ steps.vars.outputs.go_cache }}
+        # In order:
+        # * Module download cache
+        # * Build cache (Linux)
+        # * Build cache (Mac)
+        # * Build cache (Windows)
+        path: |
+          ~/go/pkg/mod
+          ~/.cache/go-build
+          ~/Library/Caches/go-build
+          ~\AppData\Local\go-build
         key: ${{ runner.os }}-${{ matrix.go }}-go-ci-${{ hashFiles('**/go.sum') }}
         restore-keys: |
           ${{ runner.os }}-${{ matrix.go }}-go-ci

.github/workflows/cross-build.yml

@@ -42,12 +42,16 @@ jobs:
           go env
           printf "\n\nSystem environment:\n\n"
           env
-          echo "::set-output name=go_cache::$(go env GOCACHE)"
 
       - name: Cache the build cache
         uses: actions/cache@v2
         with:
-          path: ${{ steps.vars.outputs.go_cache }}
+          # In order:
+          # * Module download cache
+          # * Build cache (Linux)
+          path: |
+            ~/go/pkg/mod
+            ~/.cache/go-build
           key: cross-build-go${{ matrix.go }}-${{ matrix.goos }}-${{ hashFiles('**/go.sum') }}
           restore-keys: |
             cross-build-go${{ matrix.go }}-${{ matrix.goos }}

.github/workflows/release.yml

@@ -56,7 +56,6 @@ jobs:
         env
         echo "::set-output name=version_tag::${GITHUB_REF/refs\/tags\//}"
         echo "::set-output name=short_sha::$(git rev-parse --short HEAD)"
-        echo "::set-output name=go_cache::$(go env GOCACHE)"
 
         # Add "pip install" CLI tools to PATH
         echo ~/.local/bin >> $GITHUB_PATH
@@ -91,7 +90,12 @@ jobs:
     - name: Cache the build cache
       uses: actions/cache@v2
       with:
-        path: ${{ steps.vars.outputs.go_cache }}
+        # In order:
+        # * Module download cache
+        # * Build cache (Linux)
+        path: |
+          ~/go/pkg/mod
+          ~/.cache/go-build
         key: ${{ runner.os }}-go${{ matrix.go }}-release-${{ hashFiles('**/go.sum') }}
         restore-keys: |
           ${{ runner.os }}-go${{ matrix.go }}-release

.gitignore

@@ -1,6 +1,7 @@
 _gitignore/
 *.log
 Caddyfile
+Caddyfile.*
 !caddyfile/
 
 # artifacts from pprof tooling

admin.go

@@ -25,6 +25,8 @@ import (
 	"errors"
 	"expvar"
 	"fmt"
+	"hash"
+	"hash/fnv"
 	"io"
 	"net"
 	"net/http"
@@ -894,16 +896,36 @@ func (h adminHandler) originAllowed(origin *url.URL) bool {
 	return false
 }
 
+// etagHasher returns a the hasher we used on the config to both
+// produce and verify ETags.
+func etagHasher() hash.Hash32 { return fnv.New32a() }
+
+// makeEtag returns an Etag header value (including quotes) for
+// the given config path and hash of contents at that path.
+func makeEtag(path string, hash hash.Hash) string {
+	return fmt.Sprintf(`"%s %x"`, path, hash.Sum(nil))
+}
+
 func handleConfig(w http.ResponseWriter, r *http.Request) error {
 	switch r.Method {
 	case http.MethodGet:
 		w.Header().Set("Content-Type", "application/json")
-
-		err := readConfig(r.URL.Path, w)
+		// Set the ETag as a trailer header.
+		// The alternative is to write the config to a buffer, and
+		// then hash that.
+		w.Header().Set("Trailer", "ETag")
+
+		hash := etagHasher()
+		configWriter := io.MultiWriter(w, hash)
+		err := readConfig(r.URL.Path, configWriter)
 		if err != nil {
 			return APIError{HTTPStatus: http.StatusBadRequest, Err: err}
 		}
 
+		// we could consider setting up a sync.Pool for the summed
+		// hashes to reduce GC pressure.
+		w.Header().Set("Etag", makeEtag(r.URL.Path, hash))
+
 		return nil
 
 	case http.MethodPost,
@@ -937,7 +959,7 @@ func handleConfig(w http.ResponseWriter, r *http.Request) error {
 
 		forceReload := r.Header.Get("Cache-Control") == "must-revalidate"
 
-		err := changeConfig(r.Method, r.URL.Path, body, forceReload)
+		err := changeConfig(r.Method, r.URL.Path, body, r.Header.Get("If-Match"), forceReload)
 		if err != nil && !errors.Is(err, errSameConfig) {
 			return err
 		}

admin_test.go

@@ -16,6 +16,8 @@ package caddy
 
 import (
 	"encoding/json"
+	"fmt"
+	"net/http"
 	"reflect"
 	"sync"
 	"testing"
@@ -139,10 +141,57 @@ func TestLoadConcurrent(t *testing.T) {
 			wg.Done()
 		}()
 	}
-
 	wg.Wait()
 }
 
+type fooModule struct {
+	IntField int
+	StrField string
+}
+
+func (fooModule) CaddyModule() ModuleInfo {
+	return ModuleInfo{
+		ID:  "foo",
+		New: func() Module { return new(fooModule) },
+	}
+}
+func (fooModule) Start() error { return nil }
+func (fooModule) Stop() error  { return nil }
+
+func TestETags(t *testing.T) {
+	RegisterModule(fooModule{})
+
+	if err := Load([]byte(`{"apps": {"foo": {"strField": "abc", "intField": 0}}}`), true); err != nil {
+		t.Fatalf("loading: %s", err)
+	}
+
+	const key = "/" + rawConfigKey + "/apps/foo"
+
+	// try update the config with the wrong etag
+	err := changeConfig(http.MethodPost, key, []byte(`{"strField": "abc", "intField": 1}}`), fmt.Sprintf(`"/%s not_an_etag"`, rawConfigKey), false)
+	if apiErr, ok := err.(APIError); !ok || apiErr.HTTPStatus != http.StatusPreconditionFailed {
+		t.Fatalf("expected precondition failed; got %v", err)
+	}
+
+	// get the etag
+	hash := etagHasher()
+	if err := readConfig(key, hash); err != nil {
+		t.Fatalf("reading: %s", err)
+	}
+
+	// do the same update with the correct key
+	err = changeConfig(http.MethodPost, key, []byte(`{"strField": "abc", "intField": 1}`), makeEtag(key, hash), false)
+	if err != nil {
+		t.Fatalf("expected update to work; got %v", err)
+	}
+
+	// now try another update. The hash should no longer match and we should get precondition failed
+	err = changeConfig(http.MethodPost, key, []byte(`{"strField": "abc", "intField": 2}`), makeEtag(key, hash), false)
+	if apiErr, ok := err.(APIError); !ok || apiErr.HTTPStatus != http.StatusPreconditionFailed {
+		t.Fatalf("expected precondition failed; got %v", err)
+	}
+}
+
 func BenchmarkLoad(b *testing.B) {
 	for i := 0; i < b.N; i++ {
 		Load(testCfg, true)

caddy.go

@@ -17,6 +17,7 @@ package caddy
 import (
 	"bytes"
 	"context"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -111,7 +112,7 @@ func Load(cfgJSON []byte, forceReload bool) error {
 		}
 	}()
 
-	err := changeConfig(http.MethodPost, "/"+rawConfigKey, cfgJSON, forceReload)
+	err := changeConfig(http.MethodPost, "/"+rawConfigKey, cfgJSON, "", forceReload)
 	if errors.Is(err, errSameConfig) {
 		err = nil // not really an error
 	}
@@ -125,7 +126,12 @@ func Load(cfgJSON []byte, forceReload bool) error {
 // occur unless forceReload is true. If the config is unchanged and not
 // forcefully reloaded, then errConfigUnchanged This function is safe for
 // concurrent use.
-func changeConfig(method, path string, input []byte, forceReload bool) error {
+// The ifMatchHeader can optionally be given a string of the format:
+//    "<path> <hash>"
+// where <path> is the absolute path in the config and <hash> is the expected hash of
+// the config at that path. If the hash in the ifMatchHeader doesn't match
+// the hash of the config, then an APIError with status 412 will be returned.
+func changeConfig(method, path string, input []byte, ifMatchHeader string, forceReload bool) error {
 	switch method {
 	case http.MethodGet,
 		http.MethodHead,
@@ -138,6 +144,40 @@ func changeConfig(method, path string, input []byte, forceReload bool) error {
 	currentCfgMu.Lock()
 	defer currentCfgMu.Unlock()
 
+	if ifMatchHeader != "" {
+		// expect the first and last character to be quotes
+		if len(ifMatchHeader) < 2 || ifMatchHeader[0] != '"' || ifMatchHeader[len(ifMatchHeader)-1] != '"' {
+			return APIError{
+				HTTPStatus: http.StatusBadRequest,
+				Err:        fmt.Errorf("malformed If-Match header; expect quoted string"),
+			}
+		}
+
+		// read out the parts
+		parts := strings.Fields(ifMatchHeader[1 : len(ifMatchHeader)-1])
+		if len(parts) != 2 {
+			return APIError{
+				HTTPStatus: http.StatusBadRequest,
+				Err:        fmt.Errorf("malformed If-Match header; expect format \"<path> <hash>\""),
+			}
+		}
+
+		// get the current hash of the config
+		// at the given path
+		hash := etagHasher()
+		err := unsyncedConfigAccess(http.MethodGet, parts[0], nil, hash)
+		if err != nil {
+			return err
+		}
+
+		if hex.EncodeToString(hash.Sum(nil)) != parts[1] {
+			return APIError{
+				HTTPStatus: http.StatusPreconditionFailed,
+				Err:        fmt.Errorf("If-Match header did not match current config hash"),
+			}
+		}
+	}
+
 	err := unsyncedConfigAccess(method, path, input, nil)
 	if err != nil {
 		return err
@@ -442,7 +482,7 @@ func run(newCfg *Config, start bool) error {
 
 	// Start
 	err = func() error {
-		var started []string
+		started := make([]string, 0, len(newCfg.apps))
 		for name, a := range newCfg.apps {
 			err := a.Start()
 			if err != nil {
@@ -500,7 +540,7 @@ func finishSettingUp(ctx Context, cfg *Config) error {
 
 		runLoadedConfig := func(config []byte) error {
 			logger.Info("applying dynamically-loaded config")
-			err := changeConfig(http.MethodPost, "/"+rawConfigKey, config, false)
+			err := changeConfig(http.MethodPost, "/"+rawConfigKey, config, "", false)
 			if errors.Is(err, errSameConfig) {
 				return err
 			}

caddyconfig/caddyfile/parse.go

@@ -18,13 +18,13 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"log"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 
 	"github.com/caddyserver/caddy/v2"
+	"go.uber.org/zap"
 )
 
 // Parse parses the input just enough to group tokens, in
@@ -393,7 +393,7 @@ func (p *parser) doImport() error {
 		}
 		if len(matches) == 0 {
 			if strings.ContainsAny(globPattern, "*?[]") {
-				log.Printf("[WARNING] No files matching import glob pattern: %s", importPattern)
+				caddy.Log().Warn("No files matching import glob pattern", zap.String("pattern", importPattern))
 			} else {
 				return p.Errf("File to import not found: %s", importPattern)
 			}

caddyconfig/httpcaddyfile/addresses.go

@@ -102,12 +102,20 @@ func (st *ServerType) mapAddressToServerBlocks(originalServerBlocks []serverBloc
 			}
 		}
 
+		// make a slice of the map keys so we can iterate in sorted order
+		addrs := make([]string, 0, len(addrToKeys))
+		for k := range addrToKeys {
+			addrs = append(addrs, k)
+		}
+		sort.Strings(addrs)
+
 		// now that we know which addresses serve which keys of this
 		// server block, we iterate that mapping and create a list of
 		// new server blocks for each address where the keys of the
 		// server block are only the ones which use the address; but
 		// the contents (tokens) are of course the same
-		for addr, keys := range addrToKeys {
+		for _, addr := range addrs {
+			keys := addrToKeys[addr]
 			// parse keys so that we only have to do it once
 			parsedKeys := make([]Address, 0, len(keys))
 			for _, key := range keys {
@@ -161,6 +169,7 @@ func (st *ServerType) consolidateAddrMappings(addrToServerBlocks map[string][]se
 				delete(addrToServerBlocks, otherAddr)
 			}
 		}
+		sort.Strings(a.addresses)
 
 		sbaddrs = append(sbaddrs, a)
 	}
@@ -208,13 +217,13 @@ func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key str
 	}
 
 	// the bind directive specifies hosts, but is optional
-	lnHosts := make([]string, 0, len(sblock.pile))
+	lnHosts := make([]string, 0, len(sblock.pile["bind"]))
 	for _, cfgVal := range sblock.pile["bind"] {
 		lnHosts = append(lnHosts, cfgVal.Value.([]string)...)
 	}
 	if len(lnHosts) == 0 {
-		if defaultBind, ok := options["default_bind"].(string); ok {
-			lnHosts = []string{defaultBind}
+		if defaultBind, ok := options["default_bind"].([]string); ok {
+			lnHosts = defaultBind
 		} else {
 			lnHosts = []string{""}
 		}
@@ -236,6 +245,7 @@ func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key str
 	for lnStr := range listeners {
 		listenersList = append(listenersList, lnStr)
 	}
+	sort.Strings(listenersList)
 
 	return listenersList, nil
 }