Make WordPress Core

Opened 12 years ago

Last modified 6 months ago

#20746 reopened defect (bug)

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

Reported by: arkimedia's profile arkimedia Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3.2
Component: Rewrite Rules Keywords: needs-testing has-patch dev-feedback 2nd-opinion
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 (3)

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

Download all attachments as: .zip

Change History (67)

#1 @SergeyBiryukov
12 years ago

  • Component changed from Rewrite Rules to Multisite

#2 @arkimedia
12 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.

#3 @wonderboymusic
12 years ago

  • Keywords reporter-feedback added

Example URL?

#4 @wpmuguru
12 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.

#5 @jeremyfelt
11 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.

#6 @jrfoell
11 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 11 years ago by SergeyBiryukov (previous) (diff)

#7 @jeremyfelt
11 years ago

  • Milestone set to Awaiting Review

#8 @mordauk
11 years ago

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

#9 @mordauk
11 years ago

  • Cc pippin@… added

#10 @ryanduff
11 years ago

  • Cc ryan@… added

#11 @mordauk
11 years 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.

#12 @jeremyfelt
11 years ago

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

#13 @jrfoell
11 years 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 11 years ago by jrfoell (previous) (diff)

#14 @wpmuguru
11 years 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.

#15 follow-up: @jrfoell
11 years 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?

#16 in reply to: ↑ 15 ; follow-up: @wpmuguru
11 years 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 11 years ago by wpmuguru (previous) (diff)

#17 @jrfoell
11 years 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]

#18 @lisota
10 years 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.

@thomaswm
9 years ago

Fix .htaccess by adding additional RewriteCond

@thomaswm
9 years ago

Alternative approach: Remove question mark from rewrite rule

#19 @thomaswm
9 years 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.

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


9 years ago

#21 follow-ups: @jorbin
9 years 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.

#22 in reply to: ↑ 21 @lisota
9 years 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.

#23 in reply to: ↑ 21 ; follow-up: @wpmuguru
9 years 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.

#24 in reply to: ↑ 23 @thomaswm
9 years 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.

#25 @jeremyfelt
9 years ago

#28164 was marked as a duplicate.

#26 @jeremyfelt
9 years ago

#30588 was marked as a duplicate.

#27 @lisota
9 years ago

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

#28 follow-ups: @jeremyfelt
9 years 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?

#29 @wpmuguru
9 years 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 9 years ago by wpmuguru (previous) (diff)

#30 in reply to: ↑ 28 @thomaswm
9 years 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 9 years ago by thomaswm (previous) (diff)

#31 @conner_bw
9 years ago

There are two ways to set up WP Multisite (See https://codex.wordpress.org/Multisite_Network_Administration#.htaccess_and_Mod_Rewrite)

+ Subfolder
+ SubDomain

The rule changed in this TRAC ticket is for SubFolder. Ok. Cool. But how does one change the rule for SubDomain which manifests the same behaviour? AKA:

RewriteRule ^(wp-(content|admin|includes).*) $1 [L]

Example server error with existing WP rule:

+ http://connerbw.pressbooks.com/wp-content/themes/missing/404.css

#32 @conner_bw
9 years ago

Ok. I messed around with this. I think I have something.

Instead of changing the rewrite rules, Change:

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

To:

    RewriteCond %{ENV:REDIRECT_STATUS} 200 [OR]
    RewriteCond %{REQUEST_FILENAME} -f [OR]
    RewriteCond %{REQUEST_FILENAME} -d
    RewriteRule ^ - [L]

This works by checking internal Apache variable %{ENV:REDIRECT_STATUS}. This variable is empty at the start of rewrite module but is set to 200 when first successful internal rewrite happens. This above condition says bail out of further rewrites after first successful rewrite and stops looping.

#33 @Maigret
9 years ago

Hi everyone,

I can confirm this behavior on subfolder fresh new multisite installation. The fix mentioned above (removing interrogation point) is working fine on our dedicated server.

Let me know if I can help.

#34 @jrfoell
8 years ago

Would be awesome if this could get fixed as part of the 4.6 mega-bugfix release :)

