diff --git a/src/olympia/amo/tests/images/non-image.png b/src/olympia/amo/tests/images/non-image.png new file mode 100644 index 0000000000..c40efdc145 --- /dev/null +++ b/src/olympia/amo/tests/images/non-image.png @@ -0,0 +1 @@ +this is not an image. diff --git a/src/olympia/bandwagon/forms.py b/src/olympia/bandwagon/forms.py index 07d5d6da66..e8e382df4c 100644 --- a/src/olympia/bandwagon/forms.py +++ b/src/olympia/bandwagon/forms.py @@ -10,7 +10,8 @@ from django_statsd.clients import statsd import olympia.core.logger from olympia import amo -from olympia.amo.utils import clean_nl, has_links, slug_validator, slugify +from olympia.amo.utils import ( + clean_nl, has_links, ImageCheck, slug_validator, slugify) from olympia.lib import happyforms from olympia.translations.widgets import ( TranslationTextarea, TranslationTextInput) @@ -195,10 +196,15 @@ class CollectionForm(happyforms.ModelForm): icon = self.cleaned_data['icon'] if not icon: return - if icon.content_type not in ('image/png', 'image/jpeg'): + icon_check = ImageCheck(icon) + if (icon.content_type not in ('image/png', 'image/jpeg') or + not icon_check.is_image()): raise forms.ValidationError( ugettext('Icons must be either PNG or JPG.')) + if icon_check.is_animated(): + raise forms.ValidationError(ugettext('Icons cannot be animated.')) + if icon.size > settings.MAX_ICON_UPLOAD_SIZE: size_in_mb = settings.MAX_ICON_UPLOAD_SIZE / 1024 / 1024 - 1 raise forms.ValidationError( @@ -206,32 +212,33 @@ class CollectionForm(happyforms.ModelForm): return icon def save(self, default_locale=None): - c = super(CollectionForm, self).save(commit=False) - c.author = self.initial['author'] - c.application = self.initial['application'] + collection = super(CollectionForm, self).save(commit=False) + collection.author = self.initial['author'] + collection.application = self.initial['application'] icon = self.cleaned_data.get('icon') if default_locale: - c.default_locale = default_locale + collection.default_locale = default_locale if icon: - c.icontype = 'image/png' + collection.icontype = 'image/png' - c.save() + collection.save() if icon: - dirname = c.get_img_dir() + dirname = collection.get_img_dir() - destination = os.path.join(dirname, '%d.png' % c.id) - tmp_destination = os.path.join(dirname, - '%d.png__unconverted' % c.id) + destination = os.path.join(dirname, '%d.png' % collection.id) + tmp_destination = os.path.join( + dirname, '%d.png__unconverted' % collection.id) + icon.seek(0) with storage.open(tmp_destination, 'w') as fh: for chunk in icon.chunks(): fh.write(chunk) tasks.resize_icon.delay(tmp_destination, destination, - set_modified_on=[c]) + set_modified_on=[collection]) - return c + return collection class Meta: model = Collection diff --git a/src/olympia/bandwagon/tests/test_views.py b/src/olympia/bandwagon/tests/test_views.py index 55a9021624..1f78ec39ab 100644 --- a/src/olympia/bandwagon/tests/test_views.py +++ b/src/olympia/bandwagon/tests/test_views.py @@ -1183,6 +1183,49 @@ class TestCollectionForm(TestCase): form.save() assert update_mock.called + def test_icon_invalid_though_content_type_is_correct(self): + collection = Collection.objects.get(pk=57181) + # This file is not an image at all, but we'll try to upload it with an + # image mime type. It should not work. + fake_image = get_uploaded_file('non-image.png') + assert fake_image.content_type == 'image/png' + form = forms.CollectionForm( + {'listed': collection.listed, + 'slug': collection.slug, + 'name': collection.name}, + instance=collection, + files={'icon': fake_image}, + initial={'author': collection.author, + 'application': collection.application}) + assert not form.is_valid() + assert form.errors == {'icon': [u'Icons must be either PNG or JPG.']} + + def test_icon_invalid_gif(self): + collection = Collection.objects.get(pk=57181) + form = forms.CollectionForm( + {'listed': collection.listed, + 'slug': collection.slug, + 'name': collection.name}, + instance=collection, + files={'icon': get_uploaded_file('animated.gif')}, + initial={'author': collection.author, + 'application': collection.application}) + assert not form.is_valid() + assert form.errors == {'icon': [u'Icons must be either PNG or JPG.']} + + def test_icon_invalid_animated(self): + collection = Collection.objects.get(pk=57181) + form = forms.CollectionForm( + {'listed': collection.listed, + 'slug': collection.slug, + 'name': collection.name}, + instance=collection, + files={'icon': get_uploaded_file('animated.png')}, + initial={'author': collection.author, + 'application': collection.application}) + assert not form.is_valid() + assert form.errors == {'icon': [u'Icons cannot be animated.']} + def test_denied_name(self): form = forms.CollectionForm() form.cleaned_data = {'name': 'IE6Fan'} diff --git a/src/olympia/devhub/views.py b/src/olympia/devhub/views.py index af2694e07e..94b64562b9 100644 --- a/src/olympia/devhub/views.py +++ b/src/olympia/devhub/views.py @@ -1007,8 +1007,8 @@ def ajax_upload_image(request, upload_type, addon_id=None): is_persona = upload_type.startswith('persona_') check = amo_utils.ImageCheck(upload_preview) - if (not check.is_image() or - upload_preview.content_type not in amo.IMG_TYPES): + if (upload_preview.content_type not in amo.IMG_TYPES or + not check.is_image()): if is_icon: errors.append(ugettext('Icons must be either PNG or JPG.')) else: diff --git a/src/olympia/users/forms.py b/src/olympia/users/forms.py index 5567995c75..e34084ccbc 100644 --- a/src/olympia/users/forms.py +++ b/src/olympia/users/forms.py @@ -12,7 +12,8 @@ from olympia import amo from olympia.accounts.views import fxa_error_message from olympia.activity.models import ActivityLog from olympia.amo.fields import HttpHttpsOnlyURLField -from olympia.amo.utils import clean_nl, has_links, slug_validator +from olympia.amo.utils import ( + clean_nl, has_links, ImageCheck, slug_validator) from olympia.lib import happyforms from olympia.users import notifications @@ -176,10 +177,15 @@ class UserEditForm(happyforms.ModelForm): if not photo: return - if photo.content_type not in ('image/png', 'image/jpeg'): + image_check = ImageCheck(photo) + if (photo.content_type not in ('image/png', 'image/jpeg') or + not image_check.is_image()): raise forms.ValidationError( ugettext('Images must be either PNG or JPG.')) + if image_check.is_animated(): + raise forms.ValidationError(ugettext('Images cannot be animated.')) + if photo.size > settings.MAX_PHOTO_UPLOAD_SIZE: msg = ugettext('Please use images smaller than %dMB.') size_in_mb = settings.MAX_PHOTO_UPLOAD_SIZE / 1024 / 1024 - 1 diff --git a/src/olympia/users/tests/test_forms.py b/src/olympia/users/tests/test_forms.py index 6c97afbacb..d1498d7b89 100644 --- a/src/olympia/users/tests/test_forms.py +++ b/src/olympia/users/tests/test_forms.py @@ -115,18 +115,53 @@ class TestUserEditForm(UserFormBase): @patch('olympia.amo.models.ModelBase.update') def test_photo_modified(self, update_mock): - dummy = Mock() - dummy.user = self.user - + request = Mock() + request.user = self.user data = {'username': self.user_profile.username, 'email': self.user_profile.email} files = {'photo': get_uploaded_file('transparent.png')} form = UserEditForm(data, files=files, instance=self.user_profile, - request=dummy) + request=request) assert form.is_valid() form.save() assert update_mock.called + def test_photo_invalid_though_content_type_is_correct(self): + request = Mock() + request.user = self.user + data = {'username': self.user_profile.username, + 'email': self.user_profile.email} + files = {'photo': get_uploaded_file('non-image.png')} + form = UserEditForm(data, files=files, instance=self.user_profile, + request=request) + + assert not form.is_valid() + assert form.errors == {'photo': [u'Images must be either PNG or JPG.']} + + def test_photo_invalid_gif(self): + request = Mock() + request.user = self.user + data = {'username': self.user_profile.username, + 'email': self.user_profile.email} + files = {'photo': get_uploaded_file('animated.gif')} + form = UserEditForm(data, files=files, instance=self.user_profile, + request=request) + + assert not form.is_valid() + assert form.errors == {'photo': [u'Images must be either PNG or JPG.']} + + def test_photo_invalid_animated(self): + request = Mock() + request.user = self.user + data = {'username': self.user_profile.username, + 'email': self.user_profile.email} + files = {'photo': get_uploaded_file('animated.png')} + form = UserEditForm(data, files=files, instance=self.user_profile, + request=request) + + assert not form.is_valid() + assert form.errors == {'photo': [u'Images cannot be animated.']} + def test_cannot_change_email(self): self.user.update(fxa_id='1a2b3c', email='me@example.com') form = UserEditForm(