Django-rest-framework: FileUploadParser should use more descriptive error when filename not included.

[yuyou]

Hi

The FileUploadParser code (line 181) in rest_framework.parsers.py seems to work differently as it should be.

The line does not work in my Python 2.7.8 (OSX El Capitan):
if file_obj:

I modified and works like this:
if file_obj is not None:

Could someone confirm this buggy behavior under other situations?

Thanks.

Regards,

Yu

[xordoquy]

Do you have a test project we could test against ?

[tomchristie]

How are you replicating an issue and exactly what behaviour do you see?

[BraisGabin]

I just create an test project that reproduce this issue: https://github.com/BraisGabin/drf_3_3_test
The code works with drf 3.2.5 but it doesn't with 3.3.1.

To reproduce the error just launch the tests with the two different versions ./manage.py test.

[xordoquy]

Do you have the command you use to make the post as well ?

[yuyou]

Thanks to BraisGabin for creating a test project. I am not sure what "command" do you mean? Django command or terminal command... but I cloned the test project and run a CURL command to reproduce the error.

curl -XPOST -H "Content-type: image/jpg" --data-binary @./app/test_assets/image.jpg -v "http://127.0.0.1:8000/media/"
* Hostname was NOT found in DNS cache
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 8000 (#0)
> POST /media/ HTTP/1.1
> User-Agent: curl/7.35.0
> Host: 127.0.0.1:8000
> Accept: */*
> Content-type: image/jpg
> Content-Length: 14109
> Expect: 100-continue
> 
* Done waiting for 100-continue
* HTTP 1.0, assume close after body
< HTTP/1.0 400 BAD REQUEST
< Date: Mon, 09 Nov 2015 09:46:15 GMT
< Server: WSGIServer/0.1 Python/2.7.6
< Vary: Accept, Cookie
< X-Frame-Options: SAMEORIGIN
< Content-Type: application/json
< Allow: POST, OPTIONS
< 
* Closing connection 0
{"detail":"FileUpload parse error - none of upload handlers can handle the stream"}

[xordoquy]

I suspect the content type here isn't correct as you're sending a image/jpg instead of a multipart/form

Forget about this comment I was thinking about them multipart content.

[yuyou]

According to the spec., the parser is supposed to support direct binary upload. And it works if that line in question (line 181) is fixed from "if file_obj:" to "if file_obj is not None:".

[xordoquy]

@yuyou does it work with the change you mention ? I mean do you have a non empty file in the end ?

[yuyou]

@xordoquy yes, confirmed. the CURL script got "HTTP/1.0 200 OK" after I locally made that change and the server received a correct instance of Django UploadedFile (in this case, it is InMemoryUploadedFile object).

Tested under
Django (1.8.6)
djangorestframework (3.3.1)

[BraisGabin]

Hmmm, probably related with #3374. I had made a research in that issue and I had found a problem at the same line.

[xordoquy]

@BraisGabin I think it's a bit different the content type seems consistent with sending file.

[gregmuellegger]

I found to have the same issue. I think the issue results from the implementation of Django's File.__bool__ method. It makes the File evaluate to False in the if statement if file_obj:. The __bool__ method only tests for the filename. Therefore all file uploads without a filename are rejected.

The restframework tests for the parsers are testing these cases and explicitly don't allow uploads without a filename.

If you add a filename via the HTTP_CONTENT_DISPOSITION header, it will work. I think the raised ParseError message FileUpload parse error - none of upload handlers can handle the stream is misleading in this case and should be changed, but after all it's not a bug.

[tomchristie]

Good digging @gregmuellegger!

[yuyou]

Very good explanation and thanks to @gregmuellegger. I tested with CURL command:
curl -XPOST -H "Content-type: image/jpg" --data-binary @./app/test_assets/image.jpg -v -H 'Content-Disposition: attachment;filename="image.jpg"' "http://127.0.0.1:8000/media/"

and it works. So shall I close this issue?

[tomchristie]

So shall I close this issue?

Thanks, but no - we still have behavior here we should improve. :smile:

[BraisGabin]

I did a git bisect and I found that this issue was added in 566812ac0b577f79801ec67f86f064cf39ebee01. The test of my sample code https://github.com/BraisGabin/drf_3_3_test starts to fail with that commit.