diff --git a/django/db/models/base.py b/django/db/models/base.py index 590aab97a4..51cefcf77b 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -472,11 +472,12 @@ class Model(object): return os.path.getsize(self._get_FIELD_filename(field)) def _save_FIELD_file(self, field, filename, raw_field, save=True): - directory = field.get_directory_name() - try: # Create the date-based directory if it doesn't exist. - os.makedirs(os.path.join(settings.MEDIA_ROOT, directory)) - except OSError: # Directory probably already exists. - pass + # Create the upload directory if it doesn't already exist + directory = os.path.join(settings.MEDIA_ROOT, field.get_directory_name()) + if not os.path.exists(directory): + os.makedirs(directory) + elif not os.path.isdir(directory): + raise IOError('%s exists and is not a directory' % directory) # Check for old-style usage (files-as-dictionaries). Warn here first # since there are multiple locations where we need to support both new diff --git a/tests/regressiontests/file_uploads/models.py b/tests/regressiontests/file_uploads/models.py index 2d5607b2a7..3701750afe 100644 --- a/tests/regressiontests/file_uploads/models.py +++ b/tests/regressiontests/file_uploads/models.py @@ -1,2 +1,9 @@ -# This file unintentionally left blank. -# Oops. \ No newline at end of file +import tempfile +import os +from django.db import models + +UPLOAD_ROOT = tempfile.mkdtemp() +UPLOAD_TO = os.path.join(UPLOAD_ROOT, 'test_upload') + +class FileModel(models.Model): + testfile = models.FileField(upload_to=UPLOAD_TO) diff --git a/tests/regressiontests/file_uploads/tests.py b/tests/regressiontests/file_uploads/tests.py index d2b581686f..601fc92688 100644 --- a/tests/regressiontests/file_uploads/tests.py +++ b/tests/regressiontests/file_uploads/tests.py @@ -1,9 +1,16 @@ import os +import errno import sha +import shutil import tempfile +import unittest + +from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase, client from django.utils import simplejson +from models import FileModel, UPLOAD_ROOT, UPLOAD_TO + class FileUploadTests(TestCase): def test_simple_upload(self): post_data = { @@ -179,3 +186,42 @@ class FileUploadTests(TestCase): self.assertEqual(got.get('file1'), 1) self.assertEqual(got.get('file2'), 2) + +class DirectoryCreationTests(unittest.TestCase): + """ + Tests for error handling during directory creation + via _save_FIELD_file (ticket #6450) + """ + def setUp(self): + self.obj = FileModel() + if not os.path.isdir(UPLOAD_ROOT): + os.makedirs(UPLOAD_ROOT) + + def tearDown(self): + os.chmod(UPLOAD_ROOT, 0700) + shutil.rmtree(UPLOAD_ROOT) + + def test_readonly_root(self): + """Permission errors are not swallowed""" + os.chmod(UPLOAD_ROOT, 0500) + try: + self.obj.save_testfile_file('foo.txt', SimpleUploadedFile('foo.txt', 'x')) + except OSError, err: + self.assertEquals(err.errno, errno.EACCES) + except: + self.fail("OSError [Errno %s] not raised" % errno.EACCES) + + def test_not_a_directory(self): + """The correct IOError is raised when the upload directory name exists but isn't a directory""" + # Create a file with the upload directory name + fd = open(UPLOAD_TO, 'w') + fd.close() + try: + self.obj.save_testfile_file('foo.txt', SimpleUploadedFile('foo.txt', 'x')) + except IOError, err: + # The test needs to be done on a specific string as IOError + # is raised even without the patch (just not early enough) + self.assertEquals(err.args[0], + "%s exists and is not a directory" % UPLOAD_TO) + except: + self.fail("IOError not raised")