Opened 14 years ago

Closed 11 years ago

#13518 closed New feature (fixed)

Add settings.FILE_UPLOAD_DIRECTORY_PERMISSIONS

Reported by: jacob@… Owned by: nobody
Component: File uploads/storage Version: 1.1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Hi.
In core/files/storage.py, FILE_UPLOAD_PERMISSIONS is used to set the permissions of created files. It also makes the containing directory if it doesn't exist, but doesn't set the permissions for this directory. One possible patch is attached.

I can imagine cases where one would want the directory permissions to be different from file permissions. I'm not sure how objectional new setting names are.

Thanks.

Attachments (1)

storage_perms.patch (570 bytes ) - added by jacob@… 14 years ago.

Download all attachments as: .zip

Change History (9)

by jacob@…, 14 years ago

Attachment: storage_perms.patch added

comment:1 by Russell Keith-Magee, 14 years ago

Has patch: set
Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Good suggestion, but:

  1. Patch should be generated as a diff against the root of the source tree
  2. Patch requires tests
  3. Patch probably also needs a note added to documentation describing the permissions that new directories will be created with

As for introducing a new FILE_UPLOAD_DIRECTORY_PERMISSIONS setting; while we generally try to avoid introducing new settings, in this case it makes sense to me. However, this also means extra documentation for point (3).

comment:2 by Julien Phalip, 13 years ago

Severity: Normal
Type: New feature

comment:3 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:4 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:5 by Raumkraut, 11 years ago

The lack of this feature hit me a few days ago, so I've worked up a fix, taking into account the given suggestions.

The commit in question:
https://github.com/Raumkraut/django/commit/1c0fe5d1c09bb3210812e32b88577c4572520f40

Since this is my first Django commit, and the ticket is somewhat old, there are some things I'm not certain are "correct":

  • The documentation sections I've added defer explanation of the intricacies of file permissions to the existing FILE_UPLOAD_PERMISSIONS entries, in preference to duplicating the information.
  • Since os.makedirs accounts for the system umask, and os.chmod doesn't, I've added code to temporarily zero out the umask while the directories are created. The intent is to maintain consistency of behaviour between the two settings.

Are these acceptable implementations, or do I need to tweak further before I send out a pull request?

comment:6 by Raumkraut, 11 years ago

I've gone ahead and created a pull request with my patch: https://github.com/django/django/pull/1225
Hopefully someone can take a look at it?

comment:7 by Tim Graham, 11 years ago

Needs documentation: unset
Needs tests: unset
Summary: FILE_UPLOAD_PERMISSIONS used for created files, but not created directoriesAdd settings.FILE_UPLOAD_DIRECTORY_PERMISSIONS

Comments on PR.

comment:8 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 6bdb3b1135d1bd7b2dc24131b9d26ac19ebdba67:

Fixed #13518 -- Added FILE_UPLOAD_DIRECTORY_PERMISSIONS setting

This setting does for new directories what FILE_UPLOAD_PERMISSIONS
does for new files.

Thanks jacob@ for the suggestion.

Note: See TracTickets for help on using tickets.
Back to Top