Make WordPress Core

Opened 6 years ago

Last modified 4 weeks ago

#11903 accepted defect (bug)

insert_with_markers is not threadsafe — at Version 4

Reported by: strings28 Owned by: ryan
Milestone: Future Release Priority: normal
Severity: major Version: 2.9
Component: Permalinks Keywords: has-patch needs-refresh
Focuses: Cc:

Description (last modified by Denis-de-Bernardy)

From wp-admin/includes/misc.php the function insert_with_markers may be called multiple times on a busy server and if the htaccess is already in the process of being written it is possible that two PHP threads could attempt to write to it causing corruption such as the following:

# BEGIN WordPress
<IfModule mod_rewrite.c>
RewriteEngine On
RewriteBase /
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule . /index.php [L]

# END WordPress

Notice the dangling 's' at the last line

Change History (6)

6 years ago

A proposed patch which creates a lock file

#1 @strings28
6 years ago

As an added note the patch file is what I have used on one server that was seeing a corrupted .htaccess file (and thus a 500 error) multiple times a week and it didn't matter which PHP configuration we setup our bluehost account with, the corruption took place. By using lock files it returns early - I also setup an adaptation of the mark string, which could possibly be adjusted to use the wp-prefix variable so as to help make it more obvious to those looking at the file that it is being inserted by a unique Wordpress installation.

6 years ago

Diff proper for strings28

#2 @miqrogroove
6 years ago

  • Keywords needs-patch added; .htaccess threads removed
  • Milestone changed from Unassigned to 3.0

I like this bug. I don't like the patch for two reasons: It doesn't prevent fopen() races. It doesn't handle error recovery.

I think the correct way to handle this is with flock() and no extra file.

#3 @Denis-de-Bernardy
6 years ago

I don't like the flock() idea. flock() isn't always available and is tricky to use on some servers, as was revealed by plugins such as wp-cache and its derivatives.

the "standard" workaround is to write to a temporary file, and then to rename the temporary file, overwriting the target file as needed.

#4 @Denis-de-Bernardy
6 years ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.