[metricbeat/graphite] Fix out-of-bounds panic in graphite server metric processing

metricbeat/module/graphite/server/data.go

@@ -119,12 +119,12 @@
 	parts := strings.Fields(metric)
 	currentTime := common.Time(time.Now())
 	if len(parts) < 2 {
 		return "", currentTime, 0, errors.New("Message not in expected format")
 	} else {
 		metricName = parts[0]
 		val, err := strconv.ParseFloat(parts[1], 64)
 		if err != nil {
 			return "", currentTime, 0, errors.New("Unable to parse metric value")
 		} else {
 			value = val
 		}
@@ -132,11 +132,11 @@
 
 	if len(parts) == 3 {
 		if parts[2] == "N" {
 			timestamp = currentTime
 		}
 		ts, err := strconv.ParseFloat(parts[2], 64)
 		if err != nil {
 			return "", currentTime, 0, errors.New("Unable to parse timestamp")
 		}
 
 		if ts != -1 {
@@ -155,19 +155,24 @@
 func (t *template) Apply(parts []string) (string, mapstr.M) {
 	tags := make(mapstr.M)
 
-	metric := make([]string, 0)
 	for tagKey, tagVal := range t.Tags {
 		tags[tagKey] = tagVal
 	}
 
+	var metric []string
 	tagsMap := make(map[string][]string)
-	for i := 0; i < len(t.Parts); i++ {
-		if t.Parts[i] == "metric" {
+
+	// Match template parts to corresponding metric parts.
+	for i, templatePart := range t.Parts[:min(len(t.Parts), len(parts))] {
+		switch templatePart {
+		case "metric":
 			metric = append(metric, parts[i])
-		} else if t.Parts[i] == "metric*" {
+		case "metric*":
 			metric = append(metric, parts[i:]...)
-		} else if t.Parts[i] != "" {
-			tagsMap[t.Parts[i]] = append(tagsMap[t.Parts[i]], parts[i])
+		case "":
+			// skip empty parts
+		default:
+			tagsMap[templatePart] = append(tagsMap[templatePart], parts[i])
 		}
 	}
 
@@ -177,7 +182,6 @@
 
 	if len(metric) == 0 {
 		return "", tags
-	} else {
-		return strings.Join(metric, t.Delimiter), tags
 	}
+	return strings.Join(metric, t.Delimiter), tags
 }

metricbeat/module/graphite/server/data_test.go

@@ -25,6 +25,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/elastic/beats/v7/libbeat/common"
 	"github.com/elastic/elastic-agent-libs/mapstr"
@@ -79,7 +80,6 @@ func TestMetricProcessorDeleteTemplate(t *testing.T) {
 	processor.RemoveTemplate(temp)
 	out := processor.templates.Search([]string{"a", "b", "c"})
 	assert.Nil(t, out)
-
 }
 
 func TestMetricProcessorProcess(t *testing.T) {
@@ -108,3 +108,125 @@ func TestMetricProcessorProcess(t *testing.T) {
 	assert.NotNil(t, event["stats"])
 	assert.Equal(t, event["stats"], float64(42))
 }
+
+func TestTemplateApply(t *testing.T) {
+	tests := []struct {
+		name          string
+		tmpl          template
+		parts         []string
+		wantMetric    string
+		wantTagsCount int
+	}{
+		{
+			name: "metric shorter than template",
+			tmpl: template{
+				Delimiter: ".",
+				Parts:     []string{"", "host", "region", "service", "metric"},
+			},
+			parts:         []string{"server1", "us-east"},
+			wantMetric:    "",
+			wantTagsCount: 1,
+		},
+		{
+			name: "single part metric",
+			tmpl: template{
+				Delimiter: ".",
+				Parts:     []string{"", "host", "region", "service", "metric"},
+			},
+			parts:         []string{"server1"},
+			wantMetric:    "",
+			wantTagsCount: 0,
+		},
+		{
+			name: "empty metric parts",
+			tmpl: template{
+				Delimiter: ".",
+				Parts:     []string{"", "host", "region", "service", "metric"},
+			},
+			parts:         []string{},
+			wantMetric:    "",
+			wantTagsCount: 0,
+		},
+		{
+			name: "nil metric parts",
+			tmpl: template{
+				Delimiter: ".",
+				Parts:     []string{"", "host", "region", "service", "metric"},
+			},
+			parts:         nil,
+			wantMetric:    "",
+			wantTagsCount: 0,
+		},
+		{
+			name: "metric star captures remaining from current index",
+			tmpl: template{
+				Delimiter: "_",
+				Parts:     []string{"", "host", "metric*"},
+			},
+			parts:         []string{"server1", "cpu", "idle", "percent"},
+			wantMetric:    "idle_percent",
+			wantTagsCount: 1,
+		},
+		{
+			name: "empty template parts",
+			tmpl: template{
+				Delimiter: ".",
+				Parts:     []string{},
+			},
+			parts:         []string{"server1", "us-east"},
+			wantMetric:    "",
+			wantTagsCount: 0,
+		},
+		{
+			name: "template with predefined tags",
+			tmpl: template{
+				Delimiter: ".",
+				Parts:     []string{"metric"},
+				Tags:      map[string]string{"env": "prod", "dc": "us-east"},
+			},
+			parts:         []string{"cpu"},
+			wantMetric:    "cpu",
+			wantTagsCount: 2,
+		},
+		{
+			name: "duplicate tag keys in template are combined",
+			tmpl: template{
+				Delimiter: "_",
+				Parts:     []string{"host", "host", "metric"},
+			},
+			parts:         []string{"server1", "server2", "cpu"},
+			wantMetric:    "cpu",
+			wantTagsCount: 1,
+		},
+		{
+			name: "all parts are metric",
+			tmpl: template{
+				Delimiter: ".",
+				Parts:     []string{"metric", "metric", "metric"},
+			},
+			parts:         []string{"cpu", "idle", "percent"},
+			wantMetric:    "cpu.idle.percent",
+			wantTagsCount: 0,
+		},
+		{
+			name: "metric star at beginning",
+			tmpl: template{
+				Delimiter: "_",
+				Parts:     []string{"metric*"},
+			},
+			parts:         []string{"cpu", "idle", "percent"},
+			wantMetric:    "cpu_idle_percent",
+			wantTagsCount: 0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.NotPanics(t, func() {
+				metric, tags := tt.tmpl.Apply(tt.parts)
+				assert.Equal(t, tt.wantMetric, metric)
+				assert.Len(t, tags, tt.wantTagsCount)
+			})
+		})
+	}
+}