launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02529
[Merge] lp:~sinzui/launchpad/field-errors-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/field-errors-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#50616 ValueError while validating image file using Python Imaging Lib
https://bugs.launchpad.net/bugs/50616
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/field-errors-0/+merge/48800
Catch the ValueError while validating image file
Launchpad bug: https://bugs.launchpad.net/bugs/50616
Pre-implementation: no one
Test command: ./bin/test -vv \
-t test_fields -t image-widget
Usually when the PIL can't identify an image type, it raises a IOError, but
it might raise a "ValueError: cannot read this XPM file" when uploading a
broken XPM file. The existing LaunchpadValidationError message is correct,
but it is not being raised because ValueError was not expected.
--------------------------------------------------------------------
RULES
* Reproduce an XPM fragment that raises the ValueError.
* Add ValueError to the except clause.
QA
* Upload the abcardwindow.xpm file attached to the bug.
* Verify the form explain that the image was not recognised.
LINT
lib/lp/app/widgets/doc/image-widget.txt
lib/lp/services/fields/__init__.py
lib/lp/services/fields/tests/test_fields.py
./lib/lp/app/widgets/doc/image-widget.txt
1: narrative uses a moin header.
36: narrative uses a moin header.
77: source exceeds 78 characters.
233: narrative exceeds 78 characters.
250: narrative uses a moin header.
314: narrative exceeds 78 characters.
317: narrative exceeds 78 characters.
358: narrative uses a moin header.
421: narrative exceeds 78 characters.
476: narrative uses a moin header.
./lib/lp/services/fields/__init__.py
350: E302 expected 2 blank lines, found 1
^ I will fix this before I land the branch. I do not want to clutter the
diff.
IMPLEMENTATION
Converted the existing BaseImageUpload IOError doctest into a unittest.
lib/lp/app/widgets/doc/image-widget.txt
lib/lp/services/fields/tests/test_fields.py
Reproduced the ValueError using a corrupt XPM and added the ValueError to
the exception.
lib/lp/services/fields/tests/test_fields.py
lib/lp/services/fields/__init__.py
--
https://code.launchpad.net/~sinzui/launchpad/field-errors-0/+merge/48800
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/field-errors-0 into lp:launchpad.
=== modified file 'lib/lp/app/widgets/doc/image-widget.txt'
--- lib/lp/app/widgets/doc/image-widget.txt 2011-02-01 21:46:58 +0000
+++ lib/lp/app/widgets/doc/image-widget.txt 2011-02-07 15:53:42 +0000
@@ -459,24 +459,6 @@
LaunchpadValidationError(u'\nThis image is not exactly
193x191\npixels in size.'))
-We also won't accept anything that is not an image; that is, we only accept
-what can be parsed by the Python Imaging Library module.
-
- >>> mugshot = StringIO('foo bar bz')
- >>> mugshot.filename = 'foo.jpg'
- >>> form = {'field.mugshot.action': 'change',
- ... 'field.mugshot.image': mugshot}
- >>> widget = ImageChangeWidget(
- ... person_mugshot, LaunchpadTestRequest(form=form), edit_style)
- >>> widget.getInputValue()
- Traceback (most recent call last):
- ...
- WidgetInputError: ('field.mugshot', u'Mugshot',
- LaunchpadValidationError(u'\nThe file uploaded was not
- recognized as an image;
- please\ncheck it and
- retry.'))
-
Finally, if the user specifies the 'change' action he must also provide a file
to be uploaded.
=== modified file 'lib/lp/services/fields/__init__.py'
--- lib/lp/services/fields/__init__.py 2011-01-24 20:53:10 +0000
+++ lib/lp/services/fields/__init__.py 2011-02-07 15:53:42 +0000
@@ -717,7 +717,7 @@
This image exceeds the maximum allowed size in bytes.""")))
try:
pil_image = PIL.Image.open(StringIO(image))
- except IOError:
+ except (IOError, ValueError):
raise LaunchpadValidationError(_(dedent("""
The file uploaded was not recognized as an image; please
check it and retry.""")))
=== modified file 'lib/lp/services/fields/tests/test_fields.py'
--- lib/lp/services/fields/tests/test_fields.py 2011-01-06 19:29:11 +0000
+++ lib/lp/services/fields/tests/test_fields.py 2011-02-07 15:53:42 +0000
@@ -6,16 +6,25 @@
__metaclass__ = type
import datetime
+from StringIO import StringIO
import time
+<<<<<<< TREE
from zope.interface import Interface
from zope.component import getUtility
from canonical.launchpad.interfaces.lpstorm import IStore
+=======
+
+>>>>>>> MERGE-SOURCE
from canonical.launchpad.validators import LaunchpadValidationError
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.services.fields import (
+<<<<<<< TREE
BlacklistableContentNameField,
+=======
+ BaseImageUpload,
+>>>>>>> MERGE-SOURCE
FormattableDate,
StrippableText,
)
@@ -79,6 +88,7 @@
self.assertIs(None, target.test)
+<<<<<<< TREE
class TestBlacklistableContentNameField(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -128,3 +138,33 @@
date_value = u'fnord'
login_person(self.team.teamowner)
self.assertEqual(None, field.validate(date_value))
+=======
+class TestBaseImageUpload(TestCase):
+ """Test for the BaseImageUpload field."""
+
+ class ExampleImageUpload(BaseImageUpload):
+ dimensions = (192, 192)
+ max_size = 100*1024
+
+ def test_validation_corrupt_image(self):
+ # ValueErrors raised by PIL become LaunchpadValidationErrors.
+ field = self.ExampleImageUpload(default_image_resource='dummy')
+ image = StringIO(
+ '/* XPM */\n'
+ 'static char *pixmap[] = {\n'
+ '"32 32 253 2",\n'
+ ' "00 c #01CAA3",\n'
+ ' ".. s None c None",\n'
+ '};')
+ image.filename = 'foo.xpm'
+ self.assertRaises(
+ LaunchpadValidationError, field.validate, image)
+
+ def test_validation_non_image(self):
+ # IOError raised by PIL become LaunchpadValidationErrors.
+ field = self.ExampleImageUpload(default_image_resource='dummy')
+ image = StringIO('foo bar bz')
+ image.filename = 'foo.jpg'
+ self.assertRaises(
+ LaunchpadValidationError, field.validate, image)
+>>>>>>> MERGE-SOURCE
