uploaded file permissions

[Dan Watson]

As mentioned a few times in #2070, uploaded files large enough to be
streamed to a temporary file get created with a mode of 0600, as per
python's tempfile.mkstemp. This causes two problems:

1. Files uploaded into memory and saved to disk respect the umask, so
uploads could have different permissions based on how big they are.

2. If the webserver user and django user do not match (such as when
running an external FastCGI process), the webserver can no longer
serve uploaded files.

I got around this issue by subclassing FileSystemStorage._save to call
os.chmod, but it seems there should either be a setting (eg:
FILE_UPLOAD_MODE), or a mode that respects the current umask should be
specified when saving (and moving) the files. I can create a ticket
and patch either way.

Dan

[Dan Watson]

This inconsistency seems like a bug to me, so I've created ticket
#8454 [1] with a patch that implements a FILE_UPLOAD_PERMISSIONS
setting, and marked it as 1.0. This seemed like the least intrusive
way to go, and the default behavior is unchanged.

Regards,
Dan

[1] http://code.djangoproject.com/ticket/8454

[Malcolm Tredinnick]

Why did you go with another setting rather than using the umask (which
hopefully would be set correctly -- the fastcgi server handles that
now)? Is there some technical difficulty I'm not seeing here that means
we can't get the umask and use that? It would seem nicer than having to
worry about another setting that can get out of sync with things if it
was possible.

I'm not disputing (yet :-)) that your approach is correct. Rather,
wondering what I'm missing.

Regards,
Malcolm

[Dan Watson]

On Aug 20, 3:34 pm, Malcolm Tredinnick <malc...@pointy-stick.com>
wrote:

I guess I was envisioning a situation where you'd want to chmod
uploaded files differently than other sorts of files your application
may be writing out. Also, it appears the only way to get the current
umask is by setting it at the same time, which seems like a good way
to introduce a race condition (set to X to retrieve Y, another caller
gets X instead of Y).

> I'm not disputing (yet :-)) that your approach is correct. Rather,
> wondering what I'm missing.

I don't think either solution is bad, so long as there is a solution
and it's documented. I'm not heavily in favor of one or the other, so
if you'd rather see the current umask used, I'll submit a new patch.
Another option would be to give the setting a reasonable default value
(likely 0600, 0644, or 0640).

Regards,
Dan