From 32015e1109bc955697353d8f8088e3f6b538d12c Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Wed, 2 May 2018 16:52:42 +0100
Subject: [PATCH 1/2] Escape label values in prometheus metrics

---
 synapse/metrics/metric.py    | 22 ++++++++++++++++++++--
 tests/metrics/test_metric.py | 21 ++++++++++++++++++++-
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 89bd47c3f..1a09e417c 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -16,6 +16,7 @@
 
 from itertools import chain
 import logging
+import re
 
 logger = logging.getLogger(__name__)
 
@@ -56,8 +57,7 @@ class BaseMetric(object):
         return not len(self.labels)
 
     def _render_labelvalue(self, value):
-        # TODO: escape backslashes, quotes and newlines
-        return '"%s"' % (value)
+        return '"%s"' % (_escape_label_value(value),)
 
     def _render_key(self, values):
         if self.is_scalar():
@@ -299,3 +299,21 @@ class MemoryUsageMetric(object):
             "process_psutil_rss:total %d" % sum_rss,
             "process_psutil_rss:count %d" % len_rss,
         ]
+
+
+def _escape_character(c):
+    """Replaces a single character with its escape sequence.
+    """
+    if c == "\\":
+        return "\\\\"
+    elif c == "\"":
+        return "\\\""
+    elif c == "\n":
+        return "\\n"
+    return c
+
+
+def _escape_label_value(value):
+    """Takes a label value and escapes quotes, newlines and backslashes
+    """
+    return re.sub(r"([\n\"\\])", lambda m: _escape_character(m.group(1)), value)
diff --git a/tests/metrics/test_metric.py b/tests/metrics/test_metric.py
index 39bde6e3f..069c0be76 100644
--- a/tests/metrics/test_metric.py
+++ b/tests/metrics/test_metric.py
@@ -16,7 +16,8 @@
 from tests import unittest
 
 from synapse.metrics.metric import (
-    CounterMetric, CallbackMetric, DistributionMetric, CacheMetric
+    CounterMetric, CallbackMetric, DistributionMetric, CacheMetric,
+    _escape_label_value,
 )
 
 
@@ -171,3 +172,21 @@ class CacheMetricTestCase(unittest.TestCase):
             'cache:size{name="cache_name"} 1',
             'cache:evicted_size{name="cache_name"} 2',
         ])
+
+
+class LabelValueEscapeTestCase(unittest.TestCase):
+    def test_simple(self):
+        string = "safjhsdlifhyskljfksdfh"
+        self.assertEqual(string, _escape_label_value(string))
+
+    def test_escape(self):
+        self.assertEqual(
+            "abc\\\"def\\nghi\\\\",
+            _escape_label_value("abc\"def\nghi\\"),
+        )
+
+    def test_sequence_of_escapes(self):
+        self.assertEqual(
+            "abc\\\"def\\nghi\\\\\\n",
+            _escape_label_value("abc\"def\nghi\\\n"),
+        )

From a41117c63b4e7ef2021fb59cb521c77d72f2bf70 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Wed, 2 May 2018 17:27:27 +0100
Subject: [PATCH 2/2] Make _escape_character take MatchObject

---
 synapse/metrics/metric.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/synapse/metrics/metric.py b/synapse/metrics/metric.py
index 1a09e417c..fbba94e63 100644
--- a/synapse/metrics/metric.py
+++ b/synapse/metrics/metric.py
@@ -301,9 +301,17 @@ class MemoryUsageMetric(object):
         ]
 
 
-def _escape_character(c):
+def _escape_character(m):
     """Replaces a single character with its escape sequence.
+
+    Args:
+        m (re.MatchObject): A match object whose first group is the single
+            character to replace
+
+    Returns:
+        str
     """
+    c = m.group(1)
     if c == "\\":
         return "\\\\"
     elif c == "\"":
@@ -316,4 +324,4 @@ def _escape_character(c):
 def _escape_label_value(value):
     """Takes a label value and escapes quotes, newlines and backslashes
     """
-    return re.sub(r"([\n\"\\])", lambda m: _escape_character(m.group(1)), value)
+    return re.sub(r"([\n\"\\])", _escape_character, value)