Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #6450 -- Improved the checking of errors when creating the dire…

…ctories for saved files. Thanks to henry@precheur.org for the report and patch, and vung for the excellent test case.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@8007 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit d911a64ce8bdb10e7704262837473215fcdb9cbe 1 parent 6c4c60b
Russell Keith-Magee authored July 20, 2008
11  django/db/models/base.py
@@ -472,11 +472,12 @@ def _get_FIELD_size(self, field):
472 472
         return os.path.getsize(self._get_FIELD_filename(field))
473 473
 
474 474
     def _save_FIELD_file(self, field, filename, raw_field, save=True):
475  
-        directory = field.get_directory_name()
476  
-        try: # Create the date-based directory if it doesn't exist.
477  
-            os.makedirs(os.path.join(settings.MEDIA_ROOT, directory))
478  
-        except OSError: # Directory probably already exists.
479  
-            pass
  475
+        # Create the upload directory if it doesn't already exist
  476
+        directory = os.path.join(settings.MEDIA_ROOT, field.get_directory_name())
  477
+        if not os.path.exists(directory):
  478
+            os.makedirs(directory)
  479
+        elif not os.path.isdir(directory):
  480
+            raise IOError('%s exists and is not a directory' % directory)        
480 481
 
481 482
         # Check for old-style usage (files-as-dictionaries). Warn here first
482 483
         # since there are multiple locations where we need to support both new
11  tests/regressiontests/file_uploads/models.py
... ...
@@ -1,2 +1,9 @@
1  
-# This file unintentionally left blank.
2  
-# Oops.
  1
+import tempfile
  2
+import os
  3
+from django.db import models
  4
+
  5
+UPLOAD_ROOT = tempfile.mkdtemp()
  6
+UPLOAD_TO = os.path.join(UPLOAD_ROOT, 'test_upload')
  7
+
  8
+class FileModel(models.Model):
  9
+    testfile = models.FileField(upload_to=UPLOAD_TO)
46  tests/regressiontests/file_uploads/tests.py
... ...
@@ -1,9 +1,16 @@
1 1
 import os
  2
+import errno
2 3
 import sha
  4
+import shutil
3 5
 import tempfile
  6
+import unittest
  7
+
  8
+from django.core.files.uploadedfile import SimpleUploadedFile
4 9
 from django.test import TestCase, client
5 10
 from django.utils import simplejson
6 11
 
  12
+from models import FileModel, UPLOAD_ROOT, UPLOAD_TO
  13
+
7 14
 class FileUploadTests(TestCase):
8 15
     def test_simple_upload(self):
9 16
         post_data = {
@@ -179,3 +186,42 @@ def test_fileupload_getlist(self):
179 186
 
180 187
         self.assertEqual(got.get('file1'), 1)
181 188
         self.assertEqual(got.get('file2'), 2)
  189
+
  190
+class DirectoryCreationTests(unittest.TestCase):
  191
+    """
  192
+    Tests for error handling during directory creation 
  193
+    via _save_FIELD_file (ticket #6450)
  194
+    """
  195
+    def setUp(self):
  196
+        self.obj = FileModel()
  197
+        if not os.path.isdir(UPLOAD_ROOT):
  198
+            os.makedirs(UPLOAD_ROOT)
  199
+
  200
+    def tearDown(self):
  201
+        os.chmod(UPLOAD_ROOT, 0700)
  202
+        shutil.rmtree(UPLOAD_ROOT)
  203
+
  204
+    def test_readonly_root(self):
  205
+        """Permission errors are not swallowed"""
  206
+        os.chmod(UPLOAD_ROOT, 0500)
  207
+        try:
  208
+            self.obj.save_testfile_file('foo.txt', SimpleUploadedFile('foo.txt', 'x'))
  209
+        except OSError, err:
  210
+            self.assertEquals(err.errno, errno.EACCES)
  211
+        except:
  212
+            self.fail("OSError [Errno %s] not raised" % errno.EACCES)
  213
+
  214
+    def test_not_a_directory(self):
  215
+        """The correct IOError is raised when the upload directory name exists but isn't a directory"""
  216
+        # Create a file with the upload directory name
  217
+        fd = open(UPLOAD_TO, 'w')
  218
+        fd.close()
  219
+        try:
  220
+            self.obj.save_testfile_file('foo.txt', SimpleUploadedFile('foo.txt', 'x'))
  221
+        except IOError, err:
  222
+            # The test needs to be done on a specific string as IOError
  223
+            # is raised even without the patch (just not early enough)
  224
+            self.assertEquals(err.args[0], 
  225
+                              "%s exists and is not a directory" % UPLOAD_TO)
  226
+        except:
  227
+            self.fail("IOError not raised")

0 notes on commit d911a64

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