From a9f0ae706a8dc035543ac64a1e87e9621a35fa00 Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Sat, 23 Aug 2008 17:56:02 +0000 Subject: [PATCH] Fixed #8203 -- Fixed temporary file deleation on Windows and a couple of edge cases on Unix-like systems. Patch from snaury. Testing and verification on Windows, Mac and Linux from cgrady and ramikassab. git-svn-id: http://code.djangoproject.com/svn/django/trunk@8493 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/core/files/move.py | 70 ++++++++++++++------- django/core/files/temp.py | 48 +++++++------- tests/regressiontests/file_uploads/views.py | 6 ++ 3 files changed, 81 insertions(+), 43 deletions(-) diff --git a/django/core/files/move.py b/django/core/files/move.py index 5ae776c3dd..58a0ab125d 100644 --- a/django/core/files/move.py +++ b/django/core/files/move.py @@ -8,13 +8,31 @@ Move a file in the safest way possible:: import os from django.core.files import locks +try: + from shutil import copystat +except ImportError: + def copystat(src, dst): + """Copy all stat info (mode bits, atime and mtime) from src to dst""" + st = os.stat(src) + mode = stat.S_IMODE(st.st_mode) + if hasattr(os, 'utime'): + os.utime(dst, (st.st_atime, st.st_mtime)) + if hasattr(os, 'chmod'): + os.chmod(dst, mode) + __all__ = ['file_move_safe'] -try: - import shutil - file_move = shutil.move -except ImportError: - file_move = os.rename +def _samefile(src, dst): + # Macintosh, Unix. + if hasattr(os.path,'samefile'): + try: + return os.path.samefile(src, dst) + except OSError: + return False + + # All other platforms: check for same pathname. + return (os.path.normcase(os.path.abspath(src)) == + os.path.normcase(os.path.abspath(dst))) def file_move_safe(old_file_name, new_file_name, chunk_size = 1024*64, allow_overwrite=False): """ @@ -30,31 +48,41 @@ def file_move_safe(old_file_name, new_file_name, chunk_size = 1024*64, allow_ove """ # There's no reason to move if we don't have to. - if old_file_name == new_file_name: + if _samefile(old_file_name, new_file_name): return - if not allow_overwrite and os.path.exists(new_file_name): - raise IOError("Cannot overwrite existing file '%s'." % new_file_name) - try: - file_move(old_file_name, new_file_name) + os.rename(old_file_name, new_file_name) return except OSError: # This will happen with os.rename if moving to another filesystem + # or when moving opened files on certain operating systems pass - # If the built-in didn't work, do it the hard way. - fd = os.open(new_file_name, os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, 'O_BINARY', 0)) + # first open the old file, so that it won't go away + old_file = open(old_file_name, 'rb') try: - locks.lock(fd, locks.LOCK_EX) - old_file = open(old_file_name, 'rb') - current_chunk = None - while current_chunk != '': - current_chunk = old_file.read(chunk_size) - os.write(fd, current_chunk) + # now open the new file, not forgetting allow_overwrite + fd = os.open(new_file_name, os.O_WRONLY | os.O_CREAT | getattr(os, 'O_BINARY', 0) | + (not allow_overwrite and os.O_EXCL or 0)) + try: + locks.lock(fd, locks.LOCK_EX) + current_chunk = None + while current_chunk != '': + current_chunk = old_file.read(chunk_size) + os.write(fd, current_chunk) + finally: + locks.unlock(fd) + os.close(fd) finally: - locks.unlock(fd) - os.close(fd) old_file.close() + copystat(old_file_name, new_file_name) - os.remove(old_file_name) + try: + os.remove(old_file_name) + except OSError, e: + # Certain operating systems (Cygwin and Windows) + # fail when deleting opened files, ignore it + if getattr(e, 'winerror', 0) != 32: + # FIXME: should we also ignore errno 13? + raise diff --git a/django/core/files/temp.py b/django/core/files/temp.py index 298fcbf0a5..f0d7d744ba 100644 --- a/django/core/files/temp.py +++ b/django/core/files/temp.py @@ -25,31 +25,35 @@ if os.name == 'nt': fd, name = tempfile.mkstemp(suffix=suffix, prefix=prefix, dir=dir) self.name = name - self._file = os.fdopen(fd, mode, bufsize) + self.file = os.fdopen(fd, mode, bufsize) + self.close_called = False + + # Because close can be called during shutdown + # we need to cache os.unlink and access it + # as self.unlink only + unlink = os.unlink + + def close(self): + if not self.close_called: + self.close_called = True + try: + self.file.close() + except (OSError, IOError): + pass + try: + self.unlink(self.name) + except (OSError): + pass def __del__(self): - try: - self._file.close() - except (OSError, IOError): - pass - try: - os.unlink(self.name) - except (OSError): - pass + self.close() - try: - super(TemporaryFile, self).__del__() - except AttributeError: - pass - - - def read(self, *args): return self._file.read(*args) - def seek(self, offset): return self._file.seek(offset) - def write(self, s): return self._file.write(s) - def close(self): return self._file.close() - def __iter__(self): return iter(self._file) - def readlines(self, size=None): return self._file.readlines(size) - def xreadlines(self): return self._file.xreadlines() + def read(self, *args): return self.file.read(*args) + def seek(self, offset): return self.file.seek(offset) + def write(self, s): return self.file.write(s) + def __iter__(self): return iter(self.file) + def readlines(self, size=None): return self.file.readlines(size) + def xreadlines(self): return self.file.xreadlines() NamedTemporaryFile = TemporaryFile else: diff --git a/tests/regressiontests/file_uploads/views.py b/tests/regressiontests/file_uploads/views.py index e9115f7fa1..b1de169cf0 100644 --- a/tests/regressiontests/file_uploads/views.py +++ b/tests/regressiontests/file_uploads/views.py @@ -2,6 +2,7 @@ import os from django.core.files.uploadedfile import UploadedFile from django.http import HttpResponse, HttpResponseServerError from django.utils import simplejson +from models import FileModel from uploadhandler import QuotaUploadHandler from django.utils.hashcompat import sha_constructor @@ -45,6 +46,11 @@ def file_upload_view_verify(request): if new_hash != submitted_hash: return HttpResponseServerError() + # Adding large file to the database should succeed + largefile = request.FILES['file_field2'] + obj = FileModel() + obj.testfile.save(largefile.name, largefile) + return HttpResponse('') def file_upload_echo(request):