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

Thanks to Lauri Tirkkonen for the patch.

Author: apollo13

django/core/files/storage.py

@@ -192,7 +192,10 @@ def _save(self, name, content):
  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

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
 ==========================
 

tests/regressiontests/file_storage/tests.py

@@ -4,6 +4,7 @@
 import errno
 import os
 import shutil
+import sys
 import tempfile
 import time
 from datetime import datetime, timedelta
@@ -23,6 +24,7 @@
 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 @@ def test_race_condition(self):
  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):