Do not allow metrics with trailing slashes (#3007)

It is not possible to encode a measurement, tag, or field whose last
character is a backslash due to it being an unescapable character.
Because the tight coupling between line protocol and the internal metric
model, prevent metrics like this from being created.

Measurements with a trailing slash are not allowed and the point will be
dropped.  Tags and fields with a trailing a slash will be dropped from
the point.

internal/models/makemetric.go

@@ -3,6 +3,7 @@ package models
 import (
 	"log"
 	"math"
+	"strings"
 	"time"
 
 	"github.com/influxdata/telegraf"
@@ -77,7 +78,27 @@ func makemetric(
 		}
 	}
 
+	for k, v := range tags {
+		if strings.HasSuffix(k, `\`) {
+			log.Printf("D! Measurement [%s] tag [%s] "+
+				"ends with a backslash, skipping", measurement, k)
+			delete(tags, k)
+			continue
+		} else if strings.HasSuffix(v, `\`) {
+			log.Printf("D! Measurement [%s] tag [%s] has a value "+
+				"ending with a backslash, skipping", measurement, k)
+			delete(tags, k)
+			continue
+		}
+	}
+
 	for k, v := range fields {
+		if strings.HasSuffix(k, `\`) {
+			log.Printf("D! Measurement [%s] field [%s] "+
+				"ends with a backslash, skipping", measurement, k)
+			delete(fields, k)
+			continue
+		}
 		// Validate uint64 and float64 fields
 		// convert all int & uint types to int64
 		switch val := v.(type) {
@@ -128,6 +149,14 @@ func makemetric(
 				delete(fields, k)
 				continue
 			}
+		case string:
+			if strings.HasSuffix(val, `\`) {
+				log.Printf("D! Measurement [%s] field [%s] has a value "+
+					"ending with a backslash, skipping", measurement, k)
+				delete(fields, k)
+				continue
+			}
+			fields[k] = v
 		default:
 			fields[k] = v
 		}

internal/models/running_input_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/influxdata/telegraf"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestMakeMetricNoFields(t *testing.T) {
@@ -332,6 +333,128 @@ func TestMakeMetricNameSuffix(t *testing.T) {
 	)
 }
 
+func TestMakeMetric_TrailingSlash(t *testing.T) {
+	now := time.Now()
+
+	tests := []struct {
+		name                string
+		measurement         string
+		fields              map[string]interface{}
+		tags                map[string]string
+		expectedNil         bool
+		expectedMeasurement string
+		expectedFields      map[string]interface{}
+		expectedTags        map[string]string
+	}{
+		{
+			name:        "Measurement cannot have trailing slash",
+			measurement: `cpu\`,
+			fields: map[string]interface{}{
+				"value": int64(42),
+			},
+			tags:        map[string]string{},
+			expectedNil: true,
+		},
+		{
+			name:        "Field key with trailing slash dropped",
+			measurement: `cpu`,
+			fields: map[string]interface{}{
+				"value": int64(42),
+				`bad\`:  `xyzzy`,
+			},
+			tags:                map[string]string{},
+			expectedMeasurement: `cpu`,
+			expectedFields: map[string]interface{}{
+				"value": int64(42),
+			},
+			expectedTags: map[string]string{},
+		},
+		{
+			name:        "Field value with trailing slash dropped",
+			measurement: `cpu`,
+			fields: map[string]interface{}{
+				"value": int64(42),
+				"bad":   `xyzzy\`,
+			},
+			tags:                map[string]string{},
+			expectedMeasurement: `cpu`,
+			expectedFields: map[string]interface{}{
+				"value": int64(42),
+			},
+			expectedTags: map[string]string{},
+		},
+		{
+			name:        "Must have one field after dropped",
+			measurement: `cpu`,
+			fields: map[string]interface{}{
+				"bad": `xyzzy\`,
+			},
+			tags:        map[string]string{},
+			expectedNil: true,
+		},
+		{
+			name:        "Tag key with trailing slash dropped",
+			measurement: `cpu`,
+			fields: map[string]interface{}{
+				"value": int64(42),
+			},
+			tags: map[string]string{
+				`host\`: "localhost",
+				"a":     "x",
+			},
+			expectedMeasurement: `cpu`,
+			expectedFields: map[string]interface{}{
+				"value": int64(42),
+			},
+			expectedTags: map[string]string{
+				"a": "x",
+			},
+		},
+		{
+			name:        "Tag value with trailing slash dropped",
+			measurement: `cpu`,
+			fields: map[string]interface{}{
+				"value": int64(42),
+			},
+			tags: map[string]string{
+				`host`: `localhost\`,
+				"a":    "x",
+			},
+			expectedMeasurement: `cpu`,
+			expectedFields: map[string]interface{}{
+				"value": int64(42),
+			},
+			expectedTags: map[string]string{
+				"a": "x",
+			},
+		},
+	}
+
+	ri := NewRunningInput(&testInput{}, &InputConfig{
+		Name: "TestRunningInput",
+	})
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			m := ri.MakeMetric(
+				tc.measurement,
+				tc.fields,
+				tc.tags,
+				telegraf.Untyped,
+				now)
+
+			if tc.expectedNil {
+				require.Nil(t, m)
+			} else {
+				require.NotNil(t, m)
+				require.Equal(t, tc.expectedMeasurement, m.Name())
+				require.Equal(t, tc.expectedFields, m.Fields())
+				require.Equal(t, tc.expectedTags, m.Tags())
+			}
+		})
+	}
+}
+
 type testInput struct{}
 
 func (t *testInput) Description() string                   { return "" }

metric/metric.go

@@ -6,6 +6,7 @@ import (
 	"hash/fnv"
 	"sort"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/influxdata/telegraf"
@@ -26,6 +27,9 @@ func New(
 	if len(name) == 0 {
 		return nil, fmt.Errorf("Metric cannot be made with an empty name")
 	}
+	if strings.HasSuffix(name, `\`) {
+		return nil, fmt.Errorf("Metric cannot have measurement name ending with a backslash")
+	}
 
 	var thisType telegraf.ValueType
 	if len(mType) > 0 {
@@ -44,6 +48,13 @@ func New(
 	// pre-allocate exact size of the tags slice
 	taglen := 0
 	for k, v := range tags {
+		if strings.HasSuffix(k, `\`) {
+			return nil, fmt.Errorf("Metric cannot have tag key ending with a backslash")
+		}
+		if strings.HasSuffix(v, `\`) {
+			return nil, fmt.Errorf("Metric cannot have tag value ending with a backslash")
+		}
+
 		if len(k) == 0 || len(v) == 0 {
 			continue
 		}
@@ -66,7 +77,17 @@ func New(
 
 	// pre-allocate capacity of the fields slice
 	fieldlen := 0
-	for k, _ := range fields {
+	for k, v := range fields {
+		if strings.HasSuffix(k, `\`) {
+			return nil, fmt.Errorf("Metric cannot have field key ending with a backslash")
+		}
+		switch val := v.(type) {
+		case string:
+			if strings.HasSuffix(val, `\`) {
+				return nil, fmt.Errorf("Metric cannot have field value ending with a backslash")
+			}
+		}
+
 		// 10 bytes is completely arbitrary, but will at least prevent some
 		// amount of allocations. There's a small possibility this will create
 		// slightly more allocations for a metric that has many short fields.

metric/metric_test.go

@@ -687,3 +687,55 @@ func TestEmptyTagValueOrKey(t *testing.T) {
 	assert.NoError(t, err)
 
 }
+
+func TestNewMetric_TrailingSlash(t *testing.T) {
+	now := time.Now()
+
+	tests := []struct {
+		name   string
+		tags   map[string]string
+		fields map[string]interface{}
+	}{
+		{
+			name: `cpu\`,
+			fields: map[string]interface{}{
+				"value": int64(42),
+			},
+		},
+		{
+			name: "cpu",
+			fields: map[string]interface{}{
+				`value\`: "x",
+			},
+		},
+		{
+			name: "cpu",
+			fields: map[string]interface{}{
+				"value": `x\`,
+			},
+		},
+		{
+			name: "cpu",
+			tags: map[string]string{
+				`host\`: "localhost",
+			},
+			fields: map[string]interface{}{
+				"value": int64(42),
+			},
+		},
+		{
+			name: "cpu",
+			tags: map[string]string{
+				"host": `localhost\`,
+			},
+			fields: map[string]interface{}{
+				"value": int64(42),
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		_, err := New(tc.name, tc.tags, tc.fields, now)
+		assert.Error(t, err)
+	}
+}