Opened 20 months ago

Last modified 8 days ago

#18852 accepted enhancement

Nginx rewrite rules

Reported by: johnbillion Owned by: johnbillion
Priority: normal Milestone: Future Release
Component: Rewrite Rules Version: 3.3
Severity: normal Keywords: has-patch needs-testing
Cc: matt@…, justin@…, jk@…, Brian@…, dcowgill@…, bronson@…

Description (last modified by dd32)

On the Permalinks screen we show rewrite rules for IIS and for mod_rewrite on Apache. Nginx is getting pretty popular now, so we should think about showing Nginx rewrite rules on this screen.

Unfortunately WordPress can't write directly to Nginx's rewrite configuration, but we can show the required rules for convenience and we can allow plugins to filter them if necessary (in the samw way IIS and mod_rewrite rules can be filtered).

Attachments (3)

18852.patch (5.4 KB) - added by johnbillion 20 months ago.
18852.2.patch (5.5 KB) - added by johnbillion 20 months ago.
18852.3.patch (7.9 KB) - added by johnbillion 11 months ago.

Download all attachments as: .zip

Change History (26)

  • Keywords has-patch added

First pass at a patch to see where it gets us.

got_url_rewrite() is a new function which is a wrapper for detecting if we've got any method enabled for URL rewrites (mod_rewrite, IIS7 with permalinks, or Nginx).

The suggested rewrite rules are what I'm running on a couple of sites and what seems to be the most popular form of rewrites for WordPress on Nginx.

Patch also introduces a new $is_nginx global.

Thoughts, suggestions & feedback welcome.

+1

  • Description modified (diff)

Those rules can be slimed down a bit, no need to check for the file-exists check as try_files handles that through it's priority queue:

location / {
    try_files $uri $uri/ /index.php$args;
}

ie. If File exists, Elseif, If it's a Directory, Else, Lets pass it to WordPress

Updated patch. Slim down rewrite rules from dd32 and tweak permalink screen message.

  • Milestone changed from Awaiting Review to 3.5

index.php$is_args$args is best. $is_args is ? if there are $args, empty if not.

I think we should do this for 3.5. 18852.2.patch looks comprehensive; I like it.

comment:6 follow-up: ↓ 7   devesine11 months ago

Unless I'm misreading, this looks like it only produces the rewrite rules for single-site installs. Multisite subdomain also generates this for mod_rewrite:

# uploaded files
RewriteRule ^files/(.+) wp-includes/ms-files.php?file=$1 [L]

and subdirectory generates this:

# uploaded files
RewriteRule ^([_0-9a-zA-Z-]+/)?files/(.+) wp-includes/ms-files.php?file=$2 [L]

# add a trailing slash to /wp-admin
RewriteRule ^([_0-9a-zA-Z-]+/)?wp-admin$ $1wp-admin/ [R=301,L]
# [...]
RewriteRule  ^[_0-9a-zA-Z-]+/(wp-(content|admin|includes).*) $1 [L]
RewriteRule  ^[_0-9a-zA-Z-]+/(.*\.php)$ $1 [L]