#35 @arkimedia
8 years ago

I think that this bug is a (small) security issue too, because this makes DDOS attacks easier:

  1. These requests never go to the cache
  2. With random http request attacker could cause 10 internal redirects and 500 error in the server
  3. Every error is logged into the error log, which also stresses the server

@thomaswm
8 years ago

#36 in reply to: ↑ 28 @thomaswm
8 years ago

  • Keywords has-patch added; needs-patch removed

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?

I've come up with a pretty simple solution to this. See 20746.2.patch. I've only added one line to my .htaccess file:

RewriteRule ^wp-(admin|content|includes) - [L]

That line ensures that request URIs starting with /wp-content, /wp-admin or /wp-includes are handled by Apache without loading WordPress.

After adding that one line to my .htaccess file, I see the following behaviour:

  • For http://foo.bar/subsite/non-dir/, the server responds with a WordPress 404.
  • For the following three requests, the server responds with an Apache 404 Not Found.
    • http://foo.bar/subsite/wp-content/non-dir/
    • http://foo.bar/subsite/wp-content/non-file.txt
    • http://foo.bar/subsite/wp-content/non-dir/non-file.txt

#37 follow-up: @rob006
8 years ago

Showing ugly Apache's 404 error page is unacceptable in most of the cases, especially on subdirectory multisite installations.

#38 in reply to: ↑ 37 ; follow-up: @wpmuguru
8 years ago

Replying to rob006:

Showing ugly Apache's 404 error page is unacceptable in most of the cases, especially on subdirectory multisite installations.

This ticket is for static resources (css, js, etc.) that don't exist but are referenced in the source of some or all pages of the site vs URLs that a visitor would be visiting. In the discussion here people are posting the URLs for testing/confirmation purposes.

#39 in reply to: ↑ 38 @rob006
8 years ago

Replying to wpmuguru:

This ticket is for static resources (css, js, etc.)

This ticket affects all files in wp-* directories and it could be anything: PHP scripts, images, PDFs or static html pages. If user has invalid/outdated URL, he should get 404 error page in the context of the current site.

Concerns about which error page should be used (Apache vs WP) is completely outside the scope of this ticket - fix should handle 404 error pages in the same way as for non-multisite installations.

#40 @lordspace
8 years ago

I have noticed the same apache error.

Apache redirect loop case
https://example.com/wp-admin/sub-site/

This is working fine.
https://example.com/sub-site/wp-admin/

#41 @danpw
7 years ago

Any updates on this getting an official patch/fixed?

We are noticing it on all multi-site installs. Both subdomain and subdirectory installs version 4.8.2.

#42 @ogalinski
7 years ago

I'm running WP 4.8.2 un multisite (subdomains, standard .htaccess created by WP) and also experiencing strange issues with a theme (Kleo).

#43 in reply to: ↑ 16 ; follow-up: @dchenko
7 years ago

But this does not simply remove the question mark—you're removing the capturing parentheses and changing the $2 to a $1, which makes the string "+/" required as part of the URL in front of wp-* even though it was optional before.

Replying to wpmuguru:

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]

#44 in reply to: ↑ 43 @wpmuguru
7 years ago

Replying to dchenko:

But this does not simply remove the question mark—you're removing the capturing parentheses and changing the $2 to a $1, which makes the string "+/" required as part of the URL in front of wp-* even though it was optional before.

The leading path being optional is the problem the patch is fixing. Apache loops through the rewrite rules until there are no more matches. If the leading path is optional then when the rule matches on the first pass & the leading path is removed, the rule continues to match on subsequent rewrite passes resulting in a rewrite loop.

@jeremyfelt My patch restores the rules to what was merged from WordPress MU about 8 years ago.

#45 @ajoah
7 years ago

I think it has not been said : there is a similare problem for subdomains multisite.

URL to a non existing file inside a subfolder returns a 500 error.

Examples with files with php extension :

example.com/non-dir/non-file.php

