Opened 4 years ago

Closed 3 years ago

#16946 closed Bug (duplicate)

core.files.File assumes name attribute on file-like objects

Reported by: PaulM Owned by: charettes
Component: Core (Other) Version: 1.3
Severity: Normal Keywords:
Cc: charette.s@…, nickname123 Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

The django.core.files.File object is a wrapper to add some django-specific stuff around standard python file-like objects.

https://code.djangoproject.com/browser/django/trunk/django/core/files/base.py#L10

In the __init__ and elsewhere, File works around file-like objects which may not have a name attribute (optional for python file objects). However, down in the _size() getter, it slips up and makes the assumption that the file-like object has a name attribute. This means that it produces a confusing attribute error about name rather than the desired error about being unable to determine a file's size.

This should be patched to check for self.name before doing the os.path.exists, like line 113.

Attachments (2)

16946.diff (697 bytes) - added by charettes 4 years ago.
Make sure django File wrapper size getter raises the correct exception when called on a an instance with no name
16946.with-test.diff (1.5 KB) - added by charettes 4 years ago.

Download all attachments as: .zip

Change History (10)

Changed 4 years ago by charettes

Make sure django File wrapper size getter raises the correct exception when called on a an instance with no name

comment:1 Changed 4 years ago by charettes

  • Cc charette.s@… added
  • Has patch set

comment:2 Changed 4 years ago by PaulM

  • Component changed from Uncategorized to Core (Other)
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

Looks good, needs a test. (I know, the devil in the details!) urllib.urlopen() is a good example of a file-like object that doesn't have a name that you could use for the test.

comment:3 Changed 4 years ago by PaulM

  • Needs tests set
  • Patch needs improvement set

comment:4 Changed 4 years ago by charettes

  • Owner changed from nobody to charettes
  • Status changed from new to assigned

I'll have a look at tests in the next few days.

Changed 4 years ago by charettes

comment:5 Changed 4 years ago by charettes

Added a new patch with test but it triggers an error in modeltests.files.tests.FileTests.tearDown:

ERROR: test_file_with_no_name_size (modeltests.files.tests.FileTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/simon/.virtualenvs/trunk/src/django/tests/modeltests/files/tests.py", line 21, in tearDown
    shutil.rmtree(temp_storage_location)
  File "/usr/lib/python2.7/shutil.py", line 236, in rmtree
    onerror(os.listdir, path, sys.exc_info())
  File "/usr/lib/python2.7/shutil.py", line 234, in rmtree
    names = os.listdir(path)
OSError: [Errno 2] No such file or directory: '/tmp/tmplXh8th'

Actually adding a new test_ method that doesn't create a file using temp_storage.save to modeltests.files.tests.FileTests.tearDown triggers this error in the tearDown and adding temp_storage.save('tests/default.txt', ContentFile('default content')) in the newly added test_file_with_no_name_size method prevents the tearDown from spitting this error...

Should the shutil.rmtree(temp_storage_location) be wrapped in a try statement to catch OSError?

comment:6 Changed 3 years ago by nickname123

Could these two be merged? https://code.djangoproject.com/ticket/15644

comment:7 Changed 3 years ago by nickname123

  • Cc nickname123 added

comment:8 Changed 3 years ago by claudep

  • Resolution set to duplicate
  • Status changed from assigned to closed

This is indeed a duplicate of #15644

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