Add Ping.body_raw field for storing body as bytes

CHANGELOG.md

@@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file.
 - Update Signal integration to use JSON RPC over UNIX socket
 - Update the "Add TOTP" form to display plaintext TOTP secret (#602)
 - Improve PagerDuty notifications
+- Add Ping.body_raw field for storing body as bytes
 
 ### Bug Fixes
 - Fix unwanted special character escaping in notification messages (#606)

hc/api/management/commands/smtpd.py

@@ -26,11 +26,6 @@ def _process_message(remote_addr, mailfrom, mailto, data):
     to_parts = mailto.split("@")
     code = to_parts[0]
 
-    try:
-        data = data.decode()
-    except UnicodeError:
-        data = "[binary data]"
-
     if not RE_UUID.match(code):
         return f"Not an UUID: {code}"
 
@@ -43,7 +38,8 @@ def _process_message(remote_addr, mailfrom, mailto, data):
     if check.subject or check.subject_fail:
         action = "ign"
         # Specify policy, the default policy does not decode encoded headers:
-        parsed = email.message_from_string(data, policy=email.policy.SMTP)
+        data_str = data.decode(errors="replace")
+        parsed = email.message_from_string(data_str, policy=email.policy.SMTP)
         subject = parsed.get("subject", "")
         if check.subject_fail and _match(subject, check.subject_fail):
             action = "fail"

hc/api/migrations/0084_ping_body_raw.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.0.2 on 2022-02-25 13:12
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0083_channel_disabled'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='ping',
+            name='body_raw',
+            field=models.BinaryField(null=True),
+        ),
+    ]

hc/api/models.py

@@ -309,7 +309,7 @@ class Check(models.Model):
 
         self.alert_after = self.going_down_after()
         self.n_pings = models.F("n_pings") + 1
-        self.has_confirmation_link = "confirm" in str(body).lower()
+        self.has_confirmation_link = "confirm" in body.decode(errors="replace").lower()
         self.save()
         self.refresh_from_db()
 
@@ -324,7 +324,7 @@ class Check(models.Model):
         ping.method = method
         # If User-Agent is longer than 200 characters, truncate it:
         ping.ua = ua[:200]
-        ping.body = body[: settings.PING_BODY_LIMIT]
+        ping.body_raw = body[: settings.PING_BODY_LIMIT]
         ping.exitstatus = exitstatus
         ping.save()
 
@@ -405,6 +405,7 @@ class Ping(models.Model):
     method = models.CharField(max_length=10, blank=True)
     ua = models.CharField(max_length=200, blank=True)
     body = models.TextField(blank=True, null=True)
+    body_raw = models.BinaryField(null=True)
     exitstatus = models.SmallIntegerField(null=True)
 
     def to_dict(self):
@@ -418,6 +419,18 @@ class Ping(models.Model):
             "ua": self.ua,
         }
 
+    def has_body(self):
+        if self.body or self.body_raw:
+            return True
+
+        return False
+
+    def get_body(self):
+        if self.body:
+            return self.body
+        if self.body_raw:
+            return bytes(self.body_raw).decode(errors="replace")
+
 
 class Channel(models.Model):
     name = models.CharField(max_length=100, blank=True)

hc/api/tests/test_get_pings.py

@@ -1,6 +1,6 @@
 from datetime import timedelta as td
 
-from hc.api.models import Check
+from hc.api.models import Check, Ping
 from hc.test import BaseTestCase
 
 
@@ -11,27 +11,26 @@ class GetPingsTestCase(BaseTestCase):
         self.a1 = Check(project=self.project, name="Alice 1")
         self.a1.timeout = td(seconds=3600)
         self.a1.grace = td(seconds=900)
-        self.a1.n_pings = 0
+        self.a1.n_pings = 1
         self.a1.status = "new"
         self.a1.tags = "a1-tag a1-additional-tag"
         self.a1.desc = "This is description"
         self.a1.save()
 
+        self.ping = Ping(owner=self.a1)
+        self.ping.n = 1
+        self.ping.remote_addr = "1.2.3.4"
+        self.ping.scheme = "https"
+        self.ping.method = "get"
+        self.ping.ua = "foo-agent"
+        self.ping.save()
+
         self.url = "/api/v1/checks/%s/pings/" % self.a1.code
 
     def get(self, api_key="X" * 32):
         return self.csrf_client.get(self.url, HTTP_X_API_KEY=api_key)
 
     def test_it_works(self):
-        self.a1.ping(
-            remote_addr="1.2.3.4",
-            scheme="https",
-            method="get",
-            ua="foo-agent",
-            body="",
-            action=None,
-        )
-
         r = self.get()
         self.assertEqual(r.status_code, 200)
         self.assertEqual(r["Access-Control-Allow-Origin"], "*")

hc/api/tests/test_notify_email.py