Same thing inside non existing theme folder :

example.com/wp-content/themes/non-theme/non-file.php

With other file extension, it depends... :

example.com/non-dir/non-file.txt => returns 404
example.com/wp-content/themes/non-theme/non-file.txt => returns 500, i think it is the same problem as subfolders install

These following urls return a 404 too but not the website 404 (just a 404 page with the text "File not found")

example.com/wp-content/themes/non-file.php
example.com/wp-content/non-file.php

WP 4.9.5

Last edited 7 years ago by ajoah (previous) (diff)

#46 @wpmuguru
7 years ago

@ajoah If you have the rewrite rules for a multisite folder install you have the issue in both single & multisite installs. You can compare your .htaccess to the folder & subdomain rules shown at https://codex.wordpress.org/Multisite_Network_Administration#.htaccess_and_Mod_Rewrite .

#47 @ajoah
7 years ago

@wpmuguru I use the code given in WP admin when i create the network :

https://snag.gy/mJqity.jpg

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

# add a trailing slash to /wp-admin
RewriteRule ^wp-admin$ wp-admin/ [R=301,L]

RewriteCond %{REQUEST_FILENAME} -f [OR]
RewriteCond %{REQUEST_FILENAME} -d
RewriteRule ^ - [L]
RewriteRule ^(wp-(content|admin|includes).*) $1 [L]
RewriteRule ^(.*\.php)$ $1 [L]
RewriteRule . index.php [L]

However i did some mistakes in my tests, so i have updated my previous comment.

#48 @wpmuguru
7 years ago

@ajoah Thanks. I wasn't aware that the probematic rules in this issue were now being included in subdomain installs.

#49 @woorise
5 years ago

Any updates on this? I noticed this issue on version 5.3

#50 @dennismenken
4 years ago

The problem is still there in a subdirectory setup.

Is there any official fix for this?

#51 follow-up: @bananastalktome
4 years ago

  • Keywords dev-feedback 2nd-opinion added

Similar to the comment by @thomaswm, for our subdomain install changing the $1 replacement with a dash for both of the following rules seems to work:

RewriteRule ^(wp-(content|admin|includes).*) - [L]
RewriteRule ^(.*\.php)$ - [L]

I am not sure if this is truly a valid fix, but feel that an Apache generated 404 page rather than WordPress one (while maybe not ideal) is still better than the current long-standing behavior of an Apache generated 500 page and internal redirect loop.

#52 follow-up: @jeremyfelt
4 years ago

#51266 was marked as a duplicate.

#53 in reply to: ↑ 52 @navnathsawant
4 years ago

Replying to jeremyfelt:

#51266 was marked as a duplicate.

@jeremyfelt I agree, #51266 is a duplicate ticket and this ticket too older and based on WordPress older version 3.3.2, but when the WordPress core will fix it. Basically the solutions mentioned above do not work in my case and 500 URL's redirection affecting site performance. I hope you understand my concern and we are using Nginx server.

#54 @pushevs
3 years ago

Any news? I also have this problem.

#55 @wpmuguru
3 years ago

@pushevs The patches for this issue won't fix existing sites. You have to manually update existing sites. See https://core.trac.wordpress.org/ticket/20746#comment:51 for the change you need to make on your site to address it.

#56 @pushevs
3 years ago

Thanks, the new .htaccess file by Thomas works.

https://gist.github.com/JustThomas/141ebe0764d43188d4f2

#57 @mausmalone
3 years ago

I'm reviving this ancient thread because I think I figured out what this bug is and - more importantly - where it comes from. I get the impression that nobody's willing to fix it because they're afraid that it's like this for a reason and, as far as I can tell, it isn't.

Basically the long and short of it is this. This line:

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

can result in a circular redirect in some rare circumstances because ([_0-9a-zA-Z-]+/)? will just keep matching itself over and over again after redirect. Removing the ? fixes it, but why is it there in the first place?

Here's the part of /wp-admin/network.php that outputs a .htaccess file in WordPress 3.4.

<?php
                $htaccess_file = 'RewriteEngine On
RewriteBase ' . $base . '
RewriteRule ^index\.php$ - [L]

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

                if ( ! $subdomain_install )
                        $htaccess_file .= "\n# add a trailing slash to /wp-admin\n" . 'RewriteRule ^([_0-9a-zA-Z-]+/)?wp-admin$ $1wp-admin/ [R=301,L]' . "\n";

                $htaccess_file .= "\n" . 'RewriteCond %{REQUEST_FILENAME} -f [OR]
RewriteCond %{REQUEST_FILENAME} -d
RewriteRule ^ - [L]';

                // @todo custom content dir.
                if ( ! $subdomain_install )
                        $htaccess_file .= "\nRewriteRule  ^[_0-9a-zA-Z-]+/(wp-(content|admin|includes).*) $1 [L]\nRewriteRule  ^[_0-9a-zA-Z-]+/(.*\.php)$ $1 [L]";

                $htaccess_file .= "\nRewriteRule . index.php [L]";

It's clearly a bit messy, so somebody decided that they would clean it up in version 3.5. And while they were at it, they would take that gnarly RegEx and turn it into a variable so it would be easier to read and understand. And it is much much cleaner in WordPress 3.5:

<?php
        $subdir_match          = $subdomain_install ? '' : '([_0-9a-zA-Z-]+/)?';
        $subdir_replacement_01 = $subdomain_install ? '' : '$1';
        $subdir_replacement_12 = $subdomain_install ? '$1' : '$2';


<?php
                $htaccess_file = <<<EOF
RewriteEngine On
RewriteBase {$base}
RewriteRule ^index\.php$ - [L]
{$ms_files_rewriting}
# add a trailing slash to /wp-admin
RewriteRule ^{$subdir_match}wp-admin$ {$subdir_replacement_01}wp-admin/ [R=301,L]

RewriteCond %{REQUEST_FILENAME} -f [OR]
RewriteCond %{REQUEST_FILENAME} -d
RewriteRule ^ - [L]
RewriteRule ^{$subdir_match}(wp-(content|admin|includes).*) {$rewrite_base}{$subdir_replacement_12} [L]
RewriteRule ^{$subdir_match}(.*\.php)$ {$rewrite_base}$subdir_replacement_12 [L]
RewriteRule . index.php [L]
EOF;

Have you all spotted the error yet? Whoever made these changes started by changing this

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

to this

# add a trailing slash to /wp-admin
RewriteRule ^{$subdir_match}wp-admin$ {$subdir_replacement_01}wp-admin/ [R=301,L]

And that's totally valid. It outputs the same exact string as the original but it's easier to read. But then they changed this:

if ( ! $subdomain_install )
                        $htaccess_file .= "\nRewriteRule  ^[_0-9a-zA-Z-]+/(wp-(content|admin|includes).*) $1 [L]\nRewriteRule  ^[_0-9a-zA-Z-]+/(.*\.php)$ $1 [L]";

to this:

RewriteRule ^{$subdir_match}(wp-(content|admin|includes).*) {$rewrite_base}{$subdir_replacement_12} [L]

And those do not match ... they're very similar but they don't match.

So that's where the error was introduced and unless someone can find a really good explanation for it I think it's pretty clear that someone was refactoring /wp-admin/networking.php and, though they did make it a lot cleaner, they made a tiny typo that introduced a very rare bug.

The 3.5 version of this code is character-for-character what's in 5.8, btw, except it's been moved to /wp-admin/includes/network.php.

So does that make sense? Do any of the more experienced WP developers have any info that might show that this is incorrect? There's still a lot about the WP code base I don't understand so I'm totally receptive to the idea that I've jumped to the wrong conclusion.

#58 @bradleyt
3 years ago

#40322 was marked as a duplicate.

#59 in reply to: ↑ 51 @germanoronoz
14 months ago

Hello,

Replying to bananastalktome:

