Streaming Upload file deleted before move

This is actually not the problem. The problem is #7675.

Replying to axiak:

This is actually not the problem. The problem is #7675.

I applied the patch from #7675. Now getting the following traceback.

Environment:

Request Method: POST
Request URL: http://localhost:8000/essayupload/
Django Version: 0.97-pre-SVN-7871
Python Version: 2.5.2
Installed Applications:
['appid.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.sites',
 'appid.apply',
 'django.contrib.admin',
 'registration',
 'extensions']
Installed Middleware:
('django.middleware.common.CommonMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'appid.auth.middleware.AuthenticationMiddleware',
 'django.middleware.doc.XViewMiddleware',
 'django.middleware.locale.LocaleMiddleware')


Traceback:
File "/usr/lib/python2.5/site-packages/django/core/handlers/base.py" in get_response
  86.                 response = callback(request, *callback_args, **callback_kwargs)
File "/usr/lib/python2.5/site-packages/django/contrib/auth/decorators.py" in __call__
  67.             return self.view_func(request, *args, **kwargs)
File "/home/cezar/code/appid/apply/views.py" in essayUpload
  316.             object = form.save(commit=False)
File "/usr/lib/python2.5/site-packages/django/newforms/models.py" in save
  273.         return save_instance(self, self.instance, self._meta.fields, fail_message, commit)
File "/usr/lib/python2.5/site-packages/django/newforms/models.py" in save_instance
  44.         f.save_form_data(instance, cleaned_data[f.name])
File "/usr/lib/python2.5/site-packages/django/db/models/fields/__init__.py" in save_form_data
  865.             getattr(instance, "save_%s_file" % self.name)(data.name, data, save=False)
File "/usr/lib/python2.5/site-packages/django/db/models/fields/__init__.py" in <lambda>
  817.         setattr(cls, 'save_%s_file' % self.name, lambda instance, filename, raw_field, save=True: instance._save_FIELD_file(self, filename, raw_field, save))
File "/usr/lib/python2.5/site-packages/django/db/models/base.py" in _save_FIELD_file
  533.             raw_field.close()
File "/usr/lib/python2.5/site-packages/django/core/files/uploadedfile.py" in close
  204.     def close(self):                return self._file.close()
File "/usr/lib/python2.5/tempfile.py" in close
  411.                 self.unlink(self.name)

Exception Type: OSError at /essayupload/
Exception Value: [Errno 2] No such file or directory: '/tmp/tmpVwM5Fd.upload'

Patch for #7683

Added a patch.

Attached patch doesn't fix the problem either. It looks like raw_field.close() tries to delete the file, and if file was moved first, it throws an exception. I'm wondering if we need to call close for NamedTemporaryFile? objects at all? Original patch 2070 didn't override this method and everything worked.

Close method on the NamedTemporaryFile? removes temporary file before it can be moved somewhere.

See also #7658

#7766 reported close before move as a problem also.

I think the close should still be called to handle self._file.file.close() as well as setting self._file.close_called. raw_field.close() should be moved after the file_move_safe and the exception should be caught within the close. Tested on Linux.

7683.3.dif works on mac osx 10.5, apache2 modwsgi

7683.3.diff works on ubuntu linux - hardy - apache2 mod-wsgi

some one shut apply it to the subversion..

thanks for the patch

patch .3.diff works like a charm. CC'ing myself

This patch works correctly for me under Debian/Lenny.

On the grounds that bare except catching is bad, the change to the close() method should try to catch as specific an exception as possible (and possibly even check the errno value).

Handling a specific error is good. Accidentally disguising all errors as if they are okay is bad.

Catches the specific OSError and passes the rest.

same as patch 7683.3, but just catching errno ==2 (missing file)

Depending on how specific you prefer to catch the error, patch 7683.5 will just catch OSError with errno==2 (missing file) and pass on all other exceptions. Pick you choice (I think just catching all OSError as patch 7683.4 did should be fine).

Replying to spaetz:

patch 7683.5 will just catch OSError with errno==2 (missing file)

Bear with my spamming. patch 7683.5 is based on 7683.4 and not on 7683.3 as accidentally written in the description.

Catching errno=2 is the right thing here, so thanks for the final patch. There are plenty of other filesystem errors that could occur that are unrelated and serious enough to want to know about.

Whoever checks this in should give it another read over for sanity, but this should be pretty much ready now.

#8021 is another report of this, I believe.

Replying to spaetz:

Depending on how specific you prefer to catch the error, patch 7683.5 will just catch OSError with errno==2 (missing file) and pass on all other exceptions. Pick you choice (I think just catching all OSError as patch 7683.4 did should be fine).

It's good work in ubuntu 8.04 and django 1.0a(#8156)

Does this patch still need improvement?

I don't think so. If you agree, please reset the "patch needs improvement" flag. Please note mtredinnick: "Whoever checks this in should give it another read over for sanity, but this should be pretty much ready now."

(In [8217]) Fixed #7683: Try not to delete uploaded files before moving them, and don't wig out of someone else does. Patch from screeley and spaetz.