Skip to content

Commit 7cf123a

Browse files
committed
fix: limit frame and total sizes when reading docker logs (#1322)
- Add per-frame size limit (1MB) to prevent single massive log entries - Add total log size limit (5MB) for network transfer and browser rendering - Gracefully truncate logs that exceed limits instead of consuming unbounded memory
1 parent 97394e7 commit 7cf123a

2 files changed

Lines changed: 199 additions & 0 deletions

File tree

agent/docker.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ const (
3232
maxMemoryUsage uint64 = 100 * 1024 * 1024 * 1024 * 1024
3333
// Number of log lines to request when fetching container logs
3434
dockerLogsTail = 200
35+
// Maximum size of a single log frame (1MB) to prevent memory exhaustion
36+
// A single log line larger than 1MB is likely an error or misconfiguration
37+
maxLogFrameSize = 1024 * 1024
38+
// Maximum total log content size (5MB) to prevent memory exhaustion
39+
// This provides a reasonable limit for network transfer and browser rendering
40+
maxTotalLogSize = 5 * 1024 * 1024
3541
)
3642

3743
type dockerManager struct {
@@ -657,6 +663,7 @@ func decodeDockerLogStream(reader io.Reader, builder *strings.Builder) error {
657663
const headerSize = 8
658664
var header [headerSize]byte
659665
buf := make([]byte, 0, dockerLogsTail*200)
666+
totalBytesRead := 0
660667

661668
for {
662669
if _, err := io.ReadFull(reader, header[:]); err != nil {
@@ -671,6 +678,19 @@ func decodeDockerLogStream(reader io.Reader, builder *strings.Builder) error {
671678
continue
672679
}
673680

681+
// Prevent memory exhaustion from excessively large frames
682+
if frameLen > maxLogFrameSize {
683+
return fmt.Errorf("log frame size (%d) exceeds maximum (%d)", frameLen, maxLogFrameSize)
684+
}
685+
686+
// Check if reading this frame would exceed total log size limit
687+
if totalBytesRead+int(frameLen) > maxTotalLogSize {
688+
// Read and discard remaining data to avoid blocking
689+
_, _ = io.Copy(io.Discard, io.LimitReader(reader, int64(frameLen)))
690+
slog.Debug("Truncating logs: limit reached", "read", totalBytesRead, "limit", maxTotalLogSize)
691+
return nil
692+
}
693+
674694
buf = allocateBuffer(buf, int(frameLen))
675695
if _, err := io.ReadFull(reader, buf[:frameLen]); err != nil {
676696
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
@@ -682,6 +702,7 @@ func decodeDockerLogStream(reader io.Reader, builder *strings.Builder) error {
682702
return err
683703
}
684704
builder.Write(buf[:frameLen])
705+
totalBytesRead += int(frameLen)
685706
}
686707
}
687708

agent/docker_test.go

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
package agent
55

66
import (
7+
"bytes"
78
"encoding/json"
89
"os"
10+
"strings"
911
"testing"
1012
"time"
1113

@@ -911,6 +913,8 @@ func TestConstantsAndUtilityFunctions(t *testing.T) {
911913
assert.Equal(t, uint16(60000), defaultCacheTimeMs)
912914
assert.Equal(t, uint64(5e9), maxNetworkSpeedBps)
913915
assert.Equal(t, 2100, dockerTimeoutMs)
916+
assert.Equal(t, uint32(1024*1024), uint32(maxLogFrameSize)) // 1MB
917+
assert.Equal(t, 5*1024*1024, maxTotalLogSize) // 5MB
914918

915919
// Test utility functions
916920
assert.Equal(t, 1.5, twoDecimals(1.499))
@@ -921,3 +925,177 @@ func TestConstantsAndUtilityFunctions(t *testing.T) {
921925
assert.Equal(t, 0.5, bytesToMegabytes(524288)) // 512 KB
922926
assert.Equal(t, 0.0, bytesToMegabytes(0))
923927
}
928+
929+
func TestDecodeDockerLogStream(t *testing.T) {
930+
tests := []struct {
931+
name string
932+
input []byte
933+
expected string
934+
expectError bool
935+
}{
936+
{
937+
name: "simple log entry",
938+
input: []byte{
939+
// Frame 1: stdout, 11 bytes
940+
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0B,
941+
'H', 'e', 'l', 'l', 'o', ' ', 'W', 'o', 'r', 'l', 'd',
942+
},
943+
expected: "Hello World",
944+
expectError: false,
945+
},
946+
{
947+
name: "multiple frames",
948+
input: []byte{
949+
// Frame 1: stdout, 5 bytes
950+
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05,
951+
'H', 'e', 'l', 'l', 'o',
952+
// Frame 2: stdout, 5 bytes
953+
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05,
954+
'W', 'o', 'r', 'l', 'd',
955+
},
956+
expected: "HelloWorld",
957+
expectError: false,
958+
},
959+
{
960+
name: "zero length frame",
961+
input: []byte{
962+
// Frame 1: stdout, 0 bytes
963+
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
964+
// Frame 2: stdout, 5 bytes
965+
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05,
966+
'H', 'e', 'l', 'l', 'o',
967+
},
968+
expected: "Hello",
969+
expectError: false,
970+
},
971+
{
972+
name: "empty input",
973+
input: []byte{},
974+
expected: "",
975+
expectError: false,
976+
},
977+
}
978+
979+
for _, tt := range tests {
980+
t.Run(tt.name, func(t *testing.T) {
981+
reader := bytes.NewReader(tt.input)
982+
var builder strings.Builder
983+
err := decodeDockerLogStream(reader, &builder)
984+
985+
if tt.expectError {
986+
assert.Error(t, err)
987+
} else {
988+
assert.NoError(t, err)
989+
assert.Equal(t, tt.expected, builder.String())
990+
}
991+
})
992+
}
993+
}
994+
995+
func TestDecodeDockerLogStreamMemoryProtection(t *testing.T) {
996+
t.Run("excessively large frame should error", func(t *testing.T) {
997+
// Create a frame with size exceeding maxLogFrameSize
998+
excessiveSize := uint32(maxLogFrameSize + 1)
999+
input := []byte{
1000+
// Frame header with excessive size
1001+
0x01, 0x00, 0x00, 0x00,
1002+
byte(excessiveSize >> 24), byte(excessiveSize >> 16), byte(excessiveSize >> 8), byte(excessiveSize),
1003+
}
1004+
1005+
reader := bytes.NewReader(input)
1006+
var builder strings.Builder
1007+
err := decodeDockerLogStream(reader, &builder)
1008+
1009+
assert.Error(t, err)
1010+
assert.Contains(t, err.Error(), "log frame size")
1011+
assert.Contains(t, err.Error(), "exceeds maximum")
1012+
})
1013+
1014+
t.Run("total size limit should truncate", func(t *testing.T) {
1015+
// Create frames that exceed maxTotalLogSize (5MB)
1016+
// Use frames within maxLogFrameSize (1MB) to avoid single-frame rejection
1017+
frameSize := uint32(800 * 1024) // 800KB per frame
1018+
var input []byte
1019+
1020+
// Frames 1-6: 800KB each (total 4.8MB - within 5MB limit)
1021+
for i := 0; i < 6; i++ {
1022+
char := byte('A' + i)
1023+
frameHeader := []byte{
1024+
0x01, 0x00, 0x00, 0x00,
1025+
byte(frameSize >> 24), byte(frameSize >> 16), byte(frameSize >> 8), byte(frameSize),
1026+
}
1027+
input = append(input, frameHeader...)
1028+
input = append(input, bytes.Repeat([]byte{char}, int(frameSize))...)
1029+
}
1030+
1031+
// Frame 7: 800KB (would bring total to 5.6MB, exceeding 5MB limit - should be truncated)
1032+
frame7Header := []byte{
1033+
0x01, 0x00, 0x00, 0x00,
1034+
byte(frameSize >> 24), byte(frameSize >> 16), byte(frameSize >> 8), byte(frameSize),
1035+
}
1036+
input = append(input, frame7Header...)
1037+
input = append(input, bytes.Repeat([]byte{'Z'}, int(frameSize))...)
1038+
1039+
reader := bytes.NewReader(input)
1040+
var builder strings.Builder
1041+
err := decodeDockerLogStream(reader, &builder)
1042+
1043+
// Should complete without error (graceful truncation)
1044+
assert.NoError(t, err)
1045+
// Should have read 6 frames (4.8MB total, stopping before 7th would exceed 5MB limit)
1046+
expectedSize := int(frameSize) * 6
1047+
assert.Equal(t, expectedSize, builder.Len())
1048+
// Should contain A-F but not Z
1049+
result := builder.String()
1050+
assert.Contains(t, result, "A")
1051+
assert.Contains(t, result, "F")
1052+
assert.NotContains(t, result, "Z")
1053+
})
1054+
}
1055+
1056+
func TestAllocateBuffer(t *testing.T) {
1057+
tests := []struct {
1058+
name string
1059+
currentCap int
1060+
needed int
1061+
expectedCap int
1062+
shouldRealloc bool
1063+
}{
1064+
{
1065+
name: "buffer has enough capacity",
1066+
currentCap: 1024,
1067+
needed: 512,
1068+
expectedCap: 1024,
1069+
shouldRealloc: false,
1070+
},
1071+
{
1072+
name: "buffer needs reallocation",
1073+
currentCap: 512,
1074+
needed: 1024,
1075+
expectedCap: 1024,
1076+
shouldRealloc: true,
1077+
},
1078+
{
1079+
name: "buffer needs exact size",
1080+
currentCap: 1024,
1081+
needed: 1024,
1082+
expectedCap: 1024,
1083+
shouldRealloc: false,
1084+
},
1085+
}
1086+
1087+
for _, tt := range tests {
1088+
t.Run(tt.name, func(t *testing.T) {
1089+
current := make([]byte, 0, tt.currentCap)
1090+
result := allocateBuffer(current, tt.needed)
1091+
1092+
assert.Equal(t, tt.needed, len(result))
1093+
assert.GreaterOrEqual(t, cap(result), tt.expectedCap)
1094+
1095+
if tt.shouldRealloc {
1096+
// If reallocation was needed, capacity should be at least the needed size
1097+
assert.GreaterOrEqual(t, cap(result), tt.needed)
1098+
}
1099+
})
1100+
}
1101+
}

0 commit comments

Comments
 (0)