Opened 5 years ago

Last modified 7 months ago

#13809 new Bug

FileField open method is only accepting 'rb' modes

Reported by: canassa Owned by:
Component: File uploads/storage Version: master
Severity: Normal Keywords:
Cc: real.human@…, davidfstr Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: 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)
    pdf = models.FileField(upload_to="ordem_pdf")

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
    return self.admin_site.admin_view(view)(*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\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

Attachments (1)

13809-1.diff (1.9 KB) - added by claudep 3 years ago.
Allow FielField instance to be open in writable mode

Download all attachments as: .zip

Change History (14)

comment:1 Changed 5 years ago by dmoisset

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to 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 5 years ago by jesh

  • Resolution invalid deleted
  • Status changed from closed to 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()
  1. http://code.djangoproject.com/browser/django/trunk/django/db/models/fields/files.py#L49
  2. http://code.djangoproject.com/browser/django/trunk/django/core/files/base.py#L110

comment:3 follow-up: Changed 5 years ago by russellm

  • Component changed from File uploads/storage to Documentation
  • Triage Stage changed from Unreviewed to 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 5 years ago by yishai@…

  • Component changed from Documentation to File uploads/storage

Replying to russellm:

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 4 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:6 Changed 4 years ago by mrmachine

  • Cc real.human@… added
  • Easy pickings unset
  • Needs tests set
  • UI/UX unset
  • Version changed from 1.2 to 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 4 years ago by davidfstr

  • Cc davidfstr added

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 4 years ago by davidfstr (previous) (diff)

Changed 3 years ago by claudep

Allow FielField instance to be open in writable mode

comment:8 Changed 3 years ago by claudep

  • Has patch set
  • Needs tests unset
  • Owner nobody deleted
  • Status changed from reopened to 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 3 years ago by claudep

#16964 was a duplicate.

comment:10 Changed 18 months ago by anonymous

I confirm this issue as still present in Django 1.5.

comment:11 Changed 18 months ago by jetfix

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

comment:12 Changed 13 months ago by timgraham

  • Patch needs improvement set

comment:13 Changed 7 months ago by synotna

Still present in 1.7 :(

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

Last edited 7 months ago by synotna (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top