Opened 4 years ago

Closed 4 years ago

#26495 closed Cleanup/optimization (fixed)

Saving StringIO using custom storage class may result in empty content written to file

Reported by: Maxim Novikov Owned by: nobody
Component: File uploads/storage Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Maxim Novikov)

https://github.com/django/django/blob/master/django/core/files/storage.py#L50

class Storage(object):

    def save(self, name, content, max_length=None):
        ...
        if not hasattr(content, 'chunks'):
            content = File(content)

For example if custom storage uses requests with post query and we are saving StringIO object.

from StringIO import StringIO

from django.core.files.storage import Storage
import requests

class CustomStorage(Storage):
    def _save(self, name, content, max_length=None):
          requests.post('http://testhost.org/upload', data=content)
custom_storage = CustomStorage()
custom_storage.save('new_name', StringIO('content'))

But it will result in empty upload as bool(content) is False.
Possibly better to pass a name to File object. To ensure bool(content) is True

    if not hasattr(content, 'chunks'):
        content = File(content, name=name)

Change History (8)

comment:1 Changed 4 years ago by Tim Graham

It's not clear to me how to reproduce the issue. In particular I'm not sure what the store method refers to (I don't see anything in Django itself with that name). Please try to include more details such as a test for Django's test suite (ideally) or a sample project.

comment:2 Changed 4 years ago by Maxim Novikov

Description: modified (diff)

comment:3 Changed 4 years ago by Tim Graham

Thanks for updating the description. It's still not entirely clear to me where the problem lies or how the proposed change will fix it. Any chance you could offer a tested patch? Otherwise, I'm not too sure how to proceed here.

comment:4 Changed 4 years ago by Maxim Novikov

Description: modified (diff)
Summary: Storage save method wraps StringIO in File object which is identified as False.Saving StringIO using custom storage class may result in empty content written to file

comment:6 Changed 4 years ago by Maxim Novikov

Has patch: set

comment:7 Changed 4 years ago by Tim Graham

Triage Stage: UnreviewedReady for checkin
Type: UncategorizedCleanup/optimization

It's a bit odd to me that bool(file) is based on the file's name, but in light of that, I guess this makes sense.

comment:8 Changed 4 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 4d1c229e:

Fixed #26495 -- Added name arg to Storage.save()'s File wrapping.

Note: See TracTickets for help on using tickets.
Back to Top