fix: escape all ASCII control chars in CLF/Combined access log fields
- commit
- 71218113a3b507b8703f27a9e6fddab59081f6a5
- author
- brice · · 2026-04-03 15:52 UTC
- parent
- 01b6bc5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +27 -8
M internal/adapters/out/accesslog/formatter.go +25 -8
| @@ -110,13 +110,30 @@ func buildRequestTarget(path, query string) string { | |||
| 110 | 110 | return path + "?" + query | |
| 111 | 111 | } | |
| 112 | 112 | ||
| 113 | - | // escapeQuoted escapes backslashes, double-quotes, and newline characters inside | |
| 114 | - | // a value that will be enclosed in double-quotes in the log line. Newline | |
| 115 | - | // escaping prevents log-injection via user-controlled fields such as User-Agent. | |
| 113 | + | // escapeQuoted escapes backslashes, double-quotes, and control characters inside | |
| 114 | + | // a value that will be enclosed in double-quotes in the log line. | |
| 115 | + | // All ASCII control characters (0x00-0x1F) are escaped to prevent log-injection | |
| 116 | + | // via user-controlled fields such as User-Agent or Referer. | |
| 116 | 117 | func escapeQuoted(s string) string { | |
| 117 | - | s = strings.ReplaceAll(s, `\`, `\\`) | |
| 118 | - | s = strings.ReplaceAll(s, `"`, `\"`) | |
| 119 | - | s = strings.ReplaceAll(s, "\n", `\n`) | |
| 120 | - | s = strings.ReplaceAll(s, "\r", `\r`) | |
| 121 | - | return s | |
| 118 | + | var b strings.Builder | |
| 119 | + | b.Grow(len(s)) | |
| 120 | + | for _, r := range s { | |
| 121 | + | switch { | |
| 122 | + | case r == '\\': | |
| 123 | + | b.WriteString(`\\`) | |
| 124 | + | case r == '"': | |
| 125 | + | b.WriteString(`\"`) | |
| 126 | + | case r == '\n': | |
| 127 | + | b.WriteString(`\n`) | |
| 128 | + | case r == '\r': | |
| 129 | + | b.WriteString(`\r`) | |
| 130 | + | case r == '\t': | |
| 131 | + | b.WriteString(`\t`) | |
| 132 | + | case r < 0x20: | |
| 133 | + | fmt.Fprintf(&b, `\x%02x`, r) | |
| 134 | + | default: | |
| 135 | + | b.WriteRune(r) | |
| 136 | + | } | |
| 137 | + | } | |
| 138 | + | return b.String() | |
| 122 | 139 | } | |
| @@ -110,13 +110,30 @@ func buildRequestTarget(path, query string) string { | |||||
| 110 | return path + "?" + query | 110 | return path + "?" + query | ||
| 111 | } | 111 | } | ||
| 112 | 112 | ||||
| 113 | - | // escapeQuoted escapes backslashes, double-quotes, and newline characters inside | 113 | + | // escapeQuoted escapes backslashes, double-quotes, and control characters inside |
| 114 | - | // a value that will be enclosed in double-quotes in the log line. Newline | 114 | + | // a value that will be enclosed in double-quotes in the log line. |
| 115 | - | // escaping prevents log-injection via user-controlled fields such as User-Agent. | 115 | + | // All ASCII control characters (0x00-0x1F) are escaped to prevent log-injection |
| 116 | + | // via user-controlled fields such as User-Agent or Referer. | |||
| 116 | func escapeQuoted(s string) string { | 117 | func escapeQuoted(s string) string { | ||
| 117 | - | s = strings.ReplaceAll(s, `\`, `\\`) | 118 | + | var b strings.Builder |
| 118 | - | s = strings.ReplaceAll(s, `"`, `\"`) | 119 | + | b.Grow(len(s)) |
| 119 | - | s = strings.ReplaceAll(s, "\n", `\n`) | 120 | + | for _, r := range s { |
| 120 | - | s = strings.ReplaceAll(s, "\r", `\r`) | 121 | + | switch { |
| 121 | - | return s | 122 | + | case r == '\\': |
| 123 | + | b.WriteString(`\\`) | |||
| 124 | + | case r == '"': | |||
| 125 | + | b.WriteString(`\"`) | |||
| 126 | + | case r == '\n': | |||
| 127 | + | b.WriteString(`\n`) | |||
| 128 | + | case r == '\r': | |||
| 129 | + | b.WriteString(`\r`) | |||
| 130 | + | case r == '\t': | |||
| 131 | + | b.WriteString(`\t`) | |||
| 132 | + | case r < 0x20: | |||
| 133 | + | fmt.Fprintf(&b, `\x%02x`, r) | |||
| 134 | + | default: | |||
| 135 | + | b.WriteRune(r) | |||
| 136 | + | } | |||
| 137 | + | } | |||
| 138 | + | return b.String() | |||
| 122 | } | 139 | } | ||
M internal/adapters/out/accesslog/writer.go +2 -0
| @@ -160,6 +160,8 @@ func newFileSink(cfg Config) (*fileSink, error) { | |||
| 160 | 160 | return nil, fmt.Errorf("accesslog: create log directory: %w", err) | |
| 161 | 161 | } | |
| 162 | 162 | ||
| 163 | + | // lumberjack defaults when zero: MaxSize=100MB, MaxBackups=keep all, MaxAge=no limit. | |
| 164 | + | // Config defaults in run.go set these to sensible values; zero here is valid. | |
| 163 | 165 | return &fileSink{ | |
| 164 | 166 | logger: &lumberjack.Logger{ | |
| 165 | 167 | Filename: cfg.FilePath, | |
| @@ -160,6 +160,8 @@ func newFileSink(cfg Config) (*fileSink, error) { | |||||
| 160 | return nil, fmt.Errorf("accesslog: create log directory: %w", err) | 160 | return nil, fmt.Errorf("accesslog: create log directory: %w", err) | ||
| 161 | } | 161 | } | ||
| 162 | 162 | ||||
| 163 | + | // lumberjack defaults when zero: MaxSize=100MB, MaxBackups=keep all, MaxAge=no limit. | |||
| 164 | + | // Config defaults in run.go set these to sensible values; zero here is valid. | |||
| 163 | return &fileSink{ | 165 | return &fileSink{ | ||
| 164 | logger: &lumberjack.Logger{ | 166 | logger: &lumberjack.Logger{ | ||
| 165 | Filename: cfg.FilePath, | 167 | Filename: cfg.FilePath, | ||