Can't Message Users Here, Reaching Out Via Thread

francislavoie · January 14, 2021, 10:34pm

Okay, I see what’s going wrong now.

caddyserver/caddy/blob/master/modules/caddyhttp/server.go#L219

    
      
          				zap.Object("resp_headers", LoggableHTTPHeader{
          					Header:               wrec.Header(),
          					ShouldLogCredentials: shouldLogCredentials,
          				}),
          			)
          		}()
          	}
          
          
	start := time.Now()
          
          
	// guarantee ACME HTTP challenges; handle them
          	// separately from any user-defined handlers
          	if s.tlsApp.HandleHTTPChallenge(w, r) {
          		duration = time.Since(start)
          		return
          	}
          
          
	// execute the primary handler chain
          	err := s.primaryHandlerChain.ServeHTTP(w, r)
          	duration = time.Since(start)

So you see that above this section, if there’s no error, Caddy returns. But if there’s an error – which is the case with basicauth because it returns an actual caddyhttp.Error(http.StatusUnauthorized, ...) – it resets the RemoteAddr on the request before it gets logged.

This functionality was implemented in this PR:

https://github.com/caddyserver/caddy/pull/3781

@matt I think we may want to rethink how this is handled, because there is a legitimate usecase for wanting the RemoteAddr to be permanently modified here.

Or… now that I think of it, maybe the realip plugin should also mutate origReq := r.Context().Value(OriginalRequestCtxKey).(http.Request) so that Caddy doesn’t need to change? But that does feel like too funky of a fix (would introduce a weird hidden behaviour).

Edit: Oh aaaaaactually, what if you add realip to handle_errors?

handle_errors {
	realip
}