Opened 9 years ago

Closed 6 years ago

#13518 closed New feature (fixed)


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


In core/files/, 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.


Attachments (1)

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

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by jacob@…

Attachment: storage_perms.patch added

comment:1 Changed 9 years ago by Russell Keith-Magee

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 Changed 8 years ago by Julien Phalip

Severity: Normal
Type: New feature

comment:3 Changed 8 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:4 Changed 8 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:5 Changed 6 years ago by Raumkraut

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:

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 Changed 6 years ago by Raumkraut

I've gone ahead and created a pull request with my patch:
Hopefully someone can take a look at it?

comment:7 Changed 6 years ago by Tim Graham

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 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 6bdb3b1135d1bd7b2dc24131b9d26ac19ebdba67:


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