Opened 7 years ago

Closed 10 months ago

# FileField open method is only accepting 'rb' modes

Reported by: Owned by: Cesar Canassa Chris Sinchok File uploads/storage master Normal real.human@…, David Foster Ready for checkin yes no no no no no

### Description

I have the following model in my application:

class OrdemServico(models.Model):
carrinho = models.OneToOneField('Carrinho')
criacao = models.DateTimeField(u'data de criação', default=datetime.now)


Then I have the following code in my application:

>>> ordem, created = OrdemServico.objects.get_or_create(carrinho=self)
>>> ordem.pdf.open(mode='wb')
>>> f = ordem.pdf.file
>>> print f
c:\users\cesar\documents\projetos\rm2\mysite\media\ordem_pdf\157_5.pdf
>>> print f.tell()
0
>>> print f.write('abc')
Traceback (most recent call last):
File "C:\Python26\lib\site-packages\django\core\servers\basehttp.py", line 280, in run
self.result = application(self.environ, self.start_response)
File "C:\Python26\lib\site-packages\django\core\servers\basehttp.py", line 674, in __call__
return self.application(environ, start_response)
File "C:\Python26\lib\site-packages\django\core\handlers\wsgi.py", line 241, in __call__
response = self.get_response(request)
File "C:\Python26\lib\site-packages\django\core\handlers\base.py", line 142, in get_response
return self.handle_uncaught_exception(request, resolver, exc_info)
File "C:\Python26\lib\site-packages\django\core\handlers\base.py", line 100, in get_response
response = callback(request, *callback_args, **callback_kwargs)
File "C:\Python26\lib\site-packages\django\contrib\admin\options.py", line 239, in wrapper
File "C:\Python26\lib\site-packages\django\utils\decorators.py", line 76, in _wrapped_view
response = view_func(request, *args, **kwargs)
File "C:\Python26\lib\site-packages\django\views\decorators\cache.py", line 69, in _wrapped_view_func
response = view_func(request, *args, **kwargs)
File "C:\Python26\lib\site-packages\django\contrib\admin\sites.py", line 190, in inner
return view(request, *args, **kwargs)
File "C:\Python26\lib\site-packages\django\utils\decorators.py", line 21, in _wrapper
return decorator(bound_func)(*args, **kwargs)
File "C:\Python26\lib\site-packages\django\utils\decorators.py", line 76, in _wrapped_view
response = view_func(request, *args, **kwargs)
File "C:\Python26\lib\site-packages\django\utils\decorators.py", line 17, in bound_func
return func(self, *args2, **kwargs2)
File "C:\Python26\lib\site-packages\django\contrib\admin\options.py", line 991, in changelist_view
response = self.response_action(request, queryset=cl.get_query_set())
File "C:\Python26\lib\site-packages\django\contrib\admin\options.py", line 749, in response_action
response = func(self, request, queryset)
File "C:\Users\Cesar\Documents\Projetos\RM2\mysite\..\mysite\moveit\admin.py", line 41, in gerar_pdf
carrinho.gera_ordem()
File "C:\Users\Cesar\Documents\Projetos\RM2\mysite\..\mysite\moveit\models.py", line 204, in gera_ordem
print f.write('abc')
IOError: [Errno 9] Bad file descriptor


I did some debugging and I find out that the open() method was opening the file in read-only mode. This was causing the IOError during the write call.

I checked the django.db.models.fields.file.py file:

    def _get_file(self):
self._require_file()
if not hasattr(self, '_file') or self._file is None:
self._file = self.storage.open(self.name, 'rb')
return self._file

def open(self, mode='rb'):
self._require_file()
self.file.open(mode)
# open() doesn't alter the file's contents, but it does reset the pointer
open.alters_data = True


We I called the open() method it tried to access the file property which itself also tries to open the file, but in read-only mode.

I kinda solved the problem by calling the storage method directly:

    def open(self, mode='rb'):
self._require_file()
#self.file.open(mode)
self._file = self.storage.open(self.name, mode)


Thanks,
Cesar Canassa

### comment:1 Changed 7 years ago by Daniel F Moisset

Resolution: → invalid new → closed

