Make WordPress Core

Opened 6 years ago

Closed 3 years ago

Last modified 3 years ago

#40013 closed defect (bug) (duplicate)

Inifinite loop in subfolder

Reported by: dsero's profile dSero Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7.2
Component: Canonical Keywords:
Focuses: Cc:



We implemented WP on a dedicated server, where /blog serves as a subfolder behind an AWS application ELB that directs all /blog traffic to this server. The ELB also offloads the SSL.

When configuring the WP, we faced an inifinte loop (301 redirect) on the /blog/ path, while intenral paths works great (/blog/trial for exmample).

When debugging the code, we found out that the "protect against chained redirects" in /var/www/html/wp-includes/canonical.php actually doesn't work, as the if state should be true instead of false:

Please find trace below:

Pages flow:
require_once( ABSPATH . WPINC . '/template-loader.php' );
do_action( 'template_redirect' ); ==>
add_action( 'template_redirect', 'redirect_canonical' );
The actual broken code is in:
=> $redirect_url is empty inside, so return;
And the broken code in it is:

!redirect_canonical($redirect_url, false) 
                // protect against chained redirects
                if (!redirect_canonical($redirect_url, false) ) {
                        wp_redirect($redirect_url, 301);

Looks like there is a bug when there a sub folder model is being used.
Instead of protecting from chained redirects, the if actually causes them, since the return from the funciton when URLs are the same if return;
So correct code should be:

if (redirect_canonical($redirect_url, false) ) {

And not

if (!redirect_canonical($redirect_url, false) ) {

Change History (4)

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

3 years ago

#2 follow-up: @markparnell
3 years ago

  • Component changed from General to Canonical
  • Keywords 2nd-opinion added

Hi @dSero on the surface the existing code looks correct to me, but someone who is familiar with the code is going to need to take a look. Moving to the correct component and flagging for 2nd-opinion.

#3 in reply to: ↑ 2 @SergeyBiryukov
3 years ago

  • Keywords 2nd-opinion removed
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Hi there, welcome to WordPress Trac! Sorry it took so long for someone to get back to you.

Replying to markparnell:

on the surface the existing code looks correct to me, but someone who is familiar with the code is going to need to take a look.

The existing logic is indeed correct.

If redirect_canonical( $redirect_url, false ) returns void, that means there won't be an infinite loop, and it's safe to proceed with the redirect.

If redirect_canonical( $redirect_url, false ) returns a string, and we're proceeding with the redirect as the ticket suggests, that means we're creating an infinite loop.

This appears to be a duplicate of #15733, #31288, #38273, and some other tickets.

The long and short of it is that the redirect loop is caused by the is_ssl() function, most likely due to $_SERVER['HTTPS'] value not mapped to $_SERVER['HTTP_X_FORWARDED_PROTO'] in your environment. This is a server-level configuration issue with reverse proxy web servers. You just need to add something along the lines of the following to your wp-config.php file:

$_SERVER['HTTPS'] = 1;

Any proxy configuration is "supported" by WordPress, you just need to remap the $_SERVER['HTTPS'] server variable based the particular proxy configuration you're using.

Version 0, edited 3 years ago by SergeyBiryukov (next)

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

3 years ago

Note: See TracTickets for help on using tickets.