Skip to content

Commit 134caf8

Browse files
IDisposableIDisposable
authored
Added sanitization of journald keys (#751)
Also, raised text coverage for the journald sub-package to 92.2% Fixes #668
1 parent e133b6a commit 134caf8

File tree

2 files changed

+234
-9
lines changed

2 files changed

+234
-9
lines changed

‎journald/journald.go‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ package journald
1111

1212
// Zerolog's Top level key/Value Pairs are translated to
1313
// journald's args - all Values are sent to journald as strings.
14-
// And all key strings are converted to uppercase before sending
15-
// to journald (as required by journald).
14+
// And all key strings are converted to uppercase and sanitized
15+
// by replacing any characters not in [A-Z0-9_] with '_' before
16+
// sending to journald (as required by journald).
1617

1718
// In addition, entire log message (all Key Value Pairs), is also
1819
// sent to journald under the key "JSON".
@@ -31,6 +32,12 @@ import (
3132

3233
const defaultJournalDPrio = journal.PriNotice
3334

35+
// SendFunc is the function used to send logs to journald.
36+
// It can be replaced in tests for mocking. If nil, journal.Send is used directly.
37+
// This variable should only be modified in tests and must not be changed while the
38+
// writer is in use. Tests that modify this variable should not use t.Parallel().
39+
var SendFunc func(string, journal.Priority, map[string]string) error
40+
3441
// NewJournalDWriter returns a zerolog log destination
3542
// to be used as parameter to New() calls. Writing logs
3643
// to this writer will send the log messages to journalD
@@ -69,6 +76,24 @@ func levelToJPrio(zLevel string) journal.Priority {
6976
return defaultJournalDPrio
7077
}
7178

79+
// sanitizeKey converts a key to uppercase and replaces invalid characters with '_'
80+
// JournalD requires keys start with A-Z and contain only A-Z, 0-9, or _
81+
func sanitizeKey(key string) string {
82+
sanitized := strings.Map(func(r rune) rune {
83+
if r >= 'a' && r <= 'z' {
84+
return r - 'a' + 'A'
85+
} else if (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' {
86+
return r
87+
} else {
88+
return '_'
89+
}
90+
}, key)
91+
if len(sanitized) == 0 || sanitized[0] >= '0' && sanitized[0] <= '9' || sanitized[0] == '_' {
92+
sanitized = "X" + sanitized
93+
}
94+
return sanitized
95+
}
96+
7297
func (w journalWriter) Write(p []byte) (n int, err error) {
7398
var event map[string]interface{}
7499
origPLen := len(p)
@@ -87,7 +112,7 @@ func (w journalWriter) Write(p []byte) (n int, err error) {
87112

88113
msg := ""
89114
for key, value := range event {
90-
jKey := strings.ToUpper(key)
115+
jKey := sanitizeKey(key)
91116
switch key {
92117
case zerolog.LevelFieldName, zerolog.TimestampFieldName:
93118
continue
@@ -111,7 +136,11 @@ func (w journalWriter) Write(p []byte) (n int, err error) {
111136
}
112137
}
113138
args["JSON"] = string(p)
114-
err = journal.Send(msg, jPrio, args)
139+
if SendFunc != nil {
140+
err = SendFunc(msg, jPrio, args)
141+
} else {
142+
err = journal.Send(msg, jPrio, args)
143+
}
115144

116145
if err == nil {
117146
n = origPLen

‎journald/journald_test.go‎

Lines changed: 201 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
1+
//go:build linux
12
// +build linux
23

3-
package journald_test
4+
package journald
45

56
import (
67
"bytes"
8+
"fmt"
79
"io"
10+
"strings"
811
"testing"
912

13+
"github.com/coreos/go-systemd/v22/journal"
1014
"github.com/rs/zerolog"
11-
"github.com/rs/zerolog/journald"
1215
)
1316

1417
func ExampleNewJournalDWriter() {
15-
log := zerolog.New(journald.NewJournalDWriter())
18+
log := zerolog.New(NewJournalDWriter())
1619
log.Info().Str("foo", "bar").Uint64("small", 123).Float64("float", 3.14).Uint64("big", 1152921504606846976).Msg("Journal Test")
1720
// Output:
1821
}
@@ -49,9 +52,37 @@ Thu 2018-04-26 22:30:20.768136 PDT [s=3284d695bde946e4b5017c77a399237f;i=329f0;b
4952
_SOURCE_REALTIME_TIMESTAMP=1524807020768136
5053
*/
5154

55+
func TestSanitizeKey(t *testing.T) {
56+
tests := []struct {
57+
input string
58+
expected string
59+
}{
60+
{"test", "TEST"},
61+
{"Test", "TEST"},
62+
{"test-key", "TEST_KEY"},
63+
{"Test.Key", "TEST_KEY"},
64+
{"test_key123", "TEST_KEY123"},
65+
{"invalid@key!", "INVALID_KEY_"},
66+
{"a1B2_c3D4", "A1B2_C3D4"},
67+
{"_", "X_"},
68+
{"", "X"},
69+
{"123", "X123"},
70+
{"a-b.c_d", "A_B_C_D"},
71+
}
72+
73+
for _, tt := range tests {
74+
t.Run(tt.input, func(t *testing.T) {
75+
result := sanitizeKey(tt.input)
76+
if result != tt.expected {
77+
t.Errorf("sanitizeKey(%q) = %q; want %q", tt.input, result, tt.expected)
78+
}
79+
})
80+
}
81+
}
82+
5283
func TestWriteReturnsNoOfWrittenBytes(t *testing.T) {
5384
input := []byte(`{"level":"info","time":1570912626,"message":"Starting..."}`)
54-
wr := journald.NewJournalDWriter()
85+
wr := NewJournalDWriter()
5586
want := len(input)
5687
got, err := wr.Write(input)
5788

@@ -68,7 +99,7 @@ func TestMultiWrite(t *testing.T) {
6899
var (
69100
w1 = new(bytes.Buffer)
70101
w2 = new(bytes.Buffer)
71-
w3 = journald.NewJournalDWriter()
102+
w3 = NewJournalDWriter()
72103
)
73104

74105
zerolog.ErrorHandler = func(err error) {
@@ -84,3 +115,168 @@ func TestMultiWrite(t *testing.T) {
84115
log.Info().Msg("Tick!")
85116
}
86117
}
118+
119+
func TestWriteWithVariousTypes(t *testing.T) {
120+
mock := &mockSend{}
121+
oldSend := SendFunc
122+
SendFunc = mock.send
123+
defer func() { SendFunc = oldSend }()
124+
125+
wr := NewJournalDWriter()
126+
log := zerolog.New(wr)
127+
128+
// This should cover the default case in the switch for value types
129+
log.Info().Bool("flag", true).Str("foo", "bar").Uint64("small", 123).Float64("float", 3.14).Uint64("big", 1152921504606846976).Interface("data", map[string]int{"a": 1}).Msg("Test various types")
130+
131+
// Verify the call
132+
if len(mock.calls) != 1 {
133+
t.Fatalf("Expected 1 call, got %d", len(mock.calls))
134+
}
135+
136+
call := mock.calls[0]
137+
138+
// Check that flag is sanitized to FLAG and value is "true"
139+
if call.args["FLAG"] != "true" {
140+
t.Errorf("Expected FLAG=true, got %s", call.args["FLAG"])
141+
}
142+
143+
// Check that data is marshaled (should be a JSON string)
144+
expectedData := `{"a":1}`
145+
if call.args["DATA"] != expectedData {
146+
t.Errorf("Expected DATA=%q, got %q", expectedData, call.args["DATA"])
147+
}
148+
}
149+
150+
func TestWriteWithAllLevels(t *testing.T) {
151+
wr := NewJournalDWriter()
152+
153+
// Save original FatalExitFunc
154+
oldFatalExitFunc := zerolog.FatalExitFunc
155+
defer func() { zerolog.FatalExitFunc = oldFatalExitFunc }()
156+
157+
// Set FatalExitFunc to prevent actual exit
158+
zerolog.FatalExitFunc = func() {}
159+
160+
log := zerolog.New(wr)
161+
162+
// Test all zerolog levels to cover levelToJPrio switch cases
163+
log.Trace().Msg("Trace level")
164+
log.Debug().Msg("Debug level")
165+
log.Info().Msg("Info level")
166+
log.Warn().Msg("Warn level")
167+
log.Error().Msg("Error level")
168+
log.Log().Msg("No level")
169+
170+
// For Fatal, it will call FatalExitFunc instead of exiting
171+
log.Fatal().Msg("Fatal level")
172+
173+
// For Panic, use recover to catch the panic, do last because it will stop of this test execution
174+
defer func() {
175+
if r := recover(); r == nil {
176+
t.Error("Expected panic from Panic level")
177+
}
178+
}()
179+
log.Panic().Msg("Panic level")
180+
181+
}
182+
183+
func TestWriteOutputs(t *testing.T) {
184+
mock := &mockSend{}
185+
oldSend := SendFunc
186+
SendFunc = mock.send
187+
defer func() { SendFunc = oldSend }()
188+
189+
wr := NewJournalDWriter()
190+
log := zerolog.New(wr)
191+
192+
// Log a message with various fields
193+
log.Info().Str("test-key", "value").Int("number", 42).Msg("Test message")
194+
195+
// Check that SendFunc was called
196+
if len(mock.calls) != 1 {
197+
t.Fatalf("Expected 1 call to SendFunc, got %d", len(mock.calls))
198+
}
199+
200+
call := mock.calls[0]
201+
202+
// Check message
203+
if call.msg != "Test message" {
204+
t.Errorf("Expected msg 'Test message', got %q", call.msg)
205+
}
206+
207+
// Check priority
208+
if call.prio != journal.PriInfo {
209+
t.Errorf("Expected prio %d (PriInfo), got %d", journal.PriInfo, call.prio)
210+
}
211+
212+
// Check args
213+
expectedArgs := map[string]string{
214+
"TEST_KEY": "value",
215+
"NUMBER": "42",
216+
"JSON": `{"level":"info","test-key":"value","number":42,"message":"Test message"}` + "\n",
217+
}
218+
219+
for k, v := range expectedArgs {
220+
if call.args[k] != v {
221+
t.Errorf("Expected args[%q] = %q, got %q", k, v, call.args[k])
222+
}
223+
}
224+
225+
// Check that LEVEL is not in args (since it's skipped)
226+
if _, ok := call.args["LEVEL"]; ok {
227+
t.Error("LEVEL should not be in args")
228+
}
229+
}
230+
231+
func TestWriteWithMarshalError(t *testing.T) {
232+
mock := &mockSend{}
233+
oldSend := SendFunc
234+
SendFunc = mock.send
235+
defer func() { SendFunc = oldSend }()
236+
237+
// Save original marshal func
238+
originalMarshal := zerolog.InterfaceMarshalFunc
239+
defer func() { zerolog.InterfaceMarshalFunc = originalMarshal }()
240+
241+
// Set marshal func to fail
242+
zerolog.InterfaceMarshalFunc = func(v interface{}) ([]byte, error) {
243+
return nil, fmt.Errorf("fake error")
244+
}
245+
246+
wr := NewJournalDWriter()
247+
log := zerolog.New(wr)
248+
249+
// This should trigger the error handling in the default case
250+
log.Info().Interface("data", map[string]int{"a": 1}).Msg("Test with error")
251+
252+
// Verify the call
253+
if len(mock.calls) != 1 {
254+
t.Fatalf("Expected 1 call, got %d", len(mock.calls))
255+
}
256+
257+
call := mock.calls[0]
258+
259+
// Check that data has the error message
260+
got := call.args["DATA"]
261+
want := "error: fake error"
262+
if !strings.Contains(got, want) {
263+
t.Errorf("Expected DATA to contain %q, got %q", want, got)
264+
}
265+
}
266+
267+
type mockSend struct {
268+
calls []struct {
269+
msg string
270+
prio journal.Priority
271+
args map[string]string
272+
}
273+
}
274+
275+
func (m *mockSend) send(msg string, prio journal.Priority, args map[string]string) error {
276+
m.calls = append(m.calls, struct {
277+
msg string
278+
prio journal.Priority
279+
args map[string]string
280+
}{msg, prio, args})
281+
return nil
282+
}

0 commit comments

Comments
 (0)