Skip to content

Commit 2ea643a

Browse files
bugerclaudepvormste
authored
Merging to release-1.12: [TT-15532] Alternative backward-compatible fix for syslog pump log fragmentation (#886) (#891)
* [TT-15532] Alternative backward-compatible fix for syslog pump log fragmentation (#886) * Fix syslog pump log fragmentation issue with comprehensive unit tests - Added JSON serialization to prevent multiline HTTP data from fragmenting syslog entries - Each analytics record is now serialized to JSON before sending to syslog - This ensures syslog entries remain single-line, preventing fragmentation across multiple log entries - Maintains backward compatibility as the old behavior was broken for multiline data Added comprehensive unit tests covering: - Basic syslog functionality with single and multiple records - Multiline HTTP request/response handling (the original fragmentation issue) - Special character escaping and Unicode handling - Large data payload handling - Field preservation verification - Context cancellation handling - Explicit fragmentation fix demonstration Tests use UDP mock servers to verify syslog output format and content preservation. All tests validate that multiline data is properly escaped in JSON and syslog entries remain single-line to prevent fragmentation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix syslog pump fragmentation while maintaining full backward compatibility - Revert go.mod/go.sum changes (testcontainers dependencies were unnecessary) - Maintain original map[key:value ...] output format for full backward compatibility - Escape newlines only in raw_request and raw_response fields to prevent fragmentation - Updated tests to verify map format and escaped newlines - Single-line syslog entries prevent log fragmentation while preserving exact original format 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix formatting and clean up syslog tests - Fix gofmt formatting issues in syslog_test.go - Remove problematic tests expecting JSON format - Keep focused tests for backward compatibility and fragmentation fix - All tests now expect and verify original map[...] format - Tests verify newline escaping and single-line syslog output 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix final formatting issues for CI - Fixed gofmt formatting in syslog.go and syslog_test.go - All comprehensive scenario tests pass locally - Ready for CI validation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Remove Claude settings from PR and add to gitignore - Remove .claude/settings.local.json from git tracking - Add .claude/settings.local.json to .gitignore - Keep file locally but exclude from repository 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit 0596e82) * fix conflict --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Patric Vormstein <pvormstein@googlemail.com>
1 parent a73aced commit 2ea643a

3 files changed

Lines changed: 325 additions & 3 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,4 @@ pumps/chunk.go
2020
migrate.js
2121
utils/release_rc.sh
2222
.terraform**
23+
.claude/settings.local.json

‎pumps/syslog.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"log/syslog"
7+
"strings"
78

89
"github.com/mitchellh/mapstructure"
910

@@ -145,6 +146,12 @@ func (s *SyslogPump) WriteData(ctx context.Context, data []interface{}) error {
145146
default:
146147
// Decode the raw analytics into Form
147148
decoded := v.(analytics.AnalyticsRecord)
149+
150+
// Escape newlines in raw_request and raw_response to prevent log fragmentation
151+
// while maintaining the original map format for backward compatibility
152+
escapedRawRequest := strings.ReplaceAll(decoded.RawRequest, "\n", "\\n")
153+
escapedRawResponse := strings.ReplaceAll(decoded.RawResponse, "\n", "\\n")
154+
148155
message := Json{
149156
"timestamp": decoded.TimeStamp,
150157
"method": decoded.Method,
@@ -158,16 +165,16 @@ func (s *SyslogPump) WriteData(ctx context.Context, data []interface{}) error {
158165
"api_id": decoded.APIID,
159166
"org_id": decoded.OrgID,
160167
"oauth_id": decoded.OauthID,
161-
"raw_request": decoded.RawRequest,
168+
"raw_request": escapedRawRequest,
162169
"request_time_ms": decoded.RequestTime,
163-
"raw_response": decoded.RawResponse,
170+
"raw_response": escapedRawResponse,
164171
"ip_address": decoded.IPAddress,
165172
"host": decoded.Host,
166173
"content_length": decoded.ContentLength,
167174
"user_agent": decoded.UserAgent,
168175
}
169176

170-
// Print to Syslog
177+
// Print to Syslog using original map format (maintains backward compatibility)
171178
_, _ = fmt.Fprintf(s.writer, "%s", message)
172179
}
173180
}

‎pumps/syslog_test.go‎

Lines changed: 314 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,314 @@
1+
package pumps
2+
3+
import (
4+
"context"
5+
"net"
6+
"strings"
7+
"testing"
8+
"time"
9+
10+
"github.com/TykTechnologies/tyk-pump/analytics"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// mockSyslogServer creates a simple UDP syslog server for testing
16+
func mockSyslogServer(t *testing.T) (string, chan string) {
17+
addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:0")
18+
require.NoError(t, err)
19+
20+
conn, err := net.ListenUDP("udp", addr)
21+
require.NoError(t, err)
22+
23+
messages := make(chan string, 100)
24+
25+
go func() {
26+
defer conn.Close()
27+
buffer := make([]byte, 1024)
28+
for {
29+
n, _, err := conn.ReadFromUDP(buffer)
30+
if err != nil {
31+
return
32+
}
33+
messages <- string(buffer[:n])
34+
}
35+
}()
36+
37+
return conn.LocalAddr().String(), messages
38+
}
39+
40+
// Helper function to create a SyslogPump with test configuration
41+
func createTestSyslogPump(addr string) *SyslogPump {
42+
pump := &SyslogPump{
43+
syslogConf: &SyslogConf{
44+
Transport: "udp",
45+
NetworkAddr: addr,
46+
LogLevel: 6, // Info level
47+
Tag: "test",
48+
},
49+
CommonPumpConfig: CommonPumpConfig{
50+
log: log.WithField("prefix", "test"),
51+
},
52+
}
53+
54+
// Initialize the writer
55+
pump.initWriter()
56+
return pump
57+
}
58+
59+
func TestSyslogPump_WriteData(t *testing.T) {
60+
tests := []struct {
61+
name string
62+
data []interface{}
63+
wantLogs int // expected number of log entries
64+
}{
65+
{
66+
name: "Single valid record",
67+
data: []interface{}{
68+
analytics.AnalyticsRecord{
69+
Method: "GET",
70+
Path: "/api/test",
71+
ResponseCode: 200,
72+
TimeStamp: time.Now(),
73+
},
74+
},
75+
wantLogs: 1,
76+
},
77+
{
78+
name: "Multiple valid records",
79+
data: []interface{}{
80+
analytics.AnalyticsRecord{
81+
Method: "GET",
82+
Path: "/api/test1",
83+
ResponseCode: 200,
84+
TimeStamp: time.Now(),
85+
},
86+
analytics.AnalyticsRecord{
87+
Method: "POST",
88+
Path: "/api/test2",
89+
ResponseCode: 201,
90+
TimeStamp: time.Now(),
91+
},
92+
},
93+
wantLogs: 2,
94+
},
95+
{
96+
name: "Empty data slice",
97+
data: []interface{}{},
98+
wantLogs: 0,
99+
},
100+
}
101+
102+
for _, tt := range tests {
103+
t.Run(tt.name, func(t *testing.T) {
104+
// Create mock syslog server
105+
addr, messages := mockSyslogServer(t)
106+
107+
// Create syslog pump with test configuration
108+
s := createTestSyslogPump(addr)
109+
110+
// Call WriteData
111+
err := s.WriteData(context.Background(), tt.data)
112+
assert.NoError(t, err)
113+
114+
if tt.wantLogs == 0 {
115+
// Give a small amount of time for any potential messages
116+
select {
117+
case <-messages:
118+
t.Error("Expected no messages but received one")
119+
case <-time.After(100 * time.Millisecond):
120+
// Good, no messages received
121+
}
122+
return
123+
}
124+
125+
// Collect messages
126+
var receivedMessages []string
127+
timeout := time.After(2 * time.Second)
128+
for len(receivedMessages) < tt.wantLogs {
129+
select {
130+
case msg := <-messages:
131+
receivedMessages = append(receivedMessages, msg)
132+
case <-timeout:
133+
break
134+
}
135+
}
136+
137+
assert.Equal(t, tt.wantLogs, len(receivedMessages), "Expected %d log entries, got %d", tt.wantLogs, len(receivedMessages))
138+
139+
// Verify each message contains the original map format
140+
for i, msg := range receivedMessages {
141+
// Syslog messages have a header, extract the map part
142+
// Look for the map starting with 'map['
143+
mapStart := strings.Index(msg, "map[")
144+
require.True(t, mapStart >= 0, "Message should contain map format: %s", msg)
145+
mapPart := strings.TrimSpace(msg[mapStart:])
146+
147+
// Verify it's the expected map format
148+
assert.True(t, strings.HasPrefix(mapPart, "map["), "Log entry %d should start with 'map[': %s", i, mapPart)
149+
assert.True(t, strings.HasSuffix(mapPart, "]"), "Log entry %d should end with ']': %s", i, mapPart)
150+
}
151+
})
152+
}
153+
}
154+
155+
func TestSyslogPump_WriteData_WithMultilineHTTP(t *testing.T) {
156+
// Test data with realistic multiline HTTP requests/responses that would cause fragmentation
157+
record := analytics.AnalyticsRecord{
158+
Method: "POST",
159+
Path: "/api/users",
160+
ResponseCode: 201,
161+
TimeStamp: time.Now(),
162+
// Real HTTP request with headers and body
163+
RawRequest: `POST /api/users HTTP/1.1
164+
Host: api.example.com
165+
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36
166+
Content-Type: application/json
167+
Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9
168+
Content-Length: 67
169+
170+
{
171+
"name": "John Doe",
172+
"email": "john@example.com",
173+
"age": 30
174+
}`,
175+
// Real HTTP response with headers and body
176+
RawResponse: `HTTP/1.1 201 Created
177+
Date: Wed, 15 Aug 2024 10:30:00 GMT
178+
Content-Type: application/json
179+
Server: nginx/1.18.0
180+
Content-Length: 156
181+
182+
{
183+
"id": 12345,
184+
"name": "John Doe",
185+
"email": "john@example.com",
186+
"age": 30,
187+
"created_at": "2024-08-15T10:30:00Z"
188+
}`,
189+
UserAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36",
190+
}
191+
192+
// Create mock syslog server
193+
addr, messages := mockSyslogServer(t)
194+
195+
// Create syslog pump with test configuration
196+
s := createTestSyslogPump(addr)
197+
198+
err := s.WriteData(context.Background(), []interface{}{record})
199+
assert.NoError(t, err)
200+
201+
// Wait for message
202+
select {
203+
case msg := <-messages:
204+
// Extract map from syslog message
205+
// Look for the map starting with 'map['
206+
mapStart := strings.Index(msg, "map[")
207+
require.True(t, mapStart >= 0, "Message should contain map format: %s", msg)
208+
mapPart := strings.TrimSpace(msg[mapStart:])
209+
210+
// Verify the syslog message itself is a single line (no fragmentation)
211+
lines := strings.Split(msg, "\n")
212+
nonEmptyLines := []string{}
213+
for _, line := range lines {
214+
if strings.TrimSpace(line) != "" {
215+
nonEmptyLines = append(nonEmptyLines, line)
216+
}
217+
}
218+
assert.Equal(t, 1, len(nonEmptyLines), "Syslog message should be a single line, got %d lines", len(nonEmptyLines))
219+
220+
// Verify it's the expected map format
221+
assert.True(t, strings.HasPrefix(mapPart, "map["), "Should be map format: %s", mapPart)
222+
// Note: May be truncated due to UDP packet size limits, so don't require ending with "]"
223+
224+
// Verify newlines are properly escaped (should appear as \n not actual newlines)
225+
assert.Contains(t, mapPart, "\\n", "Newlines should be escaped in map output")
226+
227+
// The key test: ensure the syslog message itself doesn't contain raw newlines that would cause fragmentation
228+
// We check this by ensuring the raw multiline content appears escaped in the single-line syslog message
229+
assert.Contains(t, mapPart, "raw_request:POST /api/users HTTP/1.1\\n", "Should contain escaped newlines in raw_request")
230+
231+
// Verify the original multiline content is present but escaped
232+
assert.Contains(t, mapPart, "raw_request:", "Should contain raw_request field")
233+
assert.Contains(t, mapPart, "raw_response:", "Should contain raw_response field")
234+
assert.Contains(t, mapPart, "POST /api/users HTTP/1.1", "Should contain HTTP request line")
235+
assert.Contains(t, mapPart, "HTTP/1.1 201 Created", "Should contain HTTP status line")
236+
237+
case <-time.After(2 * time.Second):
238+
t.Fatal("Timeout waiting for syslog message")
239+
}
240+
}
241+
242+
func TestSyslogPump_WriteData_SpecialCharacters(t *testing.T) {
243+
// Test data with special characters that could break output
244+
record := analytics.AnalyticsRecord{
245+
Method: "POST",
246+
Path: "/api/test",
247+
ResponseCode: 200,
248+
TimeStamp: time.Now(),
249+
RawRequest: `{"message": "Hello \"World\"", "data": "line1\nline2\ttab\rcarriage"}`,
250+
RawResponse: "Response with unicode: 测试 and emoji: 🚀",
251+
UserAgent: "Agent/1.0 (Compatible; Special chars: []{}();)",
252+
APIKey: "key_with_quotes_\"and\"_backslashes\\",
253+
}
254+
255+
// Create mock syslog server
256+
addr, messages := mockSyslogServer(t)
257+
258+
// Create syslog pump with test configuration
259+
s := createTestSyslogPump(addr)
260+
261+
err := s.WriteData(context.Background(), []interface{}{record})
262+
assert.NoError(t, err)
263+
264+
// Wait for message
265+
select {
266+
case msg := <-messages:
267+
// Extract map from syslog message
268+
mapStart := strings.Index(msg, "map[")
269+
require.True(t, mapStart >= 0, "Message should contain map format: %s", msg)
270+
mapPart := strings.TrimSpace(msg[mapStart:])
271+
272+
// Verify special characters and unicode are handled properly
273+
assert.Contains(t, mapPart, "raw_request:", "Should contain raw_request field")
274+
assert.Contains(t, mapPart, "raw_response:", "Should contain raw_response field")
275+
assert.Contains(t, mapPart, "测试", "Should preserve unicode characters")
276+
assert.Contains(t, mapPart, "🚀", "Should preserve emoji")
277+
278+
// Verify newlines are escaped in the raw_request field
279+
assert.Contains(t, mapPart, "\\n", "Newlines should be escaped")
280+
281+
case <-time.After(2 * time.Second):
282+
t.Fatal("Timeout waiting for syslog message")
283+
}
284+
}
285+
286+
func TestSyslogPump_WriteData_ContextCancellation(t *testing.T) {
287+
record := analytics.AnalyticsRecord{
288+
Method: "GET",
289+
Path: "/api/test",
290+
ResponseCode: 200,
291+
TimeStamp: time.Now(),
292+
}
293+
294+
// Create mock syslog server
295+
addr, messages := mockSyslogServer(t)
296+
297+
// Create syslog pump with test configuration
298+
s := createTestSyslogPump(addr)
299+
300+
// Create cancelled context
301+
ctx, cancel := context.WithCancel(context.Background())
302+
cancel()
303+
304+
err := s.WriteData(ctx, []interface{}{record})
305+
assert.NoError(t, err) // Should not error, just return early
306+
307+
// Should not have written anything due to context cancellation
308+
select {
309+
case <-messages:
310+
t.Error("Should not receive any messages when context is cancelled")
311+
case <-time.After(100 * time.Millisecond):
312+
// Good, no messages received
313+
}
314+
}

0 commit comments

Comments
 (0)