Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Django default file mode-settings for uploaded files, and limit use of system() #64

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

johnbergvall
Copy link

These commits make the following changes:

  • Use context manager to make sure file handlers are closed directly after use
  • Use Django settings FILE_UPLOAD_PERMISSIONS and FILE_UPLOAD_DIRECTORY_PERMISSIONS as fallback to determinate new file- and directory modes. This also makes sure to use correct permissions for all created subdirectories.
  • Use shutil.chown() and os.chmod() instead of spawning separate shell command to recursively change file permissions

The shutil.chown() and os.chmod() also fixes an issue where the previous recursive command may set incompatible permissions on any subdirectories for the path (e.g. no +x-permissions)

In case you would want me to split the pull request, using settings.FILE_UPLOAD_PERMISSIONS will need separate handling to not overwrite directory permissions due to the recursive flag in the system-command.

… file permissions and owner.

Only change one file at the time instead of recursive change, due to that directory permissions and file permissions usually don't mix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant