From 8f9ede301dc7c7666f32b3bf64e87a67b41ff6e5 Mon Sep 17 00:00:00 2001
From: Daniel Nelson <daniel@wavesofdawn.com>
Date: Wed, 13 Sep 2017 17:30:52 -0700
Subject: [PATCH] Whitelist allowed char classes for opentsdb output. (#3227)

---
 plugins/outputs/opentsdb/opentsdb.go      | 28 +++++++++---
 plugins/outputs/opentsdb/opentsdb_test.go | 56 +++++++++++++++++++++--
 2 files changed, 74 insertions(+), 10 deletions(-)

diff --git a/plugins/outputs/opentsdb/opentsdb.go b/plugins/outputs/opentsdb/opentsdb.go
index ac4d1224e..b7523c7d8 100644
--- a/plugins/outputs/opentsdb/opentsdb.go
+++ b/plugins/outputs/opentsdb/opentsdb.go
@@ -5,6 +5,7 @@ import (
 	"log"
 	"net"
 	"net/url"
+	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -13,6 +14,16 @@ import (
 	"github.com/influxdata/telegraf/plugins/outputs"
 )
 
+var (
+	allowedChars = regexp.MustCompile(`[^a-zA-Z0-9-_./\p{L}]`)
+	hypenChars   = strings.NewReplacer(
+		"@", "-",
+		"*", "-",
+		`%`, "-",
+		"#", "-",
+		"$", "-")
+)
+
 type OpenTSDB struct {
 	Prefix string
 
@@ -24,9 +35,6 @@ type OpenTSDB struct {
 	Debug bool
 }
 
-var sanitizedChars = strings.NewReplacer("@", "-", "*", "-", " ", "_",
-	`%`, "-", "#", "-", "$", "-", ":", "_")
-
 var sampleConfig = `
   ## prefix for metrics keys
   prefix = "my.specific.prefix."
@@ -125,8 +133,7 @@ func (o *OpenTSDB) WriteHttp(metrics []telegraf.Metric, u *url.URL) error {
 			}
 
 			metric := &HttpMetric{
-				Metric: sanitizedChars.Replace(fmt.Sprintf("%s%s_%s",
-					o.Prefix, m.Name(), fieldName)),
+				Metric:    sanitize(fmt.Sprintf("%s%s_%s", o.Prefix, m.Name(), fieldName)),
 				Tags:      tags,
 				Timestamp: now,
 				Value:     value,
@@ -176,7 +183,7 @@ func (o *OpenTSDB) WriteTelnet(metrics []telegraf.Metric, u *url.URL) error {
 			}
 
 			messageLine := fmt.Sprintf("put %s %v %s %s\n",
-				sanitizedChars.Replace(fmt.Sprintf("%s%s_%s", o.Prefix, m.Name(), fieldName)),
+				sanitize(fmt.Sprintf("%s%s_%s", o.Prefix, m.Name(), fieldName)),
 				now, metricValue, tags)
 
 			_, err := connection.Write([]byte(messageLine))
@@ -192,7 +199,7 @@ func (o *OpenTSDB) WriteTelnet(metrics []telegraf.Metric, u *url.URL) error {
 func cleanTags(tags map[string]string) map[string]string {
 	tagSet := make(map[string]string, len(tags))
 	for k, v := range tags {
-		tagSet[sanitizedChars.Replace(k)] = sanitizedChars.Replace(v)
+		tagSet[sanitize(k)] = sanitize(v)
 	}
 	return tagSet
 }
@@ -236,6 +243,13 @@ func (o *OpenTSDB) Close() error {
 	return nil
 }
 
+func sanitize(value string) string {
+	// Apply special hypenation rules to preserve backwards compatibility
+	value = hypenChars.Replace(value)
+	// Replace any remaining illegal chars
+	return allowedChars.ReplaceAllLiteralString(value, "_")
+}
+
 func init() {
 	outputs.Add("opentsdb", func() telegraf.Output {
 		return &OpenTSDB{}
diff --git a/plugins/outputs/opentsdb/opentsdb_test.go b/plugins/outputs/opentsdb/opentsdb_test.go
index 669ab5303..83a659216 100644
--- a/plugins/outputs/opentsdb/opentsdb_test.go
+++ b/plugins/outputs/opentsdb/opentsdb_test.go
@@ -10,9 +10,10 @@ import (
 	"strconv"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/testutil"
-	//"github.com/stretchr/testify/require"
 )
 
 func TestCleanTags(t *testing.T) {
@@ -29,8 +30,16 @@ func TestCleanTags(t *testing.T) {
 			map[string]string{"aaa": "bbb"},
 		},
 		{
-			map[string]string{"Sp%ci@l Chars": "g$t repl#ced"},
-			map[string]string{"Sp-ci-l_Chars": "g-t_repl-ced"},
+			map[string]string{"Sp%ci@l Chars[": "g$t repl#ce)d"},
+			map[string]string{"Sp-ci-l_Chars_": "g-t_repl-ce_d"},
+		},
+		{
+			map[string]string{"μnicodε_letters": "okαy"},
+			map[string]string{"μnicodε_letters": "okαy"},
+		},
+		{
+			map[string]string{"n☺": "emojies☠"},
+			map[string]string{"n_": "emojies_"},
 		},
 		{
 			map[string]string{},
@@ -75,6 +84,47 @@ func TestBuildTagsTelnet(t *testing.T) {
 	}
 }
 
+func TestSanitize(t *testing.T) {
+	tests := []struct {
+		name     string
+		value    string
+		expected string
+	}{
+		{
+			name:     "Ascii letters and numbers allowed",
+			value:    "ascii 123",
+			expected: "ascii_123",
+		},
+		{
+			name:     "Allowed punct",
+			value:    "-_./",
+			expected: "-_./",
+		},
+		{
+			name:     "Special conversions to hyphen",
+			value:    "@*%#$!",
+			expected: "-----_",
+		},
+		{
+			name:     "Unicode Letters allowed",
+			value:    "μnicodε_letters",
+			expected: "μnicodε_letters",
+		},
+		{
+			name:     "Other Unicode not allowed",
+			value:    "“☢”",
+			expected: "___",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual := sanitize(tt.value)
+			require.Equal(t, tt.expected, actual)
+		})
+	}
+}
+
 func BenchmarkHttpSend(b *testing.B) {
 	const BatchSize = 50
 	const MetricsCount = 4 * BatchSize