add blacklist slug table, catch attempts in form and model validation (bug 625842)

apps/addons/fixtures/addons/blacklisted.json

@@ -0,0 +1,20 @@
+[
+ {
+    "pk": 1,
+    "model": "addons.blacklistedslug",
+    "fields": {
+        "modified": "2011-01-31 11:29:37",
+        "name": "submit",
+        "created": "2011-01-31 11:29:37"
+    }
+ },
+ {
+    "pk": 2,
+    "model": "addons.blacklistedslug",
+    "fields": {
+        "modified": "2011-01-31 11:29:37",
+        "name": "validate",
+        "created": "2011-01-31 11:29:37"
+    }
+ }
+]

apps/addons/forms.py

@@ -12,8 +12,8 @@ from tower import ugettext as _, ungettext as ngettext
 import amo
 import captcha.fields
 from amo.utils import slug_validator, slugify, sorted_groupby, remove_icons
-from addons.models import (Addon, AddonCategory, Category, MiniAddon,
-                           ReverseNameLookup)
+from addons.models import (Addon, AddonCategory, BlacklistedSlug,
+                           Category, MiniAddon, ReverseNameLookup)
 from addons.widgets import IconWidgetRenderer, CategoriesSelectMultiple
 from applications.models import Application
 from devhub import tasks
@@ -121,9 +121,13 @@ class AddonFormBasic(AddonFormBase):
         target = self.cleaned_data['slug']
         slug_validator(target, lower=False)
 
-        if self.cleaned_data['slug'] != self.instance.slug:
+        if target != self.instance.slug:
             if Addon.objects.filter(slug=target).exists():
                 raise forms.ValidationError(_('This slug is already in use.'))
+
+            if BlacklistedSlug.blocked(target):
+                raise forms.ValidationError(_('The slug cannot be: %s.'
+                                              % target))
         return target
 
 

apps/addons/models.py

@@ -296,7 +296,7 @@ class Addon(amo.models.ModelBase):
             else:
                 name = self.name
             self.slug = slugify(name)[:27]
-        if self.slug.isdigit():
+        if BlacklistedSlug.blocked(self.slug):
             self.slug += "~"
         qs = Addon.objects.values_list('slug', 'id')
         match = qs.filter(slug=self.slug)
@@ -1280,3 +1280,17 @@ class Charity(amo.models.ModelBase):
         if self.pk == amo.FOUNDATION_ORG:
             return self.url
         return get_outgoing_url(unicode(self.url))
+
+
+class BlacklistedSlug(amo.models.ModelBase):
+    name = models.CharField(max_length=255, unique=True, default='')
+
+    class Meta:
+        db_table = 'addons_blacklistedslug'
+
+    def __unicode__(self):
+        return self.name
+
+    @classmethod
+    def blocked(cls, slug):
+        return slug.isdigit() or cls.objects.filter(name=slug).exists()

apps/addons/tests/test_forms.py

@@ -16,7 +16,8 @@ from amo.tests.test_helpers import get_image_path
 
 
 class FormsTest(test_utils.TestCase):
-    fixtures = ('base/addon_3615', 'base/addon_3615_categories')
+    fixtures = ('base/addon_3615', 'base/addon_3615_categories',
+                'addons/blacklisted')
 
     def setUp(self):
         self._redis = mock_redis()
@@ -59,6 +60,20 @@ class FormsTest(test_utils.TestCase):
         form = forms.AddonFormDetails(request={})
         eq_(form.fields['default_locale'].choices[0][0], 'af')
 
+    def test_slug_blacklist(self):
+        delicious = Addon.objects.get()
+        form = forms.AddonFormBasic({'slug': 'submit'}, request=None,
+                                    instance=delicious)
+        assert not form.is_valid()
+        eq_(form.errors['slug'], [u'The slug cannot be: submit.'])
+
+    def test_slug_isdigit(self):
+        delicious = Addon.objects.get()
+        form = forms.AddonFormBasic({'slug': '123'}, request=None,
+                                    instance=delicious)
+        assert not form.is_valid()
+        eq_(form.errors['slug'], [u'The slug cannot be: 123.'])
+
 
 class TestTagsForm(test_utils.TestCase):
     fixtures = ['base/addon_3615', 'base/platforms', 'base/users']

apps/addons/tests/test_models.py

@@ -91,7 +91,8 @@ class TestAddonModels(test_utils.TestCase):
                 'base/addon_4664_twitterbar',
                 'base/thunderbird',
                 'addons/featured',
-                'addons/invalid_latest_version']
+                'addons/invalid_latest_version',
+                'addons/blacklisted']
 
     def setUp(self):
         TranslationSequence.objects.create(id=99243)
@@ -506,6 +507,16 @@ class TestAddonModels(test_utils.TestCase):
         a.save()
         eq_(a.slug, '44~')
 
+    def test_slug_isblacklisted(self):
+        # When an addon is uploaded, it doesn't use the form validation,
+        # so we'll just mangle the slug if its blacklisted.
+        a = Addon.objects.create(type=1, name='xx', slug='validate')
+        eq_(a.slug, 'validate~')
+
+        a.slug = 'validate'
+        a.save()
+        eq_(a.slug, 'validate~')
+
     def delete(self):
         addon = Addon.objects.get(id=3615)
         eq_(len(mail.outbox), 0)

migrations/140-add-blacklistedslugs.sql

@@ -0,0 +1,9 @@
+CREATE TABLE `addons_blacklistedslug` (
+    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
+    `created` datetime NOT NULL,
+    `modified` datetime NOT NULL,
+    `name` varchar(255) NOT NULL UNIQUE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8
+;
+INSERT INTO `addons_blacklistedslug` VALUE(1, NOW(), NOW(), "validate");
+INSERT INTO `addons_blacklistedslug` VALUE(2, NOW(), NOW(), "submit");