Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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
  • Loading branch information...
commit 723b620c7edab47561d6387c7db3ace2f3b7aa16 1 parent 0065d8a
Jannis Leidel authored May 26, 2011
12  django/core/files/storage.py
@@ -161,10 +161,18 @@ def _open(self, name, mode='rb'):
161 161
     def _save(self, name, content):
162 162
         full_path = self.path(name)
163 163
 
  164
+        # Create any intermediate directories that do not exist.
  165
+        # Note that there is a race between os.path.exists and os.makedirs:
  166
+        # if os.makedirs fails with EEXIST, the directory was created
  167
+        # concurrently, and we can continue normally. Refs #16082.
164 168
         directory = os.path.dirname(full_path)
165 169
         if not os.path.exists(directory):
166  
-            os.makedirs(directory)
167  
-        elif not os.path.isdir(directory):
  170
+            try:
  171
+                os.makedirs(directory)
  172
+            except OSError, e:
  173
+                if e.errno != errno.EEXIST:
  174
+                    raise
  175
+        if not os.path.isdir(directory):
168 176
             raise IOError("%s exists and is not a directory." % directory)
169 177
 
170 178
         # There's a potential race condition between get_available_name and
56  tests/regressiontests/file_storage/tests.py
... ...
@@ -1,5 +1,6 @@
1 1
 # -*- coding: utf-8 -*-
2 2
 import os
  3
+import errno
3 4
 import shutil
4 5
 import sys
5 6
 import tempfile
@@ -186,6 +187,23 @@ def test_file_save_without_name(self):
186 187
 
187 188
         self.storage.delete(storage_f_name)
188 189
 
  190
+    def test_file_save_with_path(self):
  191
+        """
  192
+        Saving a pathname should create intermediate directories as necessary.
  193
+        """
  194
+        self.assertFalse(self.storage.exists('path/to'))
  195
+        self.storage.save('path/to/test.file',
  196
+            ContentFile('file saved with path'))
  197
+
  198
+        self.assertTrue(self.storage.exists('path/to'))
  199
+        self.assertEqual(self.storage.open('path/to/test.file').read(),
  200
+            'file saved with path')
  201
+
  202
+        self.assertTrue(os.path.exists(
  203
+            os.path.join(self.temp_dir, 'path', 'to', 'test.file')))
  204
+
  205
+        self.storage.delete('path/to/test.file')
  206
+
189 207
     def test_file_path(self):
190 208
         """
191 209
         File storage returns the full path of a file
@@ -282,6 +300,44 @@ def test_file_storage_preserves_filename_case(self):
282 300
                          temp_storage.path(mixed_case))
283 301
         temp_storage.delete(mixed_case)
284 302
 
  303
+    def test_makedirs_race_handling(self):
  304
+        """
  305
+        File storage should be robust against directory creation race conditions.
  306
+        """
  307
+        # Monkey-patch os.makedirs, to simulate a normal call, a raced call,
  308
+        # and an error.
  309
+        def fake_makedirs(path):
  310
+            if path == os.path.join(self.temp_dir, 'normal'):
  311
+                os.mkdir(path)
  312
+            elif path == os.path.join(self.temp_dir, 'raced'):
  313
+                os.mkdir(path)
  314
+                raise OSError(errno.EEXIST, 'simulated EEXIST')
  315
+            elif path == os.path.join(self.temp_dir, 'error'):
  316
+                raise OSError(errno.EACCES, 'simulated EACCES')
  317
+            else:
  318
+                self.fail('unexpected argument %r' % path)
  319
+
  320
+        real_makedirs = os.makedirs
  321
+        try:
  322
+            os.makedirs = fake_makedirs
  323
+
  324
+            self.storage.save('normal/test.file',
  325
+                ContentFile('saved normally'))
  326
+            self.assertEqual(self.storage.open('normal/test.file').read(),
  327
+                'saved normally')
  328
+
  329
+            self.storage.save('raced/test.file',
  330
+                ContentFile('saved with race'))
  331
+            self.assertEqual(self.storage.open('raced/test.file').read(),
  332
+                'saved with race')
  333
+
  334
+            # Check that OSErrors aside from EEXIST are still raised.
  335
+            self.assertRaises(OSError,
  336
+                lambda: self.storage.save('error/test.file',
  337
+                    ContentFile('not saved')))
  338
+        finally:
  339
+            os.makedirs = real_makedirs
  340
+
285 341
 class CustomStorage(FileSystemStorage):
286 342
     def get_available_name(self, name):
287 343
         """

0 notes on commit 723b620

Please sign in to comment.
Something went wrong with that request. Please try again.