The color module is not setting the file permissions on created files. This is a problem on sites that have a tight umask (i.e. 077). Similiar patches found their way into core in D5 (for example, the new install system), but as color is a new core module, the fixes did not make it here yet.

This patch should fix many of the Garland related problems, such as 110916 and maybe 97661, although I suspect there are more.

CommentFileSizeAuthor
#7 119196.patch1.88 KBdouggreen
#2 color_2.patch1.75 KBdouggreen
color_1.patch1.4 KBdouggreen

Comments

owen barton’s picture

owen barton commented

subscribing

douggreen’s picture

douggreen commented
Version: 5.x-dev » 6.x-dev
StatusFileSize
new color_2.patch1.75 KB

I had hoped that this would get into the 5.x bug fix releases, but seeing no movement here, the same problem exists in 6.x. While the original patch works for 5.x, the attached patch is re-rolled for 6.x (with slightly different line numbers).

douggreen’s picture

douggreen commented
Title: File Permissions - solution to Garland problems » Fix file permissions of generated color files
douggreen’s picture

douggreen commented

I just confirmed, this patch still applies.

douggreen’s picture

douggreen commented

We've been running this patch on all of our D5 sites ever since D5 was released, with no ill-effects. As real world scenerio's go, this patch only comes into effect when/if you use Garland's color picker. So, we've really only used it for a couple sites.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

There should probably be a brief comment regarding the permission setting. Here's what file.inc does in a similar context.

    // Give everyone read access so that FTP'd users or
    // non-webserver users can see/read these files,
    // and give group write permissions so group memebers
    // can alter files uploaded by the webserver.
    @chmod($dest, 0664);

Four lines is probably overkill. How about a simple:

    // Set standard file permissions for webserver-generated files

Otherwise, looks RTBC.

douggreen’s picture

douggreen commented
StatusFileSize
new 119196.patch1.88 KB

Re-rolled with the suggested comment and a few lines were moved around so the comment makes sense. I tested the new patch on our servers with the restricted umask, and it still works.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Fixed

Makes a lot of sense to reuse the file API. Committed!

drumm’s picture

drumm
he/him
Location NY, US
commented

Committed to 5.x.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)