The file field is not documented (so it isn't doing anything wrong), and the documented method (open()) work as expected, so I think this is not a bug

### comment:2 Changed 7 years ago by jesh

Resolution: invalid closed → reopened

This still does not work for me. Documented open() does not honor mode (it's not *reopening* r/o file). That has a annoying side effect, so that FieldFile.file.open("w") does not work. This is because _get_file has already opened it as read only. As a workaround, every open could be preceded by lonely close(), but IMHO that's just plain ugly. My app code looks like

class MyModel(models.Model:
file = models.FileField(...)

this does not work:
f=mymodel.file.open("rw")
mymodel.file.write(content) # -> invalid fd
mymodel.file.close()

this will work:
f=mymodel.file # file gets opened here, see [1]
f.file.close() # following open [2] uses "rb" without this -> invalid fd when writing
f.file.open("wb")
mymodel.file.write(content)
mymodel.file.close()


### comment:3 follow-up:  4 Changed 6 years ago by Russell Keith-Magee

Component: File uploads/storage → Documentation Unreviewed → Accepted

I don't think it's unreasonable to expect that when you access a file through a model field, it will be open ready for reading. The best solution I can see here is documenting the fact that files are opened "rb" when accessed, and if you want to perform write operations, you'll need to close the file first.

I'm open to any other suggestions, but I'll accept this as a documentation issue for now.

### comment:4 in reply to:  3 Changed 6 years ago by yishai@…

I don't think it's unreasonable to expect that when you access a file through a model field, it will be open ready for reading. The best solution I can see here is documenting the fact that files are opened "rb" when accessed, and if you want to perform write operations, you'll need to close the file first.

I think this is a little more subtle than that; since the underlying file object is "automatically" opened (with 'rb' mode) upon direct access, the main reason one would call FieldFile.open is to open the file in another mode. Having FieldFile.open(mode='w') return a bad file descriptor (for writing) without complaining is hardly optimal.

I'm open to any other suggestions, but I'll accept this as a documentation issue for now.

Even if deciding that one should always close first before opening the file for writing, calling FieldFile.close() is not enough as it will skip the underlying file object if it was not opened yet. This code will not work:

  obj = MyModel.objects.get(pk=1)
obj.myFileField.close() # would expect this to close the underlying file object, so it's ready to be reopened for writing
obj.myFileField.open('w')
obj.myFileField.write('something')


If we rewrite FieldFile.close() from:

    def close(self):
file = getattr(self, '_file', None)
if file is not None:
file.close()


to:

    def close(self):
self.file.close()


then the above snippet will work as advertised - the underlying file will be opened (rb) and then closed again. Currently the snippet needs to be:

  obj = MyModel.objects.get(pk=1)
obj.myFileField.file.close() # works, but hardly nice
obj.myFileField.open('w')
obj.myFileField.write('something')


which works, but accessing the underlying (undocumented?) file object directly is probably not what we really want here.

### comment:5 Changed 6 years ago by Julien Phalip

Severity: → Normal → Bug

### comment:6 Changed 6 years ago by Tai Lee

Cc: real.human@… added unset set unset 1.2 → SVN

The docs say that File.open() will "re-open" the file (and as a side effect, do File.seek(0). In reality, File.open() will only do File.seek(0) if the file is already open, and will ignore the newly specified mode.

A separate but related issue is that even if the file is explicitly closed, and File.open() is called with a new mode, File.mode will still return the original mode. I'm not sure if the intent of the docs was that re-opening a file would use the original mode if not specified, or the current mode. Either way, I think that File.mode should mirror the current file mode.

The problem with FieldFile.open() (a subclass of File) is that it calls FieldFile.file.open(), when FieldFile.file will open the file implicitly with a mode of "rb", and then .open() will just do .seek(0), regardless of the mode that was passed in.

I'm fine with the idea that FieldFile should be open in "rb" mode when accessed by default, but users shouldn't have to explicitly access it first so that they can insert an explicit call to .close() before calling .open() with a different mode.

I think the fix here involves a couple of steps.

1. File.open() should call .close() if the file is already open (instead of .seek(0)), and then actually re-open it with the new mode (or the mode that was originally used, if not specified), as per the docs.
1. FieldFile.open() should not implicitly open the file in "rb" mode when a mode is explicitly set. FieldFile.open().

Cheers.

### comment:7 Changed 6 years ago by David Foster

I confirm this issue as still present in Django 1.3.

In short, if you call FieldFile.open(mode=NOT_RB), it will always open in mode rb. This is due to FieldFile.open calling the self.file property, which auto-opens in 'rb' mode. FieldFile.open attempts to subsequently call open on this new file, but that does not reopen the file again.

My suggested workaround for:

• field_file.open(mode='w+b')

is:

• field_file.file = field_file.storage.open(field_file.name, 'w+b')

Naturally this is just a workaround, not a real fix.

Last edited 6 years ago by David Foster (previous) (diff)

### Changed 5 years ago by Claude Paroz

Allow FielField instance to be open in writable mode

### comment:8 Changed 5 years ago by Claude Paroz

Has patch: set unset nobody deleted reopened → new

The patch is still not optimal, because the file is opened in 'rb' mode upon access, then closed and reopened in the specified mode. Seems to work, though.

### comment:9 Changed 5 years ago by Claude Paroz

#16964 was a duplicate.

### comment:10 Changed 3 years ago by anonymous

I confirm this issue as still present in Django 1.5.

### comment:11 Changed 3 years ago by jetfix

I confirm this that this issue is still present in 1.5 =(

### comment:12 Changed 3 years ago by Tim Graham

Patch needs improvement: set

### comment:13 Changed 2 years ago by synotna

Still present in 1.7 :(

My workaround is .read(), .close () then open it again in another mode

Last edited 2 years ago by synotna (previous) (diff)

### comment:14 Changed 14 months ago by Tim Graham

Resolution: → duplicate new → closed

I think #26398 may have at least partially addressed this but I haven't verified completely.

### comment:15 Changed 14 months ago by Tim Graham

Resolution: duplicate closed → new

### comment:16 Changed 10 months ago by Chris Sinchok

Owner: set to Chris Sinchok new → assigned

### comment:17 Changed 10 months ago by Tim Graham

Patch needs improvement: unset Accepted → Ready for checkin

PR. I marked #26398 as a duplicate and will include the test from the PR for that ticket here.

### comment:18 Changed 10 months ago by Tim Graham <timograham@…>

Resolution: → fixed assigned → closed

In ac1975b1:

Fixed #13809 -- Made FieldFile.open() respect its mode argument.

Note: See TracTickets for help on using tickets.