WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 4 weeks ago

#20746 reopened defect (bug)

Accessing non-existing theme folder in Network install gives 500 error

Reported by: arkimedia Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3.2
Component: Rewrite Rules Keywords: needs-testing needs-patch
Focuses: multisite Cc:

Description

Accessing non-existing theme folder in Network install gives 500 error and following error in error log: Request exceeded the limit of 10 internal redirects due to probable configuration error.

Attachments (2)

20746.patch (648 bytes) - added by thomaswm 4 months ago.
Fix .htaccess by adding additional RewriteCond
20746.1.patch (1.0 KB) - added by thomaswm 4 months ago.
Alternative approach: Remove question mark from rewrite rule

Download all attachments as: .zip

Change History (32)

comment:1 @SergeyBiryukov3 years ago

  • Component changed from Rewrite Rules to Multisite

comment:2 @arkimedia3 years ago

Addition: The problem occurs at least in subdirectory installs. I have tested also with the fresh network install with default .htaccess rules and no plugins installed and I got same response from the server.

comment:3 @wonderboymusic3 years ago

  • Keywords reporter-feedback added

Example URL?

comment:4 @wpmuguru3 years ago

Any instances I've seen of this were a result of rewrite rules in the .htaccess. A network upgraded from WordPress MU might exhibit this behavior.

comment:5 @jeremyfelt2 years ago

  • Keywords reporter-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Sounds specific to server/site configuration. Closing, but feel free to reopen if it can be reproduced.

comment:6 @jrfoell2 years ago

  • Cc justin@… added
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Here's how to reproduce:

  1. Create a sub-directory multi-site installation per normal
  2. Try to load a fictitious file from wp-content such as:

http://example.com/wp-content/uploads/2013/10/test.jpg

  1. Observe "500 Internal Server Error" due to "Request exceeded the limit of 10 internal redirects due to probable configuration error."

I believe this is because after this rule is applied:

RewriteRule ^([_0-9a-zA-Z-]+/)?(wp-(content|admin|includes).*) $2 [L]

Apache tries to rewrite (internally) from "wp-content/uploads/2013/10/test.jpg" to "/wp-content/uploads/2013/10/test.jpg" with a beginning slash. Then the aforementioned rule gets applied again because it matches - creating a loop.

Expected result: 404 Page

Last edited 2 years ago by SergeyBiryukov (previous) (diff)

comment:7 @jeremyfelt2 years ago

  • Milestone set to Awaiting Review

comment:8 @mordauk22 months ago

I've seen this happen on 3 different multi-site installs, all running on different servers / hosts.

comment:9 @mordauk22 months ago

  • Cc pippin@… added

comment:10 @ryanduff22 months ago

  • Cc ryan@… added

comment:11 @mordauk22 months ago

Ron Rennick has suggested that adding RewriteCond %{REQUEST_URI} !^wp-(content|admin|includes).*$ above RewriteRule ^([_0-9a-zA-Z-]+/)?(wp-(content|admin|includes).*) $2 [L] might solve the issue.

I'm running the modified version on two live sites now but haven't confirmed for sure if it resolves it or not.

comment:12 @jeremyfelt20 months ago

  • Component changed from Multisite to Rewrite Rules
  • Focuses multisite added

comment:13 @jrfoell19 months ago

I confirmed that the Ron Rennick / Pippin suggestion did not work for me on Apache 2.4.6 (added to a fresh WP 3.8.1 multisite subfolder install). The following seems to work, but it is not elegant and I haven't determined if it has other adverse effects:

RewriteEngine On
RewriteBase /
RewriteRule ^index\.php$ - [L]

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

