Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

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
@jezdez jezdez authored
View
12 django/core/files/storage.py
@@ -161,10 +161,18 @@ def _open(self, name, mode='rb'):
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
View
56 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 @@ def test_file_save_without_name(self):
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 @@ def test_file_storage_preserves_filename_case(self):
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):
"""
Please sign in to comment.
Something went wrong with that request. Please try again.