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

@@ -8,13 +8,31 @@
 import os
 from django.core.files import locks
 
-__all__ = ['file_move_safe']
-
 try:
-    import shutil
-    file_move = shutil.move
+    from shutil import copystat
 except ImportError:
-    file_move = os.rename
+    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']
+
+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

django/core/files/temp.py

@@ -25,31 +25,35 @@ def __init__(self, mode='w+b', bufsize=-1, suffix='', prefix='',
             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
-
-            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()
+            self.close()
+
+        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:

tests/regressiontests/file_uploads/views.py

@@ -2,6 +2,7 @@
 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):