From 98ba51f44f4324cbc9a27f90551630532805c60d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Wed, 20 Nov 2019 17:44:41 +0200
Subject: [PATCH] Use hc.lib.string.replace for webhooks too.

hc.lib.string.replace only replaces placeholders that appear in the original template. It ignores any placeholders that "emerge" while doing string substitutions. This is done mainly to avoid unexpected behavior when check names or tags contain dollar signs.
---
 hc/api/tests/test_notify.py | 12 ++++++++++++
 hc/api/transports.py        | 38 +++++++++++--------------------------
 2 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py
index 024eb145..b195ac53 100644
--- a/hc/api/tests/test_notify.py
+++ b/hc/api/tests/test_notify.py
@@ -91,6 +91,18 @@ class NotifyTestCase(BaseTestCase):
         self.assertEqual(kwargs["headers"], {"User-Agent": "healthchecks.io"})
         self.assertEqual(kwargs["timeout"], 5)
 
+    @patch("hc.api.transports.requests.request")
+    def test_webhooks_handle_variable_variables(self, mock_get):
+        self._setup_data("webhook", "http://host/$$NAMETAG1")
+        self.check.tags = "foo bar"
+        self.check.save()
+
+        self.channel.notify(self.check)
+
+        # $$NAMETAG1 should *not* get transformed to "foo"
+        args, kwargs = mock_get.call_args
+        self.assertEqual(args[1], "http://host/$TAG1")
+
     @patch("hc.api.transports.requests.request")
     def test_webhooks_support_post(self, mock_request):
         template = "http://example.com\n\nThe Time Is $NOW"
diff --git a/hc/api/transports.py b/hc/api/transports.py
index f849c460..ac464662 100644
--- a/hc/api/transports.py
+++ b/hc/api/transports.py
@@ -188,39 +188,23 @@ class HttpTransport(Transport):
 
 class Webhook(HttpTransport):
     def prepare(self, template, check, urlencode=False):
-        """ Replace variables with actual values.
-
-        There should be no bad translations if users use $ symbol in
-        check's name or tags, because $ gets urlencoded to %24
-
-        """
+        """ Replace variables with actual values. """
 
         def safe(s):
             return quote(s) if urlencode else s
 
-        result = template
-        if "$CODE" in result:
-            result = result.replace("$CODE", str(check.code))
+        ctx = {
+            "$CODE": str(check.code),
+            "$STATUS": check.status,
+            "$NOW": safe(timezone.now().replace(microsecond=0).isoformat()),
+            "$NAME": safe(check.name),
+            "$TAGS": safe(check.tags),
+        }
 
-        if "$STATUS" in result:
-            result = result.replace("$STATUS", check.status)
+        for i, tag in enumerate(check.tags_list()):
+            ctx["$TAG%d" % (i + 1)] = safe(tag)
 
-        if "$NOW" in result:
-            s = timezone.now().replace(microsecond=0).isoformat()
-            result = result.replace("$NOW", safe(s))
-
-        if "$NAME" in result:
-            result = result.replace("$NAME", safe(check.name))
-
-        if "$TAGS" in result:
-            result = result.replace("$TAGS", safe(check.tags))
-
-        if "$TAG" in result:
-            for i, tag in enumerate(check.tags_list()):
-                placeholder = "$TAG%d" % (i + 1)
-                result = result.replace(placeholder, safe(tag))
-
-        return result
+        return replace(template, ctx)
 
     def is_noop(self, check):
         if check.status == "down" and not self.channel.url_down: