logging: Fix skip_hosts with wildcards (caddyserver#5102)

Fix caddyserver#4859

Author: francislavoie

caddyconfig/httpcaddyfile/builtins.go

@@ -735,7 +735,7 @@ func parseLogHelper(h Helper, globalLogNames map[string]struct{}) ([]ConfigValue
 				// reference the default logger. See the
 				// setupNewDefault function in the logging
 				// package for where this is configured.
-				globalLogName = "default"
+				globalLogName = caddy.DefaultLoggerName
 			}
 
 			// Verify this name is unused.

caddyconfig/httpcaddyfile/httptype.go

@@ -219,7 +219,7 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
 		if ncl.name == "" {
 			return
 		}
-		if ncl.name == "default" {
+		if ncl.name == caddy.DefaultLoggerName {
 			hasDefaultLog = true
 		}
 		if _, ok := options["debug"]; ok && ncl.log.Level == "" {
@@ -240,7 +240,7 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
 		// configure it with any applicable options
 		if _, ok := options["debug"]; ok {
 			customLogs = append(customLogs, namedCustomLog{
-				name: "default",
+				name: caddy.DefaultLoggerName,
 				log:  &caddy.CustomLog{Level: zap.DebugLevel.CapitalString()},
 			})
 		}
@@ -299,11 +299,11 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
 			// most users seem to prefer not writing access logs
 			// to the default log when they are directed to a
 			// file or have any other special customization
-			if ncl.name != "default" && len(ncl.log.Include) > 0 {
-				defaultLog, ok := cfg.Logging.Logs["default"]
+			if ncl.name != caddy.DefaultLoggerName && len(ncl.log.Include) > 0 {
+				defaultLog, ok := cfg.Logging.Logs[caddy.DefaultLoggerName]
 				if !ok {
 					defaultLog = new(caddy.CustomLog)
-					cfg.Logging.Logs["default"] = defaultLog
+					cfg.Logging.Logs[caddy.DefaultLoggerName] = defaultLog
 				}
 				defaultLog.Exclude = append(defaultLog.Exclude, ncl.log.Include...)
 			}
@@ -518,15 +518,6 @@ func (st *ServerType) serversFromPairings(
 		var hasCatchAllTLSConnPolicy, addressQualifiesForTLS bool
 		autoHTTPSWillAddConnPolicy := autoHTTPS != "off"
 
-		// if a catch-all server block (one which accepts all hostnames) exists in this pairing,
-		// we need to know that so that we can configure logs properly (see #3878)
-		var catchAllSblockExists bool
-		for _, sblock := range p.serverBlocks {
-			if len(sblock.hostsFromKeys(false)) == 0 {
-				catchAllSblockExists = true
-			}
-		}
-
 		// if needed, the ServerLogConfig is initialized beforehand so
 		// that all server blocks can populate it with data, even when not
 		// coming with a log directive
@@ -658,18 +649,10 @@ func (st *ServerType) serversFromPairings(
 				} else {
 					// map each host to the user's desired logger name
 					for _, h := range sblockLogHosts {
-						// if the custom logger name is non-empty, add it to the map;
-						// otherwise, only map to an empty logger name if this or
-						// another site block on this server has a catch-all host (in
-						// which case only requests with mapped hostnames will be
-						// access-logged, so it'll be necessary to add them to the
-						// map even if they use default logger)
-						if ncl.name != "" || catchAllSblockExists {
-							if srv.Logs.LoggerNames == nil {
-								srv.Logs.LoggerNames = make(map[string]string)
-							}
-							srv.Logs.LoggerNames[h] = ncl.name
+						if srv.Logs.LoggerNames == nil {
+							srv.Logs.LoggerNames = make(map[string]string)
 						}
+						srv.Logs.LoggerNames[h] = ncl.name
 					}
 				}
 			}

caddytest/integration/caddyfile_adapt/log_skip_hosts.txt

@@ -62,6 +62,9 @@ example.com {
 						}
 					],
 					"logs": {
+						"logger_names": {
+							"one.example.com": ""
+						},
 						"skip_hosts": [
 							"three.example.com",
 							"two.example.com",

logging.go

@@ -105,7 +105,7 @@ func (logging *Logging) openLogs(ctx Context) error {
 	// then set up any other custom logs
 	for name, l := range logging.Logs {
 		// the default log is already set up
-		if name == "default" {
+		if name == DefaultLoggerName {
 			continue
 		}
 
@@ -138,7 +138,7 @@ func (logging *Logging) setupNewDefault(ctx Context) error {
 
 	// extract the user-defined default log, if any
 	newDefault := new(defaultCustomLog)
-	if userDefault, ok := logging.Logs["default"]; ok {
+	if userDefault, ok := logging.Logs[DefaultLoggerName]; ok {
 		newDefault.CustomLog = userDefault
 	} else {
 		// if none, make one with our own default settings
@@ -147,7 +147,7 @@ func (logging *Logging) setupNewDefault(ctx Context) error {
 		if err != nil {
 			return fmt.Errorf("setting up default Caddy log: %v", err)
 		}
-		logging.Logs["default"] = newDefault.CustomLog
+		logging.Logs[DefaultLoggerName] = newDefault.CustomLog
 	}
 
 	// set up this new log
@@ -702,6 +702,8 @@ var (
 
 var writers = NewUsagePool()
 
+const DefaultLoggerName = "default"
+
 // Interface guards
 var (
 	_ io.WriteCloser = (*notClosable)(nil)

modules/caddyhttp/server.go

@@ -639,21 +639,18 @@ func (s *Server) shouldLogRequest(r *http.Request) bool {
 		// logging is disabled
 		return false
 	}
+	if _, ok := s.Logs.LoggerNames[r.Host]; ok {
+		// this host is mapped to a particular logger name
+		return true
+	}
 	for _, dh := range s.Logs.SkipHosts {
 		// logging for this particular host is disabled
 		if certmagic.MatchWildcard(r.Host, dh) {
 			return false
 		}
 	}
-	if _, ok := s.Logs.LoggerNames[r.Host]; ok {
-		// this host is mapped to a particular logger name
-		return true
-	}
-	if s.Logs.SkipUnmappedHosts {
-		// this host is not mapped and thus must not be logged
-		return false
-	}
-	return true
+	// if configured, this host is not mapped and thus must not be logged
+	return !s.Logs.SkipUnmappedHosts
 }
 
 // protocol returns true if the protocol proto is configured/enabled.