Merge pull request #3680 from chirathr/webhook_notifications_url_size

Webhook notification URL size validation check

Author: ericholscher

readthedocs/projects/forms.py

@@ -530,25 +530,24 @@ def save(self):
         return self.project
 
 
-class WebHookForm(forms.Form):
+class WebHookForm(forms.ModelForm):
 
     """Project webhook form."""
 
-    url = forms.URLField()
-
     def __init__(self, *args, **kwargs):
         self.project = kwargs.pop('project', None)
         super(WebHookForm, self).__init__(*args, **kwargs)
 
-    def clean_url(self):
+    def save(self, commit=True):
         self.webhook = WebHook.objects.get_or_create(
             url=self.cleaned_data['url'], project=self.project)[0]
-        return self.webhook
-
-    def save(self):
         self.project.webhook_notifications.add(self.webhook)
         return self.project
 
+    class Meta:
+        model = WebHook
+        fields = ['url']
+
 
 class TranslationBaseForm(forms.Form):
 

readthedocs/projects/migrations/0032_increase_webhook_maxsize.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-11-06 23:12
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('projects', '0031_add_modified_date_importedfile'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='webhook',
+            name='url',
+            field=models.URLField(blank=True, help_text='URL to send the webhook to', max_length=600),
+        ),
+    ]

readthedocs/projects/models.py

@@ -972,7 +972,7 @@ def __str__(self):
 
 @python_2_unicode_compatible
 class WebHook(Notification):
-    url = models.URLField(blank=True,
+    url = models.URLField(max_length=600, blank=True,
                           help_text=_('URL to send the webhook to'))
 
     def __str__(self):

readthedocs/rtd_tests/tests/test_build_notifications.py

@@ -12,6 +12,7 @@
 from readthedocs.builds.models import Build, Version
 from readthedocs.projects.models import Project, EmailHook, WebHook
 from readthedocs.projects.tasks import send_notifications
+from readthedocs.projects.forms import WebHookForm
 
 
 class BuildNotificationsTests(TestCase):
@@ -46,3 +47,34 @@ def test_send_email_and_webhook__notification(self):
             send_notifications(self.version.pk, self.build.pk)
             mock.assert_called_once()
         self.assertEqual(len(mail.outbox), 1)
+
+
+class TestForms(TestCase):
+
+    def setUp(self):
+        self.project = fixture.get(Project)
+        self.version = fixture.get(Version, project=self.project)
+        self.build = fixture.get(Build, version=self.version)
+
+    def test_webhook_form_url_length(self):
+        form = WebHookForm(
+            {
+                'url': 'https://foobar.com',
+            },
+            project=self.project,
+        )
+        self.assertTrue(form.is_valid())
+
+        form = WebHookForm(
+            {
+                'url': 'foo' * 500,
+            },
+            project=self.project,
+        )
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors,
+            {'url':
+                ['Enter a valid URL.',
+                    'Ensure this value has at most 600 characters (it has 1507).']
+             })

requirements/testing.txt

@@ -21,3 +21,4 @@ pytest-mock==1.10.0
 
 # local debugging tools
 datadiff
+ipdb