Fixed #16082 -- Fixed race condition in the FileSystemStorage backend with regard to creating directories. Thanks, pjdelport.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@16280 bcc190cf-cafb-0310-a4f2-bffc1f526a37

2 files changed, 66 insertions, 2 deletions

diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index 14e3b657b2..f29d16c985 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -161,10 +161,18 @@ class FileSystemStorage(Storage):
     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
diff --git a/tests/regressiontests/file_storage/tests.py b/tests/regressiontests/file_storage/tests.py
index cdec1879ea..f498f1f0e0 100644
--- a/tests/regressiontests/file_storage/tests.py
+++ b/tests/regressiontests/file_storage/tests.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 import os
+import errno
 import shutil
 import sys
 import tempfile
@@ -186,6 +187,23 @@ class FileStorageTests(unittest.TestCase):
 
         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 @@ class FileStorageTests(unittest.TestCase):
                          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):
         """