I am not sure if this is truly a valid fix, but feel that an Apache generated 404 page rather than WordPress one (while maybe not ideal) is still better than the current long-standing behavior of an Apache generated 500 page and internal redirect loop.

Even though this is true, passing those urls directly to Apache makes WordPress unable to interpret them, so any redirect plugin (Redirection, Rank Math, Yoast...) will not be able to redirect old urls from images or other wp-content media to new urls.

Those redirects must be done through .htaccess file, which is quite messy for unexperienced users.

Does anyone have a clue on how to solve this point?

Thank you everyone!

#60 follow-up: @wpmuguru
14 months ago

Does anyone have a clue on how to solve this point?

The WordPress virtual/pretty permalink system depends on the web server passing any non-existent URLs on to WP to handle. The now outdated patch I submitted 10+ years ago addresses the rewrite loop that occurs in subfolder multisite so that the result is consistent with a non-existent static resource in single site. The ticket is a bug not an enhancement request.

#61 in reply to: ↑ 60 ; follow-up: @germanoronoz
14 months ago

Replying to wpmuguru:

Does anyone have a clue on how to solve this point?

The WordPress virtual/pretty permalink system depends on the web server passing any non-existent URLs on to WP to handle. The now outdated patch I submitted 10+ years ago addresses the rewrite loop that occurs in subfolder multisite so that the result is consistent with a non-existent static resource in single site. The ticket is a bug not an enhancement request.

#62 in reply to: ↑ 61 @germanoronoz
14 months ago

Replying to germanoronoz:

Replying to wpmuguru:

Does anyone have a clue on how to solve this point?

The WordPress virtual/pretty permalink system depends on the web server passing any non-existent URLs on to WP to handle. The now outdated patch I submitted 10+ years ago addresses the rewrite loop that occurs in subfolder multisite so that the result is consistent with a non-existent static resource in single site. The ticket is a bug not an enhancement request.

Hello,

Thanks for your answer.

I was talking about a subdomain install, in regard to comment #51 on this thread:

RewriteRule ^(wp-(content|admin|includes).*) - [L]
RewriteRule ^(.*\.php)$ - [L]

This condition is what is not being met:


The WordPress virtual/pretty permalink system depends on the web server passing any non-existent URLs on to WP to handle.

Those urls are being passed to Apache directly.

It's crazy a 10 year old bug like this is not yet resolved.

Hopefully anyone can fix this soon.

Regards!

#63 @germanoronoz
14 months ago

Hello Everyone,

I've come up with a workaround.

Before this rewrites:

RewriteRule ^(wp-(content|admin|includes).*) - [L]
RewriteRule ^(.*\.php)$ - [L]

I am adding a redirect for those non-existent URLs inside wp-content|wp-admin|wp-includes:

RewriteCond %{REQUEST_URI} (wp-content|wp-admin|wp-includes) [NC]
RewriteCond %{REQUEST_URI} !error-404 [NC]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule ^ https://%{HTTP_HOST}/error-404/%{REQUEST_URI} [R=301,L]

So before passing those urls to Apache and thus before throwing Apache's 404 error, they are being redirected, from (for example):

https://www.domain.com/wp-content/uploads/sites/666/2023/08/non-existent-image.jpg

To:

https://www.domain.com/error-404/wp-content/uploads/sites/666/2023/08/non-existent-image.jpg

So now this leads to a 404 error, but managed inside WP instead of Apache.

Then we can use redirection plugins (such as Redirection or Rank Math) to create redirects even with regex, for example from:

^error-404/wp-content/uploads/(.*)

To:

https://www.newdomain.com/wp-content/uploads/sites/888/$1

Not optimal but at least is a working workaround.

Regards!

This ticket was mentioned in PR #6402 on WordPress/wordpress-develop by Fexiven.


6 months ago
#64

Since i cannot create an account at core.trac.wordpress.org heres the proper fix for this issue.

This new regex should work better.

I modified the regex as decribed here

Trac ticket: https://core.trac.wordpress.org/ticket/20746

Note: See TracTickets for help on using tickets.