Opened 13 years ago
Closed 9 years ago
#18852 closed enhancement (fixed)
Nginx rewrite rules
Reported by: | johnbillion | Owned by: | johnbillion |
---|---|---|---|
Milestone: | 4.4 | Priority: | low |
Severity: | minor | Version: | 3.3 |
Component: | Rewrite Rules | Keywords: | needs-patch |
Focuses: | administration | Cc: |
Description (last modified by )
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 (43)
#3
@
13 years 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
#4
@
13 years ago
Updated patch. Slim down rewrite rules from dd32 and tweak permalink screen message.
#5
@
12 years 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.
#6
follow-up:
↓ 7
@
12 years 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?
#7
in reply to:
↑ 6
@
12 years 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).
#9
@
12 years 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.
#11
@
12 years 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; }
#13
follow-up:
↓ 14
@
12 years 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.
#14
in reply to:
↑ 13
@
12 years 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.
#15
@
12 years 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; }
#16
@
12 years ago
More food for thought: http://wordpress.org/extend/plugins/nginx-compatibility/
#17
@
12 years ago
I've used that extension, and it does what it says on the tin with no apparent problem.
#18
@
12 years 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/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.
#19
@
12 years 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.
#20
@
12 years ago
+1, but we do need to be careful about this, since cgi.fix_pathinfo
has been known to cause security issues.
#21
@
12 years ago
Yeah as Nacin said above we definitely need to get a solid consensus in place on the Codex page before this goes in.
#25
@
11 years ago
- Cc mike@… added
I think the importance of this has escalted since this ticket was opened in 2011.
Since Jeremy Felt made nginx easy & standard in VVV, https://github.com/10up/varying-vagrant-vagrants, I've been talking to more people about it & almost every major WP dev shop is running nginx. Most of us have our own configs & a patchwork of things we do to support it, so it might be time to put more engineering into making it more standard.
Worth noting that on Top 1,000 sites, nginx has surpassed Apache in popularity: http://w3techs.com/blog/entry/nginx_just_became_the_most_used_web_server_among_the_top_1000_websites
---
ADDITIONAL REQUIREMENTS?
Is it worth extending this ticket, or adding a separate one for WP_Rewrite's non_wp_rules()?
If we're displaying nginx rules in the UX, then that method should alert in some way too. Or, better, get refactored to write nginx rules too.
What if we implemented something like the $scheme parameter that handles HTTP/HTTPS URLs now? That would enable plugins to add more-extensive nginx rule support, without the need to debug & maintain this in core (yet).
(Plus, plugins have a faster|easier update cycle in the event new rules are needed to fix security issues)
#26
follow-up:
↓ 32
@
11 years ago
It would be great to see Nginx pushed a bit more alongside Apache in the WordPress workflow.
I do think a lot of the value will come from having a well vetted config available before WordPress is installed. Either as part of the Codex or, better, as a default conf file of sorts that can be moved on the server with instruction.
It seems somewhat more of a chicken/egg thing than Apache has been. Your ideal PHP-FPM configuration and basic handling of static assets will probably already be setup in order for you to get through the installation. Once installed, offering a more complete setup may be helpful, though I'd be interested in seeing examples of the different configs that would come out of that process.
The nginx-wp-common.conf
that we've pieced together in VVV has served pretty well thus far, though I'm sure there's plenty of room for improvement. Ignoring the Vagrant/VM specific directives like gzip off;
, it may be a good starting point for discussion and I'm willing to make tweaks to that default config as needed if it can provide a good testing ground for core.
My overall opinion is probably that core can help guide correctly on Nginx, but will probably never be able to automatically solve the things that the ease of .htaccess
just working allows for.
From the perspective of solving annoyances, it would be nice to have the $is_nginx
global so that WordPress can report its "got_rewrite" status properly without a plugin.
#27
@
11 years ago
I've split out the support for URL rewrites in Nginx to #25098.
I'll be refreshing the patch for this ticket at some point over the next few days.
#28
@
11 years ago
- Milestone changed from Future Release to 3.8
Moving to 3.8 so we can get it in early in the cycle. Patch needs a refresh.
#32
in reply to:
↑ 26
;
follow-up:
↓ 36
@
11 years ago
I'd like to start working on this again, since I think we can make some traction here.
Replying to jeremyfelt:
The
nginx-wp-common.conf
that we've pieced together in VVV has served pretty well thus far, though I'm sure there's plenty of room for improvement. Ignoring the Vagrant/VM specific directives likegzip off;
, it may be a good starting point for discussion and I'm willing to make tweaks to that default config as needed if it can provide a good testing ground for core.
I think this is a great start, and matches the standard config I use too.
Going over individual bits of the config:
try_files $uri $uri/ /index.php?$args;
That should be /index.php$is_args$args
instead ($is_args = "?"
if $args
is non-empty, or ""
otherwise), per best practices.
# Add trailing slash to */wp-admin requests. rewrite /wp-admin$ $scheme://$host$uri/ permanent;
Do we need this? :)
# 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 in a subdirectory '.../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; }
I'd separate this into an nginx-wp-multisite.conf
personally.
# The amount of time for upstream to wait for a fastcgi process to send data. # We keep this *extremely* high so that one can be lazy when remote debugging. fastcgi_read_timeout 3600s; # Buffer size for reading the header of the backend FastCGI process. # This defaults to the value of a single fastcgi_buffers, so does not # need to be specified in our case, but it's good to be explicit. fastcgi_buffer_size 128k; # The number and size of the buffers into which the reply from the FastCGI # process in the backend is read. # # 4 buffers at 128k means that any reply by FastCGI greater than 512k goes # to disk and replies under 512k are handled directly in memory. fastcgi_buffers 4 128k;
I think we should avoid duplicating fastcgi_params
content where possible, and this stuff should definitely live there. (Also, interesting note, you've got mixed spaces and tabs in the file :) )
fastcgi_pass php;
(I'm not sure how we can *not* hardcode this, so this is probably a good idea. Anyone using other FPM setups can obviously roll their own config.)
#33
follow-up:
↓ 34
@
11 years ago
Bear in mind we'll only be outputting rules that relate to rewrites (see 18852.3.patch for example). Any other config such as fastcgi-pass
are not part of the rules that WordPress will generate.
I think @nacin was going to see if he could speak to a sysadmin at wordpress.com to review the rules in the most recent patch. Did you get anywhere with that nacin?
#34
in reply to:
↑ 33
@
11 years ago
Replying to johnbillion:
Bear in mind we'll only be outputting rules that relate to rewrites (see 18852.3.patch for example). Any other config such as
fastcgi-pass
are not part of the rules that WordPress will generate.
If we do that, what's actually needed? Just using index.php$is_args$args
as the final try_files
?
We might be better off pointing them to the Codex in that case. The text in attachment:18852.3.patch is a little misleading; on most systems, the location block should go into the site-specific config file rather than the global nginx.conf
.
#35
@
11 years ago
- Milestone changed from 3.8 to Future Release
- Severity changed from normal to minor
I think @nacin was going to see if he could speak to a sysadmin at wordpress.com to review the rules in the most recent patch. Did you get anywhere with that nacin?
I didn't get a chance to yet.
I think the try_files line + a link to the codex might be the most for what we want to do.
#36
in reply to:
↑ 32
@
11 years ago
Replying to rmccue:
# Add trailing slash to */wp-admin requests. rewrite /wp-admin$ $scheme://$host$uri/ permanent;Do we need this? :)
It's good to have and I think I finally figured out why. :)
For unauthenticated users:
http://network.com/site1/wp-admin/
results in the properredirect_to
query varhttp://network.com/site1/wp-admin
dropssite1
from theredirect_to
query var
This is more annoying than anything else. Without digging too much, we currently do $path = str_replace ( '/wp-admin/', '/', $path );
in 3.8.1 ms-settings. Something similar is done in trunk via #27003.
Edit: Actually, some or most of that is wrong.
#37
follow-up:
↓ 38
@
11 years ago
It looks like it never got noted in this ticket, but the consensus after a couple of discussions on IRC was that displaying Nginx rewrites rules here would be potentially confusing for users, especially given that Nginx conf files can be (and usually are) split up depending on whether you've got virtual hosts configured, whether your WordPress configuration is in separate files, etc etc etc.
We should just display a link to the Nginx page on the Codex here in place of the .htacess
rules when the site is running Nginx.
#25098 took care of the actual Nginx support.
#38
in reply to:
↑ 37
@
11 years ago
Replying to johnbillion:
We should just display a link to the Nginx page on the Codex here in place of the
.htacess
rules when the site is running Nginx.
The Codex URL is http://codex.wordpress.org/Nginx
And if we ever do decide to display the rules here, we'll need to change the file header notes added in [26931] for the $is_ngninx
global.
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.