@@ -24,7 +24,7 @@ class NotifyEmailTestCase(BaseTestCase):
 
         self.ping = Ping(owner=self.check)
         self.ping.remote_addr = "1.2.3.4"
-        self.ping.body = "Body Line 1\nBody Line 2"
+        self.ping.body_raw = b"Body Line 1\nBody Line 2"
         self.ping.save()
 
         self.channel = Channel(project=self.project)
@@ -34,7 +34,7 @@ class NotifyEmailTestCase(BaseTestCase):
         self.channel.save()
         self.channel.checks.add(self.check)
 
-    def test_email(self):
+    def test_it_works(self):
         self.channel.notify(self.check)
 
         n = Notification.objects.get()
@@ -66,6 +66,17 @@ class NotifyEmailTestCase(BaseTestCase):
         # Check's code must not be in the plain text body
         self.assertNotIn(str(self.check.code), email.body)
 
+    def test_it_displays_body(self):
+        self.ping.body = "Body Line 1\nBody Line 2"
+        self.ping.body_raw = None
+        self.ping.save()
+
+        self.channel.notify(self.check)
+
+        email = mail.outbox[0]
+        html = email.alternatives[0][0]
+        self.assertIn("Line 1<br>Line2", html)
+
     def test_it_shows_cron_schedule(self):
         self.check.kind = "cron"
         self.check.schedule = "0 18-23,0-8 * * *"

hc/api/tests/test_ping.py

@@ -56,7 +56,7 @@ class PingTestCase(BaseTestCase):
 
         ping = Ping.objects.get()
         self.assertEqual(ping.method, "POST")
-        self.assertEqual(ping.body, "hello world")
+        self.assertEqual(bytes(ping.body_raw), b"hello world")
 
     def test_head_works(self):
         csrf_client = Client(enforce_csrf_checks=True)
@@ -205,14 +205,14 @@ class PingTestCase(BaseTestCase):
 
         ping = Ping.objects.get()
         self.assertEqual(ping.method, "POST")
-        self.assertEqual(ping.body, "hello")
+        self.assertEqual(bytes(ping.body_raw), b"hello")
 
     @override_settings(PING_BODY_LIMIT=None)
     def test_it_allows_unlimited_body(self):
         self.client.post(self.url, "A" * 20000, content_type="text/plain")
 
         ping = Ping.objects.get()
-        self.assertEqual(len(ping.body), 20000)
+        self.assertEqual(len(ping.body_raw), 20000)
 
     def test_it_handles_manual_resume_flag(self):
         self.check.status = "paused"
@@ -255,11 +255,11 @@ class PingTestCase(BaseTestCase):
         r = self.client.get(self.url + "/256")
         self.assertEqual(r.status_code, 400)
 
-    def test_it_handles_bad_unicode(self):
+    def test_it_accepts_bad_unicode(self):
         csrf_client = Client(enforce_csrf_checks=True)
         r = csrf_client.post(self.url, b"Hello \xe9 World", content_type="text/plain")
         self.assertEqual(r.status_code, 200)
 
         ping = Ping.objects.get()
         self.assertEqual(ping.method, "POST")
-        self.assertEqual(ping.body, "Hello <20> World")
+        self.assertEqual(bytes(ping.body_raw), b"Hello \xe9 World")

hc/api/tests/test_ping_by_slug.py

@@ -24,7 +24,7 @@ class PingBySlugTestCase(BaseTestCase):
 
         ping = Ping.objects.get()
         self.assertEqual(ping.method, "POST")
-        self.assertEqual(ping.body, "hello world")
+        self.assertEqual(bytes(ping.body_raw), b"hello world")
 
     def test_head_works(self):
         csrf_client = Client(enforce_csrf_checks=True)

hc/api/tests/test_smtpd.py

@@ -24,7 +24,7 @@ class SmtpdTestCase(BaseTestCase):
         ping = Ping.objects.latest("id")
         self.assertEqual(ping.scheme, "email")
         self.assertEqual(ping.ua, "Email from foo@example.org")
-        self.assertEqual(ping.body, "hello world")
+        self.assertEqual(bytes(ping.body_raw), b"hello world")
         self.assertEqual(ping.kind, None)
 
     def test_it_handles_subject_filter_match(self):
@@ -104,7 +104,8 @@ class SmtpdTestCase(BaseTestCase):
         self.check.subject_fail = "FAIL"
         self.check.save()
 
-        body = PAYLOAD_TMPL % "[SUCCESS] 1 Backup completed, [FAIL] 1 Backup did not complete"
+        subject = "[SUCCESS] 1 Backup completed, [FAIL] 1 Backup did not complete"
+        body = PAYLOAD_TMPL % subject
         _process_message("1.2.3.4", "foo@example.org", self.email, body.encode("utf8"))
 
         ping = Ping.objects.latest("id")

