Opened 3 years ago
Closed 3 years ago
#12136 closed defect (bug) (fixed)
Improve MS rewrite rules
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Priority: | normal | Milestone: | 3.0 |
| Component: | Rewrite Rules | Version: | |
| Severity: | normal | Keywords: | multisite has-patch |
| Cc: |
Description
I've been looking through the MS rewrite rules, trying to improve performance where possible and possibly coming up with a rule that prevents the need to hard-code wp-content (or, eventually, a custom content directory).
Patch attached what I've simplified so far, which is to reduce the uploaded files and wp-admin trailing slash rules each to a single line, and modify slug handling a bit. Tested on a production subdirectory install.
Attachments (2)
Change History (17)
I don't have a problem with using the more specific regex instead of [/]+. Does the rest of it look good?
- Resolution set to fixed
- Status changed from new to closed
This one has been done, I believe. We had discussed in IRC & proceeded.
- Resolution fixed deleted
- Status changed from closed to reopened
None of these changes were applied. I think there is still some potential for optimization but otherwise we should close as wontfix.
- Resolution set to wontfix
- Status changed from reopened to closed
I did a bit of testing and changing the rewrite rules to those in the patch allowed me to access the admin area from a bogus path
ex. http://domain.com/none-existant-path/wp-admin let me access the admin area at http://domain.com/wp-admin.
Forgot to add - [13630] brought the allowed blognames in line with the rewrite rules.
- Resolution wontfix deleted
- Status changed from closed to reopened
Going to re-open this for some incremental improvements. The bogus path works on the current rewrite rules as well.
There are numerous improvements that seem okay to me. In order:
- Remove RewriteRule ^(.*/)?files/$ index.php [L] and change files/(.*) in the second rule to files/(.+).
- Remove the condition on the wp-admin trailing slash rule by improving the rule itself.
- Convert the files and wp-admin rules from ^(.*/)? to ^([_0-9a-zA-Z-]+/)?, to match the other rules and current restrictions.
- Remove the wp-content/plugins restriction on files, as the conditions for which it was added shouldn't apply under a properly constructed rule.
This has a new patch. Also includes a small optimization from Denis in #12497 (closed as a duplicate).
The issue with wp-content/plugins is that we didn't restrict the initial files rewrite to only one directory deep. Thus it matched:
^blog/files/* ^blog/blah/files/* ^wp-content/plugins/plugin_name/files/*
And that's why we excluded wp-content/plugins.
The proper fix would have been to restrict the depth, via ^([^/]+/)?, which would only have matched the first example above. A desire to instead whitelist directories via _0-9a-zA-Z- also restricts it to a single directory as it prevents a slash, and thus rendering the wp-content/plugins check useless.
Food for thought: especially since .htaccess is generated dynamically, should we strip the directory matching when we're on a subdomain install? Seems like it would greatly simplify the rules (unless I'm mistaken, we'd end up with just ms-files and the standard WP rules).
comment:10
jamescollins — 3 years ago
Also, WPMU's current URL rewriting rules don't work with URLs that contain ".php". I raised this problem a few months ago (http://trac.mu.wordpress.org/ticket/1167), but a fix hasn't been applied yet.
I had a go at trying to fix it (see the wpmu trac ticket above), but my mod_rewrite knowledge isn't quite up to scratch.
comment:11
nacin — 3 years ago
The 3.0 rules are improved over the MU rules, and only restrict .php files in the root directory. They still restrict any files in wp-admin/wp-includes/wp-content, but the file must not exist. This doesn't solve your problem, but I wanted to point it out.
What could solve the problem for subdomain installs would be to scrap subdirectory-specific rules when installing a subdomain-based network.
comment:12
nacin — 3 years ago
comment:13
nacin — 3 years ago
comment:14
wpmuguru — 3 years ago
This rewrite rule should be removed to improve the site rewrite efficiency:
RewriteRule ^index\.php$ - [L]
Whether or not it improves the efficiency of a request for index.php, it decreases the efficiency of all other requests (wp-content, wp-admin, wp-includes, css, js, tags, categories, archives, attachments, feeds, comments, etc.). Requests for index.php comprise only a small number of the requests on any site.
comment:15
wpmuguru — 3 years ago
- Resolution set to fixed
- Status changed from reopened to closed

The rewrite base is / or /path/ so in rewriting a request for /path/ron/wp-content/themes/... apache is running the regex on ron/wp-content/themes/... not /ron/wp-content/themes/....
The more specific regex limits the rewrites to what is possible as a blog name and probably provides a bit of security.