RewriteCond %{REQUEST_FILENAME} -f [OR]
RewriteCond %{REQUEST_FILENAME} -d
RewriteRule ^ - [L]
# start new
RewriteCond %{REQUEST_URI} ^/wp-(content|admin|includes).*$
RewriteRule ^ - [S=2]
# end new
RewriteRule ^([_0-9a-zA-Z-]+/)?(wp-(content|admin|includes).*) $2 [L]
RewriteRule ^([_0-9a-zA-Z-]+/)?(.*\.php)$ $2 [L]
RewriteRule . index.php [L]`

The key is really noticing the difference between "wp-content/uploads/2013/10/test.jpg" and "/wp-content/uploads/2013/10/test.jpg" - with or without a beginning slash - so Apache doesn't go looking for a file outside of the webroot.

Some of this seems to go against the documentation regarding "Per-directory Rewrites" found here: http://httpd.apache.org/docs/current/mod/mod_rewrite.html#rewriterule specifically the second-to-last point:

The removed prefix always ends with a slash, meaning the matching occurs against a string which never has a leading slash. Therefore, a Pattern with ^/ never matches in per-directory context.

But I could be reading it wrong.

Last edited 19 months ago by jrfoell (previous) (diff)

comment:14 @wpmuguru19 months ago

The key is really noticing the difference between "wp-content/uploads/2013/10/test.jpg" and "/wp-content/uploads/2013/10/test.jpg" - with or without a beginning slash - so Apache doesn't go looking for a file outside of the webroot.

If you do not have the RewriteBase / at the beginning of your .htaccess (per standard WP .htaccess) then

RewriteCond %{REQUEST_URI} ^/wp-(content|admin|includes).*$

would work. If you do have it then the requested URL would need to be http://example.com//wp-content.... for the rewrite condition to be met.

Since the standard rewrite rules for the .htaccess are for a rewrite base of / (or /some-path/) then the correct rule is

RewriteCond %{REQUEST_URI} ^wp-(content|admin|includes).*$

If you are missing the RewriteBase that would explain why the rule Pippin posted above didn't work for you.

comment:15 follow-up: @jrfoell18 months ago

Thanks Ron... I amended my comment to show the full .htaccess I'm using.

Were you able to confirm that your suggested change (from comment 11) fixes the issue with a new WP 3.8 multi-site subfolder installation?

comment:16 in reply to: ↑ 15 @wpmuguru18 months ago

Replying to jrfoell:

Thanks Ron... I amended my comment to show the full .htaccess I'm using.

In the full htaccess above the rewrite condition

RewriteCond %{REQUEST_URI} ^/wp-(content|admin|includes).*$`

will only be met if the requested path begins with //. The RewriteBase directive tells the rewrite engine to start rewriting after the base value.

Secondly, that .htaccess isn't what I suggested to Pippin. What I suggested was

RewriteCond %{REQUEST_URI} !^wp-(content|admin|includes).*$
RewriteRule ^([_0-9a-zA-Z-]+/)?(wp-(content|admin|includes).*) $2 [L]

Looking at the original rule it would probably be easier to remove the ? from the original rule

RewriteRule ^[_0-9a-zA-Z-]+/(wp-(content|admin|includes).*) $1 [L]
Last edited 18 months ago by wpmuguru (previous) (diff)

comment:17 @jrfoell18 months ago

I verified that the first (original) suggestion from ticket:20746#comment:11 (reiterated in ticket:20746#comment:16) still produces a 500 error on a sub-folder multi-site.

The second suggestion from ticket:20746#comment:16 does seem to work, but I have not verified if it has any other adverse effects:

RewriteRule ^[_0-9a-zA-Z-]+/(wp-(content|admin|includes).*) $1 [L]

comment:18 @lisota5 months ago

I can confirm this bug in subfolder Multisite in WP 4.1.1. Using the standard .htaccess rules for subfolder multisite.

Would be great if someone can get a fix pushed into the core .htaccess rules. I can't imagine that these redirect loops wouldn't have at least some negative impact on Apache performance.

@thomaswm4 months ago

Fix .htaccess by adding additional RewriteCond

@thomaswm4 months ago

Alternative approach: Remove question mark from rewrite rule

comment:19 @thomaswm4 months ago

  • Keywords has-patch needs-testing added

I can also confirm this bug in my setup (WordPress Multisite, sub-directory install).

I've added patches for both solutions that were suggested in this ticket.

comment:20 @slackbot3 months ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.

comment:21 follow-ups: @jorbin3 months ago

This needs some further testing, along with some clearer reproduction steps. We need to be very careful to not break any of the existing behavior.

When this is fixed, we should also do a make/core post with the changes since this will only change the rewrite rules for new sites.

comment:22 in reply to: ↑ 21 @lisota3 months ago

Replying to jorbin:

This needs some further testing, along with some clearer reproduction steps. We need to be very careful to not break any of the existing behavior.

When this is fixed, we should also do a make/core post with the changes since this will only change the rewrite rules for new sites.

We are experiencing this issue with a Multisite subdirectory install, with the same reproduction steps in https://core.trac.wordpress.org/ticket/20746#comment:11. Our error logs fill daily with "Request exceeded the limit of 10 internal redirects due to probable configuration error."

Our installation was originally a single site that was upgraded to multisite. Upgrading to multisite caused this error on our site, though multiple commenters above describe this issue on fresh multisite installs as well.

comment:23 in reply to: ↑ 21 ; follow-up: @wpmuguru3 months ago

Replying to jorbin:

This needs some further testing, along with some clearer reproduction steps. We need to be very careful to not break any of the existing behavior.

I believe you can reproduce this by requesting any non-existant file from a theme folder in a sub folder install

http://example.com/subsite/wp-content/themes/twentyfifteen/image-that-does-not-exist.jpg

