Opened 21 months ago
Last modified 6 weeks ago
#18852 accepted enhancement
Nginx rewrite rules
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| 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)
Change History (26)
johnbillion
— 21 months ago
comment:1
johnbillion
— 21 months ago
- Keywords has-patch added
comment:3
dd32
— 21 months ago
- 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
johnbillion
— 21 months ago
comment:4
johnbillion
— 21 months ago
Updated patch. Slim down rewrite rules from dd32 and tweak permalink screen message.
comment:5
nacin
— 12 months ago
- 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
devesine
— 12 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
johnbillion
— 12 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).
comment:8
johnbillion
— 12 months ago
Also, the nginx page on Codex is a mess.
johnbillion
— 12 months ago
comment:9
johnbillion
— 12 months ago
- 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.
comment:10
mattrude
— 12 months ago
- Cc matt@… added
comment:11
devesine
— 12 months ago
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;
}
comment:12
devesine
— 12 months ago
- Cc justin@… added
comment:13
follow-up:
↓ 14
johnkleinschmidt
— 11 months ago
- 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
nacin
— 11 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.
comment:15
johnkleinschmidt
— 11 months ago
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;
}
comment:16
nacin
— 11 months ago
More food for thought: http://wordpress.org/extend/plugins/nginx-compatibility/
comment:17
devesine
— 11 months ago
I've used that extension, and it does what it says on the tin with no apparent problem.
comment:18
brianlayman
— 11 months ago
- 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/notrealla.jpg/.php
will allow the file notrealla.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 configuration WordPress:
http://wiki.nginx.org/Wordpress
That config works quite well.
comment:19
nacin
— 9 months ago
- 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.
comment:20
rmccue
— 7 months ago
+1, but we do need to be careful about this, since cgi.fix_pathinfo has been known to cause security issues.
comment:21
johnbillion
— 7 months ago
Yeah as Nacin said above we definitely need to get a solid consensus in place on the Codex page before this goes in.
comment:22
dcowgill
— 3 months ago
- Cc dcowgill@… added
comment:23
sennza
— 6 weeks ago
- Cc bronson@… 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.