Upload folder permissions

[petyo1313]

After upgrading to 3.7.1 I noticed that all new items uploaded to media folder cannot be opened with "Forbidden 403 error". I believe the newly created folders "11" are not open for reading by everyone.Those issues do not exist for anything uploaded prior October, 2013

[SergeyBiryukov]

The only related changeset appears to be [25047]. Moving for investigation.

[dd32]

More information regarding the permissions of the existing folders (wp-content/upload) and the other previously created folders would be good here.

The changeset didn't appear to change any logic - although it may be that PHP isn't applying the $mode to recursively created directories in some cases.

[dd32]

The only thing I've been able to find is that mkdir()'s $mode is affected by umask(), but chmod() isn't, that would be a behaviour change between 3.6 and 3.7.

umask basically sets the defaults for created files, by removing permissions from created files/directories.

This means that we cannot use the $recursive option of mkdir() AND use the $mode parameter reliably without a work around.

25822.diff​ attempts to work around this by in the event that it's detected that the umask altered the permissions it'd have created. In addition, dirname() doesn't return false, and also catches a case where stat() returns false resulting in the default value of 0777 not being used.

[dd32]

In 26449:

Fix a regression in wp_mkdir_p() where the $mode of the parent folder is not correctly applied to all created paths. Fixes #25822 for trunk

[fboender]

There's a bug in the patch (and thus now in the live code) which causes the permission-fixing done by wp_mkdir_p to fail. This line:

1381:                if ( $dir_perms != $dir_perms & ~umask() ) {

Shoud be:

1381:                if ( $dir_perms != ($dir_perms & ~umask()) ) {

Right now, the condition is not met properly due to operator precedence. This causes the code to re-set the $dir_perms correctly to not be run.

I can attach a patch if desired.

[schamane]

Thanks a lot fboender! I'd like to confirm the bug and your patch for 3.8.

[markoheijnen]

I think this ticket need to be closed since it was done for 3.8.

@fboender: Could you create a new ticket and add a patch to that one?

[fboender]

I've created a new ticket (#26781) for this bug. I've also added proof, steps to reproduce and a patch to fix the problem to the ticket.

[markoheijnen]

Thank you for doing that.

[dd32]

In 26927:

Uploads: Fix the Order of Operations for wp_mkdir_p() which caused this branch to never be hit. Props fboender. Fixes #26781 for 3.8.1. See #25822

[nacin]

In 27887:

Fix a regression in wp_mkdir_p() where the $mode of the parent folder is not correctly applied to all created paths.

Merges [26449] and [26927] from 3.8.x to the 3.7 branch.

props dd32.
fixes #25822.