WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#23196 closed defect (bug) (fixed)

mkdir not recursive (in wp-includes/functions.php)

Reported by: uuf6429 Owned by: dd32
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Filesystem API Keywords: has-patch needs-testing
Focuses: Cc:

Description

This January, Wordpress tried to create a directory and a subdirectory in "uploads": "2013" and inside it "01".

mkdir($target) was used to create these two directories, but since the $recursive option (3rd argument) was not set, this operation failed.

I have just checked this path and it does exist, hence I suspect this procedure is being done somewhere else (and succeeding).

Still, this is a problem since:

  1. It is showing up in our error logging system.
  2. It is very easy to fix.
  3. It potentially fixes more problems than it seems.

The solution is to change this line:

1306: if ( @mkdir( $target ) ) {

To the following:

1306: if ( @mkdir( $target, 0777, true ) ) {

On a sidenote, PHP's default directory creation mode is 0777 (so the above code is best for compatibility), but as you may know, it is not good in terms of security, so you may want to look for a more secure fix (eg, lowering the mode if it doesn't cause problems).

Attachments (1)

23196.diff (1.7 KB) - added by dd32 6 years ago.

Download all attachments as: .zip

Change History (10)

#1 @SergeyBiryukov
7 years ago

  • Component changed from General to Filesystem
  • Keywords has-patch removed

#2 follow-up: @dd32
7 years ago

Although a error may be logged in this case, the function, wp_mkdir_p does attempt to recursively create the directories, but doesn't use the recursive option (since it was written when we still supported PHP4).

#3 in reply to: ↑ 2 @uuf6429
7 years ago

Replying to dd32:

Although a error may be logged in this case, the function, wp_mkdir_p does attempt to recursively create the directories, but doesn't use the recursive option (since it was written when we still supported PHP4).

Well then, go fix it. :-)

#4 follow-up: @dd32
7 years ago

If the folder didn't get created for you, chances are your uploads folder doesn't allow folder creation at present, and using the recursive option still wouldn't have fixed it.

Well then, go fix it. :-)

Feel free to submit a patch.

#5 in reply to: ↑ 4 ; follow-up: @uuf6429
7 years ago

Replying to dd32:

If the folder didn't get created for you, chances are your uploads folder doesn't allow folder creation at present, and using the recursive option still wouldn't have fixed it.

If you read my issue above again, I said at some point in time the folder was created anyway.
And in case that wasn't enough, I know for sure Wordpress does have permission to create folders on the host in question.

I don't track when/who creates folders on my server, I only log issues such as this one, including it's time, file/line of occurrence and the variables in scope. From all this info, it seems the only logical issue is what I mentioned above.

Feel free to submit a patch.

But then, we bypass the need of Trac, and we don't want that, do we?

#6 in reply to: ↑ 5 @rmccue
7 years ago

Replying to uuf6429:

But then, we bypass the need of Trac, and we don't want that, do we?

Actually, that's kinda the main point of Trac: you can write a patch, and attach it to this ticket right here (via the "Attach file" button just below the description). :)

@dd32
6 years ago

#7 @dd32
6 years ago

  • Keywords has-patch needs-testing added
  • Milestone changed from Awaiting Review to 3.7
  • Owner set to dd32
  • Status changed from new to accepted

This patch works for me, and it feels like it'll work with Streams too, but I'm not 100% positive.

Does anyone have a setup handy with a Streams setup that they can test this on?

#8 @dd32
6 years ago

https://bugs.php.net/bug.php?id=33140 is the only PHP bug I could find that affects the recursive option, it looks like it was fixed for windows in 5.2.0 finally, so we should be safe moving to it.

#9 @dd32
6 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 25047:

Make use of the recursive option in mkdir() in wp_mkdir_p(). Avoids a bunch of silenced PHP Notices being logged. Fixes #23196

Note: See TracTickets for help on using tickets.