sanitize names when using them as tag value

lib/carbon/tests/test_util.py

@@ -4,6 +4,7 @@
 
 from carbon.util import parseDestinations
 from carbon.util import enableTcpKeepAlive
+from carbon.util import TaggedSeries
 
 
 class UtilTest(TestCase):
@@ -21,6 +22,72 @@ def setTcpKeepAlive(self, value):
         enableTcpKeepAlive(_Transport(), True, None)
         self.assertEquals(s.getsockopt(socket.SOL_TCP, socket.SO_KEEPALIVE), 1)
 
+    def test_sanitizing_name_as_tag_value(self):
+        test_cases = [
+            {
+                'original': "my~.test.abc",
+                'expected': "my~.test.abc",
+            }, {
+                'original': "a.b.c",
+                'expected': "a.b.c",
+            }, {
+                'original': "~~a~~.~~~b~~~.~~~c~~~",
+                'expected': "a~~.~~~b~~~.~~~c~~~",
+            }, {
+                'original': "a.b.c~",
+                'expected': "a.b.c~",
+            }, {
+                'original': "~a.b.c",
+                'expected': "a.b.c",
+            }, {
+                'original': "~a~",
+                'expected': "a~",
+            }, {
+                'original': "~~~",
+                'raises': True,
+            }, {
+                'original': "~",
+                'raises': True,
+            },
+        ]
+
+        for test_case in test_cases:
+            if test_case.get('raises', False):
+                self.assertRaises(
+                    Exception,
+                    TaggedSeries.sanitize_name_as_tag_value,
+                    test_case['original'],
+                )
+            else:
+                result = TaggedSeries.sanitize_name_as_tag_value(test_case['original'])
+                self.assertEquals(result, test_case['expected'])
+
+    def test_validate_tag_key_and_value(self):
+        # assert that it raises exception when sanitized name is still not valid
+        with self.assertRaises(Exception):
+            # sanitized name is going to be '', which is not a valid tag value
+            TaggedSeries.sanitize_name_as_tag_value('~~~~')
+
+        with self.assertRaises(Exception):
+            # given tag value is invalid because it has length 0
+            TaggedSeries.validateTagAndValue('metric.name;tag=')
+
+        with self.assertRaises(Exception):
+            # given tag key is invalid because it has length 0
+            TaggedSeries.validateTagAndValue('metric.name;=value')
+
+        with self.assertRaises(Exception):
+            # given tag is missing =
+            TaggedSeries.validateTagAndValue('metric.name;tagvalue')
+
+        with self.assertRaises(Exception):
+            # given tag value is invalid because it starts with ~
+            TaggedSeries.validateTagAndValue('metric.name;tag=~value')
+
+        with self.assertRaises(Exception):
+            # given tag key is invalid because it contains !
+            TaggedSeries.validateTagAndValue('metric.name;ta!g=value')
+
 
 # Destinations have the form:
 # <host> ::= <string without colons> | "[" <string> "]"

lib/carbon/util.py

@@ -336,6 +336,28 @@ def __init__(classObj, name, bases, members):
 
 
 class TaggedSeries(object):
+  prohibitedTagChars = ';!^='
+
+  @classmethod
+  def validateTagAndValue(cls, tag, value):
+    """validate the given tag / value based on the specs in the documentation"""
+    if len(tag) == 0:
+      raise Exception('Tag may not be empty')
+    if len(value) == 0:
+      raise Exception('Value for tag "{tag}" may not be empty'.format(tag=tag))
+
+    for char in cls.prohibitedTagChars:
+      if char in tag:
+        raise Exception(
+          'Character "{char}" is not allowed in tag "{tag}"'.format(char=char, tag=tag))
+
+    if ';' in value:
+      raise Exception(
+        'Character ";" is not allowed in value "{value}" of tag {tag}'.format(value=value, tag=tag))
+
+    if value[0] == '~':
+      raise Exception('Tag values are not allowed to start with "~" in tag "{tag}"'.format(tag=tag))
+
   @classmethod
   def parse(cls, path):
     # if path is in openmetrics format: metric{tag="value",...}
@@ -362,10 +384,15 @@ def parse_openmetrics(cls, path):
       if not m:
         raise Exception('Cannot parse path %s, invalid segment %s' % (path, rawtags))
 
-      tags[m.group(1)] = m.group(2).replace(r'\"', '"').replace(r'\\', '\\')
+      tag = m.group(1)
+      value = m.group(2).replace(r'\"', '"').replace(r'\\', '\\')
+
+      cls.validateTagAndValue(tag, value)
+
+      tags[tag] = value
       rawtags = rawtags[len(m.group(0)):]
 
-    tags['name'] = metric
+    tags['name'] = cls.sanitize_name_as_tag_value(metric)
     return cls(metric, tags)
 
   @classmethod
@@ -384,11 +411,23 @@ def parse_carbon(cls, path):
       if len(tag) != 2 or not tag[0]:
         raise Exception('Cannot parse path %s, invalid segment %s' % (path, segment))
 
+      cls.validateTagAndValue(*tag)
+
       tags[tag[0]] = tag[1]
 
-    tags['name'] = metric
+    tags['name'] = cls.sanitize_name_as_tag_value(metric)
     return cls(metric, tags)
 
+  @staticmethod
+  def sanitize_name_as_tag_value(name):
+    """take a metric name and sanitize it so it is guaranteed to be a valid tag value"""
+    sanitized = name.lstrip('~')
+
+    if len(sanitized) == 0:
+      raise Exception('Cannot use metric name %s as tag value, results in emptry string' % (name))
+
+    return sanitized
+
   @staticmethod
   def format(tags):
     return tags.get('name', '') + ''.join(sorted([