From 8e5aafa5cdb0bd6ad062014172ed21fdc1012cc1 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 2 Dec 2021 15:23:19 -0500 Subject: [PATCH] fastcgi: Fix a TODO, prevent zap using reflection for logging env (#4437) * fastcgi: Fix a TODO, prevent zap using reflection for logging env * Update modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go Co-authored-by: Mohammed Al Sahaf Co-authored-by: Mohammed Al Sahaf --- .../caddyhttp/reverseproxy/fastcgi/fastcgi.go | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index 05b776da051..5404424dd10 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -29,6 +29,7 @@ import ( "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy" "github.com/caddyserver/caddy/v2/modules/caddytls" "go.uber.org/zap" + "go.uber.org/zap/zapcore" "github.com/caddyserver/caddy/v2" ) @@ -132,7 +133,7 @@ func (t Transport) RoundTrip(r *http.Request) (*http.Response, error) { t.logger.Debug("roundtrip", zap.Object("request", caddyhttp.LoggableHTTPRequest{Request: r}), zap.String("dial", address), - zap.Any("env", env), // TODO: this uses reflection I think + zap.Object("env", env), ) fcgiBackend, err := DialContext(ctx, network, address) @@ -171,10 +172,10 @@ func (t Transport) RoundTrip(r *http.Request) (*http.Response, error) { } // buildEnv returns a set of CGI environment variables for the request. -func (t Transport) buildEnv(r *http.Request) (map[string]string, error) { +func (t Transport) buildEnv(r *http.Request) (envVars, error) { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - var env map[string]string + var env envVars // Separate remote IP and port; more lenient than net.SplitHostPort var ip, port string @@ -259,7 +260,7 @@ func (t Transport) buildEnv(r *http.Request) (map[string]string, error) { // Some variables are unused but cleared explicitly to prevent // the parent environment from interfering. - env = map[string]string{ + env = envVars{ // Variables defined in CGI 1.1 spec "AUTH_TYPE": "", // Not used "CONTENT_LENGTH": r.Header.Get("Content-Length"), @@ -353,6 +354,16 @@ func (t Transport) splitPos(path string) int { return -1 } +// envVars is a simple type to allow for speeding up zap log encoding. +type envVars map[string]string + +func (env envVars) MarshalLogObject(enc zapcore.ObjectEncoder) error { + for k, v := range env { + enc.AddString(k, v) + } + return nil +} + // Map of supported protocols to Apache ssl_mod format // Note that these are slightly different from SupportedProtocols in caddytls/config.go var tlsProtocolStrings = map[uint16]string{ @@ -366,6 +377,8 @@ var headerNameReplacer = strings.NewReplacer(" ", "_", "-", "_") // Interface guards var ( + _ zapcore.ObjectMarshaler = (*envVars)(nil) + _ caddy.Provisioner = (*Transport)(nil) _ http.RoundTripper = (*Transport)(nil) )