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

@@ -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"

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.
+        """ 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
-
-        """
 
         def safe(s):
             return quote(s) if urlencode else s
 
-        result = template
+        ctx = {
-        if "$CODE" in result:
+            "$CODE": str(check.code),
-            result = result.replace("$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)
-
-        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)
+            ctx["$TAG%d" % (i + 1)] = safe(tag)
-                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: