From 6b350edf596a24cedc754793585874e83bf5cdc6 Mon Sep 17 00:00:00 2001
From: Edd Robinson <me@edd.io>
Date: Wed, 14 Mar 2018 13:28:55 +0000
Subject: [PATCH] Ensure correct number of tags parsed

This commit fixes a parsing bug that was causing extra tags to be
generated when the incoming point contained escaped commas.

As an optimisation, the slice of tags associated with a point was
being pre-allocated using the number of commas in the series key as a hint to
the appropriate size. The hinting did not consider literal comma values in
the key though, and so it was possible for extra (empty) tag key and
value pairs to be part of the tags structure associated with a parsed
point.
---
 models/points.go      | 10 +++++-----
 models/points_test.go | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/models/points.go b/models/points.go
index 15478137675..f2905b5c3cf 100644
--- a/models/points.go
+++ b/models/points.go
@@ -1551,12 +1551,12 @@ func parseTags(buf []byte) Tags {
 		return nil
 	}
 
-	tags := make(Tags, bytes.Count(buf, []byte(",")))
-	p := 0
+	// Series keys can contain escaped commas, therefore the number of commas
+	// in a series key only gives an estimation of the upper bound on the number
+	// of tags.
+	tags := make(Tags, 0, bytes.Count(buf, []byte(",")))
 	walkTags(buf, func(key, value []byte) bool {
-		tags[p].Key = key
-		tags[p].Value = value
-		p++
+		tags = append(tags, Tag{Key: key, Value: value})
 		return true
 	})
 	return tags
diff --git a/models/points_test.go b/models/points_test.go
index a0131bfe9b5..94f8a614771 100644
--- a/models/points_test.go
+++ b/models/points_test.go
@@ -96,6 +96,42 @@ func BenchmarkMarshal(b *testing.B) {
 		tags.HashKey()
 	}
 }
+func TestPoint_Tags(t *testing.T) {
+	examples := []struct {
+		Point string
+		Tags  models.Tags
+	}{
+		{`cpu value=1`, models.Tags{}},
+		{"cpu,tag0=v0 value=1", models.NewTags(map[string]string{"tag0": "v0"})},
+		{"cpu,tag0=v0,tag1=v0 value=1", models.NewTags(map[string]string{"tag0": "v0", "tag1": "v0"})},
+		{`cpu,tag0=v\ 0 value=1`, models.NewTags(map[string]string{"tag0": "v 0"})},
+		{`cpu,tag0=v\ 0\ 1,tag1=v2 value=1`, models.NewTags(map[string]string{"tag0": "v 0 1", "tag1": "v2"})},
+		{`cpu,tag0=\, value=1`, models.NewTags(map[string]string{"tag0": ","})},
+		{`cpu,ta\ g0=\, value=1`, models.NewTags(map[string]string{"ta g0": ","})},
+		{`cpu,tag0=\,1 value=1`, models.NewTags(map[string]string{"tag0": ",1"})},
+		{`cpu,tag0=1\"\",t=k value=1`, models.NewTags(map[string]string{"tag0": `1\"\"`, "t": "k"})},
+	}
+
+	for _, example := range examples {
+		t.Run(example.Point, func(t *testing.T) {
+			pts, err := models.ParsePointsString(example.Point)
+			if err != nil {
+				t.Fatal(err)
+			} else if len(pts) != 1 {
+				t.Fatalf("parsed %d points, expected 1", len(pts))
+			}
+
+			// Repeat to test Tags() caching
+			for i := 0; i < 2; i++ {
+				tags := pts[0].Tags()
+				if !reflect.DeepEqual(tags, example.Tags) {
+					t.Fatalf("got %#v (%s), expected %#v", tags, tags.String(), example.Tags)
+				}
+			}
+
+		})
+	}
+}
 
 func TestPoint_StringSize(t *testing.T) {
 	testPoint_cube(t, func(p models.Point) {