hc/api/views.py

@@ -47,7 +47,6 @@ def ping(request, code, check=None, action="success", exitstatus=None):
     scheme = headers.get("HTTP_X_FORWARDED_PROTO", "http")
     method = headers["REQUEST_METHOD"]
     ua = headers.get("HTTP_USER_AGENT", "")
-    body = request.body.decode(errors="replace")
 
     if exitstatus is not None and exitstatus > 0:
         action = "fail"
@@ -55,7 +54,7 @@ def ping(request, code, check=None, action="success", exitstatus=None):
     if check.methods == "POST" and method != "POST":
         action = "ign"
 
-    check.ping(remote_addr, scheme, method, ua, body, action, exitstatus)
+    check.ping(remote_addr, scheme, method, ua, request.body, action, exitstatus)
 
     response = HttpResponse("OK")
     response["Access-Control-Allow-Origin"] = "*"

hc/front/tests/test_log.py

@@ -9,20 +9,39 @@ class LogTestCase(BaseTestCase):
         super().setUp()
         self.check = Check.objects.create(project=self.project)
 
-        ping = Ping.objects.create(owner=self.check)
+        self.ping = Ping.objects.create(owner=self.check, n=1)
+        self.ping.body_raw = b"hello world"
 
         # Older MySQL versions don't store microseconds. This makes sure
         # the ping is older than any notifications we may create later:
-        ping.created = "2000-01-01T00:00:00+00:00"
-        ping.save()
+        self.ping.created = "2000-01-01T00:00:00+00:00"
+        self.ping.save()
 
         self.url = "/checks/%s/log/" % self.check.code
 
     def test_it_works(self):
+        self.client.login(username="alice@example.org", password="password")
+        r = self.client.get(self.url)
+        self.assertContains(r, "Browser's time zone", status_code=200)
+        self.assertContains(r, "hello world")
+
+    def test_it_displays_body(self):
+        self.ping.body = "hello world"
+        self.ping.body_raw = None
+        self.ping.save()
 
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(self.url)
         self.assertContains(r, "Browser's time zone", status_code=200)
+        self.assertContains(r, "hello world")
+
+    def test_it_displays_email(self):
+        self.ping.scheme = "email"
+        self.ping.save()
+
+        self.client.login(username="alice@example.org", password="password")
+        r = self.client.get(self.url)
+        self.assertContains(r, "hello world", status_code=200)
 
     def test_team_access_works(self):
 

hc/front/tests/test_ping_details.py

@@ -1,7 +1,7 @@
 from hc.api.models import Check, Ping
 from hc.test import BaseTestCase
 
-PLAINTEXT_EMAIL = """Content-Type: multipart/alternative; boundary=bbb
+PLAINTEXT_EMAIL = b"""Content-Type: multipart/alternative; boundary=bbb
 
 --bbb
 Content-Type: text/plain;charset=utf-8
@@ -12,7 +12,7 @@ aGVsbG8gd29ybGQ=
 --bbb
 """
 
-BAD_BASE64_EMAIL = """Content-Type: multipart/alternative; boundary=bbb
+BAD_BASE64_EMAIL = b"""Content-Type: multipart/alternative; boundary=bbb
 
 --bbb
 Content-Type: text/plain;charset=utf-8
@@ -23,7 +23,7 @@ Content-Transfer-Encoding: base64
 --bbb
 """
 
-HTML_EMAIL = """Content-Type: multipart/alternative; boundary=bbb
+HTML_EMAIL = b"""Content-Type: multipart/alternative; boundary=bbb
 
 --bbb
 Content-Type: text/html;charset=utf-8
@@ -42,6 +42,13 @@ class PingDetailsTestCase(BaseTestCase):
         self.url = "/checks/%s/last_ping/" % self.check.code
 
     def test_it_works(self):
+        Ping.objects.create(owner=self.check, body_raw=b"this is body")
+
+        self.client.login(username="alice@example.org", password="password")
+        r = self.client.get(self.url)
+        self.assertContains(r, "this is body", status_code=200)
+
+    def test_it_displays_body(self):
         Ping.objects.create(owner=self.check, body="this is body")
 
         self.client.login(username="alice@example.org", password="password")
@@ -69,9 +76,9 @@ class PingDetailsTestCase(BaseTestCase):
         self.assertContains(r, "/start", status_code=200)
 
     def test_it_accepts_n(self):
-        # remote_addr, scheme, method, ua, body:
-        self.check.ping("1.2.3.4", "http", "post", "tester", "foo-123", "success")
-        self.check.ping("1.2.3.4", "http", "post", "tester", "bar-456", "success")
+        # remote_addr, scheme, method, ua, body, action:
+        self.check.ping("1.2.3.4", "http", "post", "tester", b"foo-123", "success")
+        self.check.ping("1.2.3.4", "http", "post", "tester", b"bar-456", "success")
 
         self.client.login(username="alice@example.org", password="password")
 
@@ -108,7 +115,7 @@ class PingDetailsTestCase(BaseTestCase):
         self.assertContains(r, "(exit status 0)", status_code=200)
 
     def test_it_decodes_plaintext_email_body(self):
-        Ping.objects.create(owner=self.check, scheme="email", body=PLAINTEXT_EMAIL)
+        Ping.objects.create(owner=self.check, scheme="email", body_raw=PLAINTEXT_EMAIL)
 
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(self.url)
@@ -120,8 +127,19 @@ class PingDetailsTestCase(BaseTestCase):
         self.assertContains(r, "aGVsbG8gd29ybGQ=")
         self.assertContains(r, "hello world")
 
+    def test_it_decodes_plaintext_email_body_str(self):
+        body = PLAINTEXT_EMAIL.decode()
+        Ping.objects.create(owner=self.check, scheme="email", body=body)
+
+        self.client.login(username="alice@example.org", password="password")
+        r = self.client.get(self.url)
+
+        self.assertContains(r, "email-body-plain", status_code=200)
+        self.assertContains(r, "aGVsbG8gd29ybGQ=")
+        self.assertContains(r, "hello world")
+
     def test_it_handles_bad_base64_in_email_body(self):
-        Ping.objects.create(owner=self.check, scheme="email", body=BAD_BASE64_EMAIL)
+        Ping.objects.create(owner=self.check, scheme="email", body_raw=BAD_BASE64_EMAIL)
 
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(self.url)
@@ -131,7 +149,7 @@ class PingDetailsTestCase(BaseTestCase):
         self.assertNotContains(r, "email-body-html")
 
     def test_it_decodes_html_email_body(self):
-        Ping.objects.create(owner=self.check, scheme="email", body=HTML_EMAIL)
+        Ping.objects.create(owner=self.check, scheme="email", body_raw=HTML_EMAIL)
 
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(self.url)

hc/front/views.py

@@ -500,7 +500,7 @@ def ping_details(request, code, n=None):
     ctx = {"check": check, "ping": ping, "plain": None, "html": None}
 
     if ping.scheme == "email":
-        parsed = email.message_from_string(ping.body, policy=email.policy.SMTP)
+        parsed = email.message_from_string(ping.get_body(), policy=email.policy.SMTP)
         ctx["subject"] = parsed.get("subject", "")
 
         plain_mime_part = parsed.get_body(("plain",))

templates/emails/alert-body-html.html

@@ -96,9 +96,9 @@
     {% endif %}
 </table>
 
-{% if ping and ping.body %}
+{% if ping and ping.has_body %}
 <p><b>Last Ping Body</b></p>
-<pre style="font-family: monospace; line-height: 1em;">{{ ping.body|slice:":10000"|linebreaksbr }}{% if ping.body|length > 10000 %} [truncated]{% endif %}</pre>
+<pre style="font-family: monospace; line-height: 1em;">{{ ping.get_body|slice:":10000"|linebreaksbr }}{% if ping.get_body|length > 10000 %} [truncated]{% endif %}</pre>
 {% endif %}
 
 {% if projects %}

templates/front/log.html

@@ -75,8 +75,8 @@
                 {% if event.scheme == "email" %}
                     {{ event.ua }}
                     <span class="ua-body">
-                        {% if event.body %}
-                            -  {{ event.body|truncatechars:150 }}
+                        {% if event.has_body %}
+                            -  {{ event.get_body|truncatechars:150 }}
                         {% endif %}
                     </span>
                 {% else %}
@@ -89,8 +89,8 @@
                         {% if event.ua %}
                         - {{ event.ua }}
                         {% endif %}
-                        {% if event.body %}
-                        -  {{ event.body|truncatechars:150 }}
+                        {% if event.has_body %}
+                        -  {{ event.get_body|truncatechars:150 }}
                         {% endif %}
                     </span>
                 {% endif %}

templates/front/ping_details.html

@@ -73,7 +73,7 @@
         {% endif %}
     </div>
 
-    {% if ping.body %}
+    {% if ping.has_body %}
         <h4>Request Body</h4>
 
         {% if plain or html %}
@@ -94,7 +94,7 @@
         </ul>
         <div class="tab-content">
             <div id="email-body-raw" class="tab-pane active">
-                <pre>{{ ping.body }}</pre>
+                <pre>{{ ping.get_body }}</pre>
             </div>
 
             {% if plain %}
@@ -110,7 +110,7 @@
             {% endif %}
         </div>
         {% else %}
-        <pre>{{ ping.body }}</pre>
+        <pre>{{ ping.get_body }}</pre>
         {% endif %}
     {% endif %}
 </div>