From 99ffe9338837eff80def7e5d51c2a0e02f56abdb Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 5 Oct 2022 14:14:13 -0400 Subject: [PATCH] logging: Fix `skip_hosts` with wildcards (#5102) Fix #4859 --- caddyconfig/httpcaddyfile/builtins.go | 2 +- caddyconfig/httpcaddyfile/httptype.go | 33 +++++-------------- .../caddyfile_adapt/log_skip_hosts.txt | 3 ++ logging.go | 8 +++-- modules/caddyhttp/server.go | 15 ++++----- 5 files changed, 23 insertions(+), 38 deletions(-) diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 7f23fd59d..f4c9ce01b 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/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. diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index 7159454b0..c220c066d 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/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 } } } diff --git a/caddytest/integration/caddyfile_adapt/log_skip_hosts.txt b/caddytest/integration/caddyfile_adapt/log_skip_hosts.txt index 6fbd85a87..25e4bd1c2 100644 --- a/caddytest/integration/caddyfile_adapt/log_skip_hosts.txt +++ b/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", diff --git a/logging.go b/logging.go index 5f8ea06a9..c00c8315d 100644 --- a/logging.go +++ b/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) diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 4d47d266e..60fc3c387 100644 --- a/modules/caddyhttp/server.go +++ b/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.