The codex nginx page has an example which includes this (which are materially equivalent to what we've been using in a number of production sites):

# Add trailing slash to */wp-admin requests.
rewrite /wp-admin$ $scheme://$host$uri/ permanent;

# Directives to send expires headers and turn off 404 error logging.
location ~* \.(js|css|png|jpg|jpeg|gif|ico)$ {
	expires 24h;
	log_not_found off;
}

# Pass uploaded files to wp-includes/ms-files.php.
rewrite /files/$ /index.php last;

if ($uri !~ wp-content/plugins) {
	rewrite /files/(.+)$ /wp-includes/ms-files.php?file=$1 last;
}

# Rewrite multisite '.../wp-.*' and '.../*.php'.
if (!-e $request_filename) {
	rewrite ^/[_0-9a-zA-Z-]+(/wp-.*) $1 last;
	rewrite ^/[_0-9a-zA-Z-]+.*(/wp-admin/.*\.php)$ $1 last;
	rewrite ^/[_0-9a-zA-Z-]+(/.*\.php)$ $1 last;
}

Should those multisite-specific rules be part of this patch as well?

comment:7 in reply to: ↑ 6   johnbillion11 months ago

  • Keywords needs-refresh added
  • Owner set to johnbillion
  • Status changed from new to accepted

Replying to devesine:

Unless I'm misreading, this looks like it only produces the rewrite rules for single-site installs.

Correct. I'll update the patch (it needs a refresh anyway).

Also, the nginx page on Codex is a mess.

  • Keywords needs-testing added; needs-refresh removed

18852.3.patch is a tentatively updated patch (against r21157).

Included in the patch are single site rewrite rules which are displayed on the Permalinks screen, and Multisite rewrite rules which are displayed on the Network Setup screen.

The try_files directive (?) has been updated as per Nacin's suggestion.

The Multisite rules will need verifying for correctness as they are not the same as those found on the Codex page for nginx. Instead they more closely resemble the mod_rewrite and IIS rewrite rules for Multisite. I have just deployed these rewrite rules to an nginx site of mine, but I've yet to test them thoroughly.

location / {
	try_files $uri $uri/ /index.php$is_args$args;
}

# Uploaded files
rewrite ^/([_0-9a-zA-Z-]+/)?files/(.+) /wp-includes/ms-files.php?file=$2 last;

# Add trailing slash to /wp-admin
rewrite /wp-admin$ $scheme://$host$uri/ permanent;

if (!-e $request_filename) {
	rewrite ^/[_0-9a-zA-Z-]+(/wp-(content|admin|includes).*) $1 last;
	rewrite ^/[_0-9a-zA-Z-]+(/.*\.php)$ $1 last;
}

In addition, I don't believe the rewrite rules take into account single sites installs in subdirectories. I've not tested it. We may need to include the base subdirectory (eg. /foo for a site at example.com/foo at the start of each rewrite rule as nginx doesn't have a RewriteBase equivalent.

  • Cc matt@… added

Multisite:

In the ms-files directive:

$nginx_config_file .= 'rewrite ^/' . ( $subdomain_install ? '' : '([_0-9a-zA-Z-]+/)?' ) . 'files/(.+) /wp-includes/ms-files.php?file=$2 last;' . "\n\n"; 

$2 should be $1 if $subdomain_install is true (since there isn't the subdomain regex capture group, then), something like like so:

$nginx_config_file .= 'rewrite ^/' . ( $subdomain_install ? '' : '([_0-9a-zA-Z-]+/)?' ) . 'files/(.+) /wp-includes/ms-files.php?file=' . ( $subdomain_install ? '$2' : '$1 ) . ' last;' . "\n\n"; 

Aside from that, these directives work correctly as far as I can tell for multisite installs.

Single site installed in a subdirectory

As you observed, these rules do not take into account a single site which is installed in a subdirectory (WP_SITEURL and WP_HOME both 'http://example.com/blog', or WP_SITEURL 'http://example.com/blog/wp' and WP_HOME 'http://example.com/blog'). Adding the base subdirectory to the location directive and the index.php part of try_files works fine:

	location /blog {
		try_files $uri $uri/ /blog/index.php$is_args$args;
	}
  • Cc justin@… added
  • Cc jk@… added

This may be out of the scope of this ticket, but what about including external rewrite rules (eg those added via $wp_rewrite->add_external_rule or $wp_rewrite->add_rule) in the nginx configuration display? Right now those rewrite rules only make their way to .htaccess files... it would be great to include those rules for nginx users.

comment:14 in reply to: ↑ 13   nacin10 months ago

Replying to johnkleinschmidt:

This may be out of the scope of this ticket, but what about including external rewrite rules (eg those added via $wp_rewrite->add_external_rule or $wp_rewrite->add_rule) in the nginx configuration display? Right now those rewrite rules only make their way to .htaccess files... it would be great to include those rules for nginx users.

Are the regex syntaxes identical? Should be noted that A) these don't work for IIS either, and B) using external rewrite rules is very rare and should be used sparingly as it is.

I believe the regex syntaxes are identical; though there may be cases they are not. Though ideally there would be use cases for performance reasons where you would want to encapsulate a rewrite rule in a location. eg:

#Apache rewrite rule:
RewriteRule ^/news$ /blog/category/correspondents

#Nginx rewrite rule
rewrite ^/news$ /blog/category/correspondents last;

#Better performing nginx rewrite rule:
location ^~ /news {
    rewrite ^ /blog/category/correspondents last;
}

I've used that extension, and it does what it says on the tin with no apparent problem.

  • Cc Brian@… added

I think what is listed in this ticket is fine, but I want to raise awareness of the dangers of the try_files simply tossing all traffic that ends in .php over to be processed by fastcgi/whatever.
In some configurations a constructed url along the lines of :
http://example.com/wp-content/uploads/2012/1/1/notreallya.jpg/.php
will allow the file notreallya.jpg to be sent to the php engine for processing. In that way a php file can be uploaded as a .jpg and then executed.

That's described here: http://forum.nginx.org/read.php?2,124297,page=1

Also I haven't seen anyone reference the official pange nginx has describing the configuration of WordPress:
http://wiki.nginx.org/Wordpress

That config works quite well.

Last edited 10 months ago by brianlayman (previous) (diff)
  • Milestone changed from 3.5 to Future Release

There doesn't appear to be a consensus on this yet, and I'm weary of going forward especially given potential security gotchas.

I am thinking that we should solidify a Codex page (similar to how nginx has one for us) that is our definitive, official, suggested best practice for both single-site and multisite. Once we get there, we can go forward here.

I really, really want this, but I don't think it is something we can rush.

+1, but we do need to be careful about this, since cgi.fix_pathinfo has been known to cause security issues.

Yeah as Nacin said above we definitely need to get a solid consensus in place on the Codex page before this goes in.

  • Cc dcowgill@… added
  • Cc bronson@… added
Note: See TracTickets for help on using tickets.