From 58a118c49499ed814d3aedf3a761bc83624b460d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Fri, 17 Jan 2020 12:44:39 +0200 Subject: [PATCH] Make Ping.body size limit configurable. Fixes #301 --- CHANGELOG.md | 1 + README.md | 5 ++ hc/api/migrations/0068_auto_20200117_1023.py | 18 +++++++ hc/api/models.py | 4 +- hc/api/tests/test_ping.py | 55 +++++++++++--------- hc/settings.py | 1 + 6 files changed, 58 insertions(+), 26 deletions(-) create mode 100644 hc/api/migrations/0068_auto_20200117_1023.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 499fe969..49185ae2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. - Show a red "!" in project's top navigation if any integration is not working - createsuperuser management command requires an unique email address (#318) - For superusers, show "Site Administration" in top navigation, note in README (#317) +- Make Ping.body size limit configurable (#301) ### Bug Fixes - Increase the allowable length of Matrix room alias to 100 (#320) diff --git a/README.md b/README.md index 26cb7534..29280475 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,7 @@ Configurations settings loaded from environment variables: | MASTER_BADGE_LABEL | `"Mychecks"` | PING_ENDPOINT | `"http://localhost:8000/ping/"` | PING_EMAIL_DOMAIN | `"localhost"` +| PING_BODY_LIMIT | 10000 | In bytes. Set to `None` to always log full request body | DISCORD_CLIENT_ID | `None` | DISCORD_CLIENT_SECRET | `None` | SLACK_CLIENT_ID | `None` @@ -168,6 +169,10 @@ Set it to `False` if you are setting up a private healthchecks instance where you trust your users and want to avoid the extra verification step. +`PING_BODY_LIMIT` sets the size limit in bytes for logged ping request bodies. +The default value is 10000 (10 kilobytes). You can remove the limit altogether by +setting this value to `None`. + ## Database Configuration Database configuration is loaded from environment variables. If you diff --git a/hc/api/migrations/0068_auto_20200117_1023.py b/hc/api/migrations/0068_auto_20200117_1023.py new file mode 100644 index 00000000..8fccff75 --- /dev/null +++ b/hc/api/migrations/0068_auto_20200117_1023.py @@ -0,0 +1,18 @@ +# Generated by Django 3.0.1 on 2020-01-17 10:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0067_last_error_values'), + ] + + operations = [ + migrations.AlterField( + model_name='ping', + name='body', + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/hc/api/models.py b/hc/api/models.py index 83917d04..4682690f 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -275,7 +275,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[:10000] + ping.body = body[: settings.PING_BODY_LIMIT] ping.save() def downtimes(self, months=3): @@ -327,7 +327,7 @@ class Ping(models.Model): remote_addr = models.GenericIPAddressField(blank=True, null=True) method = models.CharField(max_length=10, blank=True) ua = models.CharField(max_length=200, blank=True) - body = models.CharField(max_length=10000, blank=True, null=True) + body = models.TextField(blank=True, null=True) class Channel(models.Model): diff --git a/hc/api/tests/test_ping.py b/hc/api/tests/test_ping.py index 72aa5022..08b30715 100644 --- a/hc/api/tests/test_ping.py +++ b/hc/api/tests/test_ping.py @@ -1,6 +1,7 @@ from datetime import timedelta as td from django.test import Client +from django.test.utils import override_settings from django.utils.timezone import now from hc.api.models import Check, Flip, Ping from hc.test import BaseTestCase @@ -10,9 +11,10 @@ class PingTestCase(BaseTestCase): def setUp(self): super().setUp() self.check = Check.objects.create(project=self.project) + self.url = "/ping/%s/" % self.check.code def test_it_works(self): - r = self.client.get("/ping/%s/" % self.check.code) + r = self.client.get(self.url) self.assertEqual(r.status_code, 200) self.check.refresh_from_db() @@ -28,7 +30,7 @@ class PingTestCase(BaseTestCase): self.check.status = "paused" self.check.save() - r = self.client.get("/ping/%s/" % self.check.code) + r = self.client.get(self.url) self.assertEqual(r.status_code, 200) self.check.refresh_from_db() @@ -38,7 +40,7 @@ class PingTestCase(BaseTestCase): self.check.last_start = now() self.check.save() - r = self.client.get("/ping/%s/" % self.check.code) + r = self.client.get(self.url) self.assertEqual(r.status_code, 200) self.check.refresh_from_db() @@ -46,9 +48,7 @@ class PingTestCase(BaseTestCase): def test_post_works(self): csrf_client = Client(enforce_csrf_checks=True) - r = csrf_client.post( - "/ping/%s/" % self.check.code, "hello world", content_type="text/plain" - ) + r = csrf_client.post(self.url, "hello world", content_type="text/plain") self.assertEqual(r.status_code, 200) ping = Ping.objects.latest("id") @@ -57,7 +57,7 @@ class PingTestCase(BaseTestCase): def test_head_works(self): csrf_client = Client(enforce_csrf_checks=True) - r = csrf_client.head("/ping/%s/" % self.check.code) + r = csrf_client.head(self.url) self.assertEqual(r.status_code, 200) self.assertEqual(Ping.objects.count(), 1) @@ -81,7 +81,7 @@ class PingTestCase(BaseTestCase): "Chrome/44.0.2403.89 Safari/537.36" ) - r = self.client.get("/ping/%s/" % self.check.code, HTTP_USER_AGENT=ua) + r = self.client.get(self.url, HTTP_USER_AGENT=ua) self.assertEqual(r.status_code, 200) ping = Ping.objects.latest("id") @@ -90,7 +90,7 @@ class PingTestCase(BaseTestCase): def test_it_truncates_long_ua(self): ua = "01234567890" * 30 - r = self.client.get("/ping/%s/" % self.check.code, HTTP_USER_AGENT=ua) + r = self.client.get(self.url, HTTP_USER_AGENT=ua) self.assertEqual(r.status_code, 200) ping = Ping.objects.latest("id") @@ -99,38 +99,30 @@ class PingTestCase(BaseTestCase): def test_it_reads_forwarded_ip(self): ip = "1.1.1.1" - r = self.client.get("/ping/%s/" % self.check.code, HTTP_X_FORWARDED_FOR=ip) + r = self.client.get(self.url, HTTP_X_FORWARDED_FOR=ip) ping = Ping.objects.latest("id") self.assertEqual(r.status_code, 200) self.assertEqual(ping.remote_addr, "1.1.1.1") ip = "1.1.1.1, 2.2.2.2" - r = self.client.get( - "/ping/%s/" % self.check.code, - HTTP_X_FORWARDED_FOR=ip, - REMOTE_ADDR="3.3.3.3", - ) + r = self.client.get(self.url, HTTP_X_FORWARDED_FOR=ip, REMOTE_ADDR="3.3.3.3",) ping = Ping.objects.latest("id") self.assertEqual(r.status_code, 200) self.assertEqual(ping.remote_addr, "1.1.1.1") def test_it_reads_forwarded_protocol(self): - r = self.client.get( - "/ping/%s/" % self.check.code, HTTP_X_FORWARDED_PROTO="https" - ) + r = self.client.get(self.url, HTTP_X_FORWARDED_PROTO="https") ping = Ping.objects.latest("id") self.assertEqual(r.status_code, 200) self.assertEqual(ping.scheme, "https") def test_it_never_caches(self): - r = self.client.get("/ping/%s/" % self.check.code) + r = self.client.get(self.url) assert "no-cache" in r.get("Cache-Control") def test_it_updates_confirmation_flag(self): payload = "Please Confirm ..." - r = self.client.post( - "/ping/%s/" % self.check.code, data=payload, content_type="text/plain" - ) + r = self.client.post(self.url, data=payload, content_type="text/plain") self.assertEqual(r.status_code, 200) self.check.refresh_from_db() @@ -181,7 +173,7 @@ class PingTestCase(BaseTestCase): self.check.last_start = now() - td(seconds=10) self.check.save() - r = self.client.get("/ping/%s/" % self.check.code) + r = self.client.get(self.url) self.assertEqual(r.status_code, 200) self.check.refresh_from_db() @@ -191,7 +183,7 @@ class PingTestCase(BaseTestCase): self.check.methods = "POST" self.check.save() - r = self.client.get("/ping/%s/" % self.check.code) + r = self.client.get(self.url) self.assertEqual(r.status_code, 200) self.check.refresh_from_db() @@ -201,3 +193,18 @@ class PingTestCase(BaseTestCase): ping = Ping.objects.latest("id") self.assertEqual(ping.scheme, "http") self.assertEqual(ping.kind, "ign") + + @override_settings(PING_BODY_LIMIT=5) + def test_it_chops_long_body(self): + self.client.post(self.url, "hello world", content_type="text/plain") + + ping = Ping.objects.latest("id") + self.assertEqual(ping.method, "POST") + self.assertEqual(ping.body, "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.latest("id") + self.assertEqual(len(ping.body), 20000) diff --git a/hc/settings.py b/hc/settings.py index cba89b76..b725da35 100644 --- a/hc/settings.py +++ b/hc/settings.py @@ -153,6 +153,7 @@ SITE_NAME = os.getenv("SITE_NAME", "Mychecks") MASTER_BADGE_LABEL = os.getenv("MASTER_BADGE_LABEL", SITE_NAME) PING_ENDPOINT = os.getenv("PING_ENDPOINT", SITE_ROOT + "/ping/") PING_EMAIL_DOMAIN = os.getenv("PING_EMAIL_DOMAIN", "localhost") +PING_BODY_LIMIT = envint("PING_BODY_LIMIT", "10000") STATIC_URL = "/static/" STATICFILES_DIRS = [os.path.join(BASE_DIR, "static")] STATIC_ROOT = os.path.join(BASE_DIR, "static-collected")