FileUploadParser breaks with empty file names and multiple upload handlers

[skrivanos]

When there are multiple upload handlers defined and one of them raises StopFutureHandlers (in my case, MemoryFileUploadHandler and TemporaryFileUploadHandler) the self.file property is never set on the remaining handlers (since this is usually done in the new_file method).

This causes issues when iterating over the handlers later trying to complete the file:

class FileUploadParser():
        for i, handler in enumerate(upload_handlers):
            file_obj = handler.file_complete(counters[i])
            if file_obj:
                return DataAndFiles(None, {'file': file_obj})
        raise ParseError("FileUpload parse error - "
                         "none of upload handlers can handle the stream")

class File():
    def __bool__(self):
        return bool(self.name)

With an empty filename, since bool(file_obj) == False, it'll continue the iteration even the handler that issued StopFutureHandlers earlier. Since the file property wasn't set on the remaining handlers handler.file_complete will raise an AttributeError:

AttributeError:'TemporaryFileUploadHandler' object has no attribute 'file'

[tomchristie]

Any chance you confirm if this issue also affect Django core. We're using the same file parsing as from core so would be good to know if there's also a bigger issue to be fixed?

[skrivanos]

Django's parser skips files without a filename completely:

file_name = disposition.get('filename')
if not file_name:
    continue

I guess the question is whether or not to allow raw uploads without a filename. In my opinion there's a use case for where file names might be set beforehand since we're handling raw data.

[tomchristie]

Also confirmed by #2137.

[brianmay]

Have I understood this correctly?

diff --git a/rest_framework/parsers.py b/rest_framework/parsers.py
index ccb82f0..cf5de4a 100644
--- a/rest_framework/parsers.py
+++ b/rest_framework/parsers.py
@@ -256,12 +256,15 @@ class FileUploadParser(BaseParser):
         chunks = ChunkIter(stream, chunk_size)
         counters = [0] * len(upload_handlers)

+        new_handlers = []
         for handler in upload_handlers:
             try:
                 handler.new_file(None, filename, content_type,
                                  content_length, encoding)
+                new_handlers.append(handler)
             except StopFutureHandlers:
                 break
+        upload_handlers = new_handlers

         for chunk in chunks:
             for i, handler in enumerate(upload_handlers):

Unfortunately this doesn't solve #2137, but I do at least get a cleaner error message now. Will followup with the other ticket if this patch looks ok.

[tomchristie]

Not sure @brianmay, havn't investigated this ticket yet, myself.

[tomchristie]

Thanks for the reports & investigation folks! :)