Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixed #18233: Raise IOError if destination file already exists, in django.core.files.move.file_move_safe. #18

Closed
wants to merge 2 commits into
from
Jump to file or symbol
Failed to load files and symbols.
+25 −0
Split
View
@@ -563,6 +563,7 @@ answer newbie questions, and generally made Django that much better:
Gasper Zejn <zejn@kiberpipa.org>
Jarek Zgoda <jarek.zgoda@gmail.com>
Cheng Zhang
+ Aviral Dasgupta <aviraldg@gmail.com>
A big THANK YOU goes to:
@@ -51,6 +51,8 @@ def file_move_safe(old_file_name, new_file_name, chunk_size = 1024*64, allow_ove
return
try:
+ if not allow_overwrite and os.path.exists(new_file_name):
+ raise IOError('Destination file already exists: \'{0}\''.format(new_file_name))
@jezdez

jezdez May 3, 2012

Owner

It's best practice to use common string substitution.

os.rename(old_file_name, new_file_name)
return
except OSError:
@@ -23,6 +23,7 @@
from django.core.files.images import get_image_dimensions
from django.core.files.storage import FileSystemStorage, get_storage_class
from django.core.files.uploadedfile import UploadedFile
+from django.core.files.move import file_move_safe
from django.test import SimpleTestCase
from django.utils import unittest
from ..servers.tests import LiveServerBase
@@ -586,3 +587,24 @@ def test_urllib2_urlopen(self):
remote_file = self.urlopen('/example_view/')
self.assertEqual(stored_file.read(), remote_file.read())
+
+class MoveTestCase(unittest.TestCase):
+ """
+ Test django.core.files.move.
+ """
+ def setUp(self):
+ self.file_a = tempfile.mkstemp()
+ self.file_b = tempfile.mkstemp()
+
+ def tearDown(self):
+ os.close(self.file_a[0])
+ os.close(self.file_b[0])
+ os.remove(self.file_a[1])
+ os.remove(self.file_b[1])
+
+ def test_overwrite_existing_file(self):
+ """
+ Test that file_move_safe raises an IOError if you try to overwrite an
+ existing file without passing in allow_overwrite=True.
+ """
+ self.assertRaises(IOError, file_move_safe, self.file_a[1], self.file_b[1])