Skip to content

Commit b0a4e19

Browse files
committed
[metricbeat/zookeeper] Fix panics in server parser and add edge case tests
- Add bounds checking for version and date regex matches to prevent index-out-of-range panics on specific input - Fix off-by-one error in mode split check (< 1 → < 2) - Remove unused regex variables (ipCapturer, thatNumberCapturer, portCapturer, dataCapturer) - Add TestParseSrvrEdgeCases with table-driven tests for edge cases
1 parent 56a35a9 commit b0a4e19

File tree

2 files changed

+72
-7
lines changed

2 files changed

+72
-7
lines changed

metricbeat/module/zookeeper/server/data.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ import (
3434
)
3535

3636
var latencyCapturer = regexp.MustCompile(`(\d+)/(\d+)/(\d+)`)
37-
var ipCapturer = regexp.MustCompile(`\d+\.\d+\.\d+\.\d+`)
38-
var thatNumberCapturer = regexp.MustCompile(`\[(\d+)\]`)
39-
var portCapturer = regexp.MustCompile(`:(\d+)\[`)
40-
var dataCapturer = regexp.MustCompile(`(\w+)=(\d+)`)
4137
var fieldsCapturer = regexp.MustCompile(`^([a-zA-Z\s]+):\s(\d+)`)
4238
var versionCapturer = regexp.MustCompile(`:\s(.*),`)
4339
var dateCapturer = regexp.MustCompile(`built on (.*)`)
@@ -54,8 +50,17 @@ func parseSrvr(i io.Reader, logger *logp.Logger) (mapstr.M, string, error) {
5450

5551
output := mapstr.M{}
5652

57-
version := versionCapturer.FindStringSubmatch(scanner.Text())[1]
58-
dateString := dateCapturer.FindStringSubmatch(scanner.Text())[1]
53+
versionMatches := versionCapturer.FindStringSubmatch(scanner.Text())
54+
if len(versionMatches) < 2 {
55+
return nil, "", errors.New("no match found for version")
56+
}
57+
version := versionMatches[1]
58+
59+
dateStringMatches := dateCapturer.FindStringSubmatch(scanner.Text())
60+
if len(dateStringMatches) < 2 {
61+
return nil, "", errors.New("no match found for build date")
62+
}
63+
dateString := dateStringMatches[1]
5964

6065
date, err := time.Parse("01/02/2006 03:04 GMT", dateString)
6166
if err != nil {
@@ -108,7 +113,7 @@ func parseSrvr(i io.Reader, logger *logp.Logger) (mapstr.M, string, error) {
108113

109114
if strings.Contains(line, "Mode") {
110115
modeSplit := strings.Split(line, " ")
111-
if len(modeSplit) < 1 {
116+
if len(modeSplit) < 2 {
112117
logger.Debugf("no tokens after splitting line '%s'", line)
113118
continue
114119
}

metricbeat/module/zookeeper/server/server_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,63 @@ func TestParser(t *testing.T) {
7070
assert.Equal(t, uint32(7), mapStr["epoch"])
7171
assert.Equal(t, uint32(0x601132), mapStr["count"])
7272
}
73+
74+
func TestParseSrvrEdgeCases(t *testing.T) {
75+
tests := []struct {
76+
name string
77+
input string
78+
expectError bool
79+
checkMode bool
80+
hasMode bool
81+
}{
82+
{
83+
name: "invalid version line",
84+
input: "some invalid first line\n",
85+
expectError: true,
86+
},
87+
{
88+
name: "mode line without value",
89+
input: `Zookeeper version: 3.5.5, built on 05/03/2019 12:07 GMT
90+
Mode:
91+
`,
92+
expectError: false,
93+
checkMode: true,
94+
hasMode: false,
95+
},
96+
{
97+
name: "malformed input",
98+
input: "00000000",
99+
expectError: true,
100+
},
101+
{
102+
name: "missing date",
103+
input: ": ,00000",
104+
expectError: true,
105+
},
106+
{
107+
name: "malformed mode line",
108+
input: ": ,built on \nMode",
109+
expectError: false,
110+
checkMode: true,
111+
hasMode: false,
112+
},
113+
}
114+
115+
for _, tt := range tests {
116+
t.Run(tt.name, func(t *testing.T) {
117+
logger := logptest.NewTestingLogger(t, "zookeeper.server")
118+
mapStr, _, err := parseSrvr(bytes.NewReader([]byte(tt.input)), logger)
119+
120+
if tt.expectError {
121+
assert.Error(t, err)
122+
} else {
123+
assert.NoError(t, err)
124+
}
125+
126+
if tt.checkMode {
127+
_, hasMode := mapStr["mode"]
128+
assert.Equal(t, tt.hasMode, hasMode)
129+
}
130+
})
131+
}
132+
}

0 commit comments

Comments
 (0)