When this is fixed, we should also do a make/core post with the changes since this will only change the rewrite rules for new sites.

This change would only affect new installs. .htaccess is not updated on WP updates.

comment:24 in reply to: ↑ 23 @thomaswm3 months ago

Replying to wpmuguru:

Replying to jorbin:

This needs some further testing, along with some clearer reproduction steps. We need to be very careful to not break any of the existing behavior.

I believe you can reproduce this by requesting any non-existant file from a theme folder in a sub folder install

http://example.com/subsite/wp-content/themes/twentyfifteen/image-that-does-not-exist.jpg

That's right. But not only the themes folder is affected. This problem occurs for any non-existent file in wp-content and any subfolder of wp-content.

I tried to explain this in a bit more detail here.

comment:25 @jeremyfelt3 months ago

#28164 was marked as a duplicate.

comment:26 @jeremyfelt3 months ago

#30588 was marked as a duplicate.

comment:27 @lisota4 weeks ago

How do we get a core committer to review and elevate the proposed patch to a release milestone?

comment:28 follow-up: @jeremyfelt4 weeks ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to Future Release

Hey everyone, thanks for hanging in there. I've been able to confirm and test the following.

In 4.2:

  • http://foo.bar/subsite/non-dir/ responds as a WordPress 404.
  • http://foo.bar/subsite/wp-content/non-dir/ responds as an Apache 500.
  • http://foo.bar/subsite/wp-content/non-file.txt responds as an Apache 404.
  • http://foo.bar/subsite/wp-content/non-dir/non-file.txt responds as an Apache 500.

After modifying the wp-* line in htaccess to the suggested RewriteRule ^[_0-9a-zA-Z-]+/(wp-(content|admin|includes).*) $1 [L]

  • http://foo.bar/subsite/non-dir/ responds as a WordPress 404.
  • http://foo.bar/subsite/wp-content/non-dir/ responds as a WordPress 404.
  • http://foo.bar/subsite/wp-content/non-file.txt responds as an WordPress 404.
  • http://foo.bar/subsite/wp-content/non-dir/non-file.txt responds as a WordPress 404.

How I would expect things to respond:

  • http://foo.bar/subsite/non-dir/ responds as a WordPress 404.
  • http://foo.bar/subsite/wp-content/non-dir/ responds as Apache 403 forbidden.
  • http://foo.bar/subsite/wp-content/non-file.txt responds as Apache 404 not found.
  • http://foo.bar/subsite/wp-content/non-dir/non-file.txt responds as Apache 404 not found.

Overall - I think it would be appropriate to avoid loading the entire WordPress 404 process for missing static files. My Apache rewrite memories have mostly been lost to Nginx rules. Any ideas on how that could be handled?

comment:29 @wpmuguru4 weeks ago

I think it would be appropriate to avoid loading the entire WordPress 404 process for missing static files.

Agreed. However, the web server has no way of knowing whether /subsite/non-dir/ is a request for a file system folder or a post/page on the subsite. If the folder does not exist then the request has to be passed to WP for WP to determine if the content exists.

Last edited 4 weeks ago by wpmuguru (previous) (diff)

comment:30 in reply to: ↑ 28 @thomaswm4 weeks ago

Replying to jeremyfelt:

Overall - I think it would be appropriate to avoid loading the entire WordPress 404 process for missing static files. My Apache rewrite memories have mostly been lost to Nginx rules. Any ideas on how that could be handled?

Look at these directives in the current version of .htaccess:

RewriteCond %{REQUEST_FILENAME} -f [OR]
RewriteCond %{REQUEST_FILENAME} -d
RewriteRule ^ - [L]

They basically say: "If the user has requested an existing file or folder, stop the URL rewriting (and send the requested file or folder to the user)."

So, if there's a file or folder that matches the path and filename in the request URI, the following rewrite rules will not be evaluated which means that in all following rewrite rules, we can assume that there's no file or folder that matches the request URI.

After those directives that I cited above, we could add another rule which then handles the case that the user requested a file or subfolder of wp-content which does not exist.

RewriteRule ^wp-content/(.+) - [R=404]

This would make Apache return an error 404 whenever the requested URL starts with wp-content/ and no file or folder with matching name was found.

Or, if you want to return different HTTP status codes for non-existent files and non-existent folders, then we can go with these rules:

RewriteRule ^wp-content/(.+)/$ - [R=403]
RewriteRule ^wp-content/(.+) - [R=404]

Here, the first rule checks that the request URI starts with wp-content/ and ends with a trailing slash /. If that's the case, Apache returns an error 403, otherwise an error 404.

Last edited 4 weeks ago by thomaswm (previous) (diff)
Note: See TracTickets for help on using tickets.