#15644 closed Bug (fixed)
django.core.files.base.File enhancement / fix
Reported by: | nickname123 | Owned by: | nobody |
---|---|---|---|
Component: | File uploads/storage | Version: | dev |
Severity: | Normal | Keywords: | file |
Cc: | michael.palumbo87@… | 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
Hi,
I think that django.core.files.base.File should be expanded to handle a wider range of file objects. In my specific case, StringIO/cStringIO and tempfile.SpooledTemporaryFile objects.
Here is a simple demonstration of where the File class breaks:
from tempfile import SpooledTemporaryFile from django.core.files import File f = SpooledTemporaryFile( max_size = 1024,# 1kb mode = 'w+b',# must be open in binary mode bufsize = -1, suffix = '', prefix = 'tmp', dir = None ) f.write("hello") print len(File(f))
Here is the result (on Windows using Python2.6):
Traceback (most recent call last): File "<stdin>", line 1, in <module> File "C:\...\django\core\files\base.py", line 33, in __len__ return self.size File "C:\...\django\core\files\base.py", line 39, in _get_size elif os.path.exists(self.file.name): File "C:\...\lib\tempfile.py", line 559, in name return self._file.name AttributeError: 'cStringIO.StringO' object has no attribute 'name'
It should be noted that not only does the current implementation fail, but it breaks in the wrong code block because it doesn't verify that the name attribute is available.
I propose that the file objects seek and tell method be used as an additional fallback before throwing the attribute error as follows:
def _get_size(self): if not hasattr(self, '_size'): if hasattr(self.file, 'size'): self._size = self.file.size elif hasattr(self.file, 'name') and os.path.exists(self.file.name): self._size = os.path.getsize(self.file.name) elif hasattr(self.file, 'tell') and hasattr(self.file, 'seek'): pos = self.file.tell() self.file.seek(0,os.SEEK_END) self._size = self.file.tell() self.file.seek(pos) else: raise AttributeError("Unable to determine the file's size.") return self._size
My proposed patch fixes the problems mentioned above.
Attachments (3)
Change History (16)
by , 14 years ago
Attachment: | django.core.files.base.File.diff added |
---|
comment:1 by , 14 years ago
Version: | 1.2 → SVN |
---|
comment:2 by , 14 years ago
Type: | → New feature |
---|
comment:3 by , 14 years ago
Severity: | → Normal |
---|
comment:4 by , 14 years ago
Component: | Uncategorized → File uploads/storage |
---|---|
Needs documentation: | set |
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
comment:13 by , 13 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Type: | New feature → Bug |
I've reviewed the File documentation (https://docs.djangoproject.com/en/dev/ref/files/file/), but didn't find any complement to add related to this fix.
Changed Type to bug because of the error raised by _get_size when the underlying file object has no name (see also #16946 which is a duplicate).
comment:14 by , 13 years ago
That would be nice to make the File object more file-like object so that we could use the File Storage API with any file-like objects (e.g. with what urllib2.urlopen returns)
Because as for now, this does not work for the main reason said previously (elif os.path.exists(self.file.name):
import urllib2 from django.core.files.base import File from django.core.files.storage import FileSystemStorage url = 'http://www.google.fr' fs = FileSystemStorage(location='./here/') f = File(urllib2.urlopen(urllib2.Request(url))) fs.save("downloaded_file.html", f)
Is it planned to be changed ? Maybe for django 1.4 ? How can we help ?
Thanks.
by , 13 years ago
Attachment: | 15644-3.diff added |
---|
comment:15 by , 13 years ago
Cc: | added |
---|
I added a new patch because the chunks method of the File object did not work with urllib2.urlopen().
Indeed, this method was using the size which is unknown in this case.
I completed claudep's patch.
patch