From 726a9a8fde60e4d72282b24fb553573fb388607c Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 22 Apr 2024 08:33:07 -0400 Subject: [PATCH] logging: Fix default access logger (#6251) * logging: Fix default access logger * Simplify logic, remove retry without port, reject config with port, docs * Nil check --- modules/caddyhttp/app.go | 12 +++++++++++- modules/caddyhttp/logging.go | 38 ++++++++++++++++++------------------ modules/caddyhttp/server.go | 11 +++++++++-- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 2d2c12cb..4df8689a 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -329,9 +329,10 @@ func (app *App) Provision(ctx caddy.Context) error { // Validate ensures the app's configuration is valid. func (app *App) Validate() error { - // each server must use distinct listener addresses lnAddrs := make(map[string]string) + for srvName, srv := range app.Servers { + // each server must use distinct listener addresses for _, addr := range srv.Listen { listenAddr, err := caddy.ParseNetworkAddress(addr) if err != nil { @@ -347,6 +348,15 @@ func (app *App) Validate() error { lnAddrs[addr] = srvName } } + + // logger names must not have ports + if srv.Logs != nil { + for host := range srv.Logs.LoggerNames { + if _, _, err := net.SplitHostPort(host); err == nil { + return fmt.Errorf("server %s: logger name must not have a port: %s", srvName, host) + } + } + } } return nil } diff --git a/modules/caddyhttp/logging.go b/modules/caddyhttp/logging.go index 313f12e1..774bd22f 100644 --- a/modules/caddyhttp/logging.go +++ b/modules/caddyhttp/logging.go @@ -37,10 +37,16 @@ type ServerLogConfig struct { DefaultLoggerName string `json:"default_logger_name,omitempty"` // LoggerNames maps request hostnames to one or more custom logger - // names. For example, a mapping of "example.com" to "example" would + // names. For example, a mapping of `"example.com": ["example"]` would // cause access logs from requests with a Host of example.com to be // emitted by a logger named "http.log.access.example". If there are // multiple logger names, then the log will be emitted to all of them. + // If the logger name is an empty, the default logger is used, i.e. + // the logger "http.log.access". + // + // Keys must be hostnames (without ports), and may contain wildcards + // to match subdomains. The value is an array of logger names. + // // For backwards compatibility, if the value is a string, it is treated // as a single-element array. LoggerNames map[string]StringArray `json:"logger_names,omitempty"` @@ -64,10 +70,19 @@ type ServerLogConfig struct { // wrapLogger wraps logger in one or more logger named // according to user preferences for the given host. func (slc ServerLogConfig) wrapLogger(logger *zap.Logger, host string) []*zap.Logger { - hosts := slc.getLoggerHosts(host) + // logger config should always be only + // the hostname, without the port + hostWithoutPort, _, err := net.SplitHostPort(host) + if err != nil { + hostWithoutPort = host + } + + hosts := slc.getLoggerHosts(hostWithoutPort) loggers := make([]*zap.Logger, 0, len(hosts)) for _, loggerName := range hosts { + // no name, use the default logger if loggerName == "" { + loggers = append(loggers, logger) continue } loggers = append(loggers, logger.Named(loggerName)) @@ -76,23 +91,8 @@ func (slc ServerLogConfig) wrapLogger(logger *zap.Logger, host string) []*zap.Lo } func (slc ServerLogConfig) getLoggerHosts(host string) []string { - tryHost := func(key string) ([]string, bool) { - // first try exact match - if hosts, ok := slc.LoggerNames[key]; ok { - return hosts, ok - } - // strip port and try again (i.e. Host header of "example.com:1234" should - // match "example.com" if there is no "example.com:1234" in the map) - hostOnly, _, err := net.SplitHostPort(key) - if err != nil { - return []string{}, false - } - hosts, ok := slc.LoggerNames[hostOnly] - return hosts, ok - } - // try the exact hostname first - if hosts, ok := tryHost(host); ok { + if hosts, ok := slc.LoggerNames[host]; ok { return hosts } @@ -104,7 +104,7 @@ func (slc ServerLogConfig) getLoggerHosts(host string) []string { } labels[i] = "*" wildcardHost := strings.Join(labels, ".") - if hosts, ok := tryHost(wildcardHost); ok { + if hosts, ok := slc.LoggerNames[wildcardHost]; ok { return hosts } } diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 2418a590..0c616192 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -717,13 +717,20 @@ func (s *Server) shouldLogRequest(r *http.Request) bool { // logging is disabled return false } - if _, ok := s.Logs.LoggerNames[r.Host]; ok { + + // strip off the port if any, logger names are host only + hostWithoutPort, _, err := net.SplitHostPort(r.Host) + if err != nil { + hostWithoutPort = r.Host + } + + if _, ok := s.Logs.LoggerNames[hostWithoutPort]; 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) { + if certmagic.MatchWildcard(hostWithoutPort, dh) { return false } }