Fixed #18947 -- Don't make uploaded files executeable by default.

Thanks to Lauri Tirkkonen for the patch.

3 files changed, 22 insertions, 5 deletions

diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index 0b300cd31e..650373f0c3 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -192,7 +192,10 @@ class FileSystemStorage(Storage):
                 else:
                     # This fun binary flag incantation makes os.open throw an
                     # OSError if the file already exists before we open it.
-                    fd = os.open(full_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, 'O_BINARY', 0))
+                    flags = (os.O_WRONLY | os.O_CREAT | os.O_EXCL |
+                             getattr(os, 'O_BINARY', 0))
+                    # The current umask value is masked out by os.open!
+                    fd = os.open(full_path, flags, 0o666)
                     try:
                         locks.lock(fd, locks.LOCK_EX)
                         _file = None
diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt
index 6420239f47..26b6ad1bfa 100644
--- a/docs/releases/1.5.txt
+++ b/docs/releases/1.5.txt
@@ -333,6 +333,11 @@ Miscellaneous
   function at :func:`django.utils.text.slugify`. Similarly, ``remove_tags`` is
   available at :func:`django.utils.html.remove_tags`.
 
+* Uploaded files are no longer created as executable by default. If you need
+  them to be executeable change :setting:`FILE_UPLOAD_PERMISSIONS` to your
+  needs. The new default value is `0666` (octal) and the current umask value
+  is first masked out.
+
 Features deprecated in 1.5
 ==========================
 
diff --git a/tests/regressiontests/file_storage/tests.py b/tests/regressiontests/file_storage/tests.py
index 281041e651..6b57ad6160 100644
--- a/tests/regressiontests/file_storage/tests.py
+++ b/tests/regressiontests/file_storage/tests.py
@@ -4,6 +4,7 @@ from __future__ import absolute_import, unicode_literals
 import errno
 import os
 import shutil
+import sys
 import tempfile
 import time
 from datetime import datetime, timedelta
@@ -23,6 +24,7 @@ from django.core.files.uploadedfile import UploadedFile
 from django.test import SimpleTestCase
 from django.utils import six
 from django.utils import unittest
+from django.test.utils import override_settings
 from ..servers.tests import LiveServerBase
 
 # Try to import PIL in either of the two ways it can end up installed.
@@ -433,22 +435,29 @@ class FileSaveRaceConditionTest(unittest.TestCase):
         self.storage.delete('conflict')
         self.storage.delete('conflict_1')
 
+@unittest.skipIf(sys.platform.startswith('win'), "Windows only partially supports umasks and chmod.")
 class FileStoragePermissions(unittest.TestCase):
     def setUp(self):
-        self.old_perms = settings.FILE_UPLOAD_PERMISSIONS
-        settings.FILE_UPLOAD_PERMISSIONS = 0o666
+        self.umask = 0o027
+        self.old_umask = os.umask(self.umask)
         self.storage_dir = tempfile.mkdtemp()
         self.storage = FileSystemStorage(self.storage_dir)
 
     def tearDown(self):
-        settings.FILE_UPLOAD_PERMISSIONS = self.old_perms
         shutil.rmtree(self.storage_dir)
+        os.umask(self.old_umask)
 
+    @override_settings(FILE_UPLOAD_PERMISSIONS=0o654)
     def test_file_upload_permissions(self):
         name = self.storage.save("the_file", ContentFile("data"))
         actual_mode = os.stat(self.storage.path(name))[0] & 0o777
-        self.assertEqual(actual_mode, 0o666)
+        self.assertEqual(actual_mode, 0o654)
 
+    @override_settings(FILE_UPLOAD_PERMISSIONS=None)
+    def test_file_upload_default_permissions(self):
+        fname = self.storage.save("some_file", ContentFile("data"))
+        mode = os.stat(self.storage.path(fname))[0] & 0o777
+        self.assertEqual(mode, 0o666 & ~self.umask)
 
 class FileStoragePathParsing(unittest.TestCase):
     def setUp(self):