encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
28.45k stars 6.84k forks source link

Browsable API shows error message on successful image upload. #1378

Closed LilyFoote closed 10 years ago

LilyFoote commented 10 years ago

html_form_image_upload_bug

The code I used to test this can be found at image-upload-test.

tomchristie commented 10 years ago

This is most likely to be a PIL/codec issue? (Eg we've had problems with sorl-thumbnail when users upload certain images with thumbnailing those images, but the problem has never been to do with sorl-thumbnail itself, just with the codecs that happened to be installed on the system not supporting the uploaded image)

Marked as easy pickings, as it's not too much work to figure out if we can mark this ticket as resolved. Set up a model with an image field, and create a basic view for editing it. Can you upload any images or not. If so then I believe we can close this issue. If not then this needs more attention.

LilyFoote commented 10 years ago

Just to clarify: The image upload itself succeeds. The problem is the incorrectly displayed error message.

tomchristie commented 10 years ago

Ah gotcha. Occurring for all cases, or just some images?

LilyFoote commented 10 years ago

It occurs for all the jpg and png images I've tested with.

kevin-brown commented 10 years ago

This sounds like an issue with the Django form and how the data is populated, not with the core of DRF.

koepked commented 10 years ago

I have this too and did some poking around. I don't think it's in Django form, I think it's in rest_framework.fields.ImageField.from_native():

try:
            # load() could spot a truncated JPEG, but it loads the entire
            # image in memory, which is a DoS vector. See #3848 and #18520.
            # verify() must be called immediately after the constructor.
            Image.open(file).verify()
        except ImportError:
            # Under PyPy, it is possible to import PIL. However, the underlying
            # _imaging C module isn't available, so an ImportError will be
            # raised. Catch and re-raise.
            raise
        except Exception:  # Python Imaging Library doesn't recognize it as an image
            print 'Here it is...'
            raise ValidationError(self.error_messages['invalid_image'])

If I upload the photo using django-admin instead, the form validates fine. I don't know if it matters or not, but from_native() gets called twice on upload and the exception occurs on the 2nd.

kevin-brown commented 10 years ago

The serializer is used to generate the Django form automatically. During the process I believe from_native is called, which would explain the errors that occur in one place and not the other.

koepked commented 10 years ago

On the second call, file in Image.open(file).verify() appears to be empty.

tomchristie commented 10 years ago

Should be resolved as part of 3.0

carltongibson commented 10 years ago

Can we close this? #1681 — which addresses this was confirmed as fixed in 3.0.