Author: jezdez Date: 2011-05-26 01:21:35 -0700 (Thu, 26 May 2011) New Revision: 16280
Modified: django/trunk/django/core/files/storage.py django/trunk/tests/regressiontests/file_storage/tests.py Log: Fixed #16082 -- Fixed race condition in the FileSystemStorage backend with regard to creating directories. Thanks, pjdelport. Modified: django/trunk/django/core/files/storage.py =================================================================== --- django/trunk/django/core/files/storage.py 2011-05-25 17:31:47 UTC (rev 16279) +++ django/trunk/django/core/files/storage.py 2011-05-26 08:21:35 UTC (rev 16280) @@ -161,10 +161,18 @@ def _save(self, name, content): full_path = self.path(name) + # Create any intermediate directories that do not exist. + # Note that there is a race between os.path.exists and os.makedirs: + # if os.makedirs fails with EEXIST, the directory was created + # concurrently, and we can continue normally. Refs #16082. directory = os.path.dirname(full_path) if not os.path.exists(directory): - os.makedirs(directory) - elif not os.path.isdir(directory): + try: + os.makedirs(directory) + except OSError, e: + if e.errno != errno.EEXIST: + raise + if not os.path.isdir(directory): raise IOError("%s exists and is not a directory." % directory) # There's a potential race condition between get_available_name and Modified: django/trunk/tests/regressiontests/file_storage/tests.py =================================================================== --- django/trunk/tests/regressiontests/file_storage/tests.py 2011-05-25 17:31:47 UTC (rev 16279) +++ django/trunk/tests/regressiontests/file_storage/tests.py 2011-05-26 08:21:35 UTC (rev 16280) @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- import os +import errno import shutil import sys import tempfile @@ -186,6 +187,23 @@ self.storage.delete(storage_f_name) + def test_file_save_with_path(self): + """ + Saving a pathname should create intermediate directories as necessary. + """ + self.assertFalse(self.storage.exists('path/to')) + self.storage.save('path/to/test.file', + ContentFile('file saved with path')) + + self.assertTrue(self.storage.exists('path/to')) + self.assertEqual(self.storage.open('path/to/test.file').read(), + 'file saved with path') + + self.assertTrue(os.path.exists( + os.path.join(self.temp_dir, 'path', 'to', 'test.file'))) + + self.storage.delete('path/to/test.file') + def test_file_path(self): """ File storage returns the full path of a file @@ -282,6 +300,44 @@ temp_storage.path(mixed_case)) temp_storage.delete(mixed_case) + def test_makedirs_race_handling(self): + """ + File storage should be robust against directory creation race conditions. + """ + # Monkey-patch os.makedirs, to simulate a normal call, a raced call, + # and an error. + def fake_makedirs(path): + if path == os.path.join(self.temp_dir, 'normal'): + os.mkdir(path) + elif path == os.path.join(self.temp_dir, 'raced'): + os.mkdir(path) + raise OSError(errno.EEXIST, 'simulated EEXIST') + elif path == os.path.join(self.temp_dir, 'error'): + raise OSError(errno.EACCES, 'simulated EACCES') + else: + self.fail('unexpected argument %r' % path) + + real_makedirs = os.makedirs + try: + os.makedirs = fake_makedirs + + self.storage.save('normal/test.file', + ContentFile('saved normally')) + self.assertEqual(self.storage.open('normal/test.file').read(), + 'saved normally') + + self.storage.save('raced/test.file', + ContentFile('saved with race')) + self.assertEqual(self.storage.open('raced/test.file').read(), + 'saved with race') + + # Check that OSErrors aside from EEXIST are still raised. + self.assertRaises(OSError, + lambda: self.storage.save('error/test.file', + ContentFile('not saved'))) + finally: + os.makedirs = real_makedirs + class CustomStorage(FileSystemStorage): def get_available_name(self, name): """ -- You received this message because you are subscribed to the Google Groups "Django updates" group. To post to this group, send email to django-updates@googlegroups.com. To unsubscribe from this group, send email to django-updates+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-updates?hl=en.