Skip to content

Commit

Permalink
caddyhttp: Split up remote address into IP and port
Browse files Browse the repository at this point in the history
Closes #4396, related to #4148

This is a breaking change, but we're already planning on removing  the`common_log` field as well at the same time, so might as well all do it together.

In general, the remote IP is the useful part of the remote address. The port is rarely useful, because it only identifies ephemeral information about the connection from the client. But we were logging both in one field, so certain tooling that would want to only get the remote IP would need to split it up. Splitting is non-trivial, because of IPv4, IPv6, shenanigans. So it's best if we split it up-front before logging. If the log consumer actually cares about the remote port, it can re-assemble it.
  • Loading branch information
francislavoie committed Nov 29, 2021
1 parent 0eb0b60 commit 335b49f
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 8 deletions.
8 changes: 7 additions & 1 deletion admin.go
Expand Up @@ -659,11 +659,17 @@ type adminHandler struct {
// ServeHTTP is the external entry point for API requests.
// It will only be called once per request.
func (h adminHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
ip, port, err := net.SplitHostPort(r.RemoteAddr)
if err != nil {
ip = r.RemoteAddr
port = ""
}
log := Log().Named("admin.api").With(
zap.String("method", r.Method),
zap.String("host", r.Host),
zap.String("uri", r.RequestURI),
zap.String("remote_addr", r.RemoteAddr),
zap.String("remote_ip", ip),
zap.String("remote_port", port),
zap.Reflect("headers", r.Header),
)
if r.TLS != nil {
Expand Down
4 changes: 2 additions & 2 deletions caddyconfig/httpcaddyfile/builtins_test.go
Expand Up @@ -37,7 +37,7 @@ func TestLogDirectiveSyntax(t *testing.T) {
format filter {
wrap console
fields {
request>remote_addr ip_mask {
request>remote_ip ip_mask {
ipv4 24
ipv6 32
}
Expand All @@ -46,7 +46,7 @@ func TestLogDirectiveSyntax(t *testing.T) {
}
}
`,
output: `{"logging":{"logs":{"default":{"exclude":["http.log.access.log0"]},"log0":{"encoder":{"fields":{"request\u003eremote_addr":{"filter":"ip_mask","ipv4_cidr":24,"ipv6_cidr":32}},"format":"filter","wrap":{"format":"console"}},"include":["http.log.access.log0"]}}},"apps":{"http":{"servers":{"srv0":{"listen":[":8080"],"logs":{"default_logger_name":"log0"}}}}}}`,
output: `{"logging":{"logs":{"default":{"exclude":["http.log.access.log0"]},"log0":{"encoder":{"fields":{"request\u003eremote_ip":{"filter":"ip_mask","ipv4_cidr":24,"ipv6_cidr":32}},"format":"filter","wrap":{"format":"console"}},"include":["http.log.access.log0"]}}},"apps":{"http":{"servers":{"srv0":{"listen":[":8080"],"logs":{"default_logger_name":"log0"}}}}}}`,
expectError: false,
},
{
Expand Down
Expand Up @@ -3,7 +3,7 @@
format filter {
wrap console
fields {
request>remote_addr ip_mask {
request>remote_ip ip_mask {
ipv4 24
ipv6 32
}
Expand All @@ -18,7 +18,7 @@
"custom-logger": {
"encoder": {
"fields": {
"request\u003eremote_addr": {
"request\u003eremote_ip": {
"filter": "ip_mask",
"ipv4_cidr": 24,
"ipv6_cidr": 32
Expand Down
4 changes: 2 additions & 2 deletions caddytest/integration/caddyfile_adapt/log_filters.txt
Expand Up @@ -15,7 +15,7 @@ log {
replace foo REDACTED
delete bar
}
request>remote_addr ip_mask {
request>remote_ip ip_mask {
ipv4 24
ipv6 32
}
Expand Down Expand Up @@ -64,7 +64,7 @@ log {
"request\u003eheaders\u003eServer": {
"filter": "delete"
},
"request\u003eremote_addr": {
"request\u003eremote_ip": {
"filter": "ip_mask",
"ipv4_cidr": 24,
"ipv6_cidr": 32
Expand Down
10 changes: 9 additions & 1 deletion modules/caddyhttp/marshalers.go
Expand Up @@ -16,6 +16,7 @@ package caddyhttp

import (
"crypto/tls"
"net"
"net/http"
"strings"

Expand All @@ -27,7 +28,14 @@ type LoggableHTTPRequest struct{ *http.Request }

// MarshalLogObject satisfies the zapcore.ObjectMarshaler interface.
func (r LoggableHTTPRequest) MarshalLogObject(enc zapcore.ObjectEncoder) error {
enc.AddString("remote_addr", r.RemoteAddr)
ip, port, err := net.SplitHostPort(r.RemoteAddr)
if err != nil {
ip = r.RemoteAddr
port = ""
}

enc.AddString("remote_ip", ip)
enc.AddString("remote_port", port)
enc.AddString("proto", r.Proto)
enc.AddString("method", r.Method)
enc.AddString("host", r.Host)
Expand Down

0 comments on commit 335b49f

Please sign in to comment.