WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 6 months ago

Last modified 4 months ago

#40013 closed defect (bug) (duplicate)

Inifinite loop in subfolder

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

Description

Hi,

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:
index.php
wp-blog-header.php
require_once( ABSPATH . WPINC . '/template-loader.php' );
/var/www/html/wp-includes/template-loader.php
do_action( 'template_redirect' ); ==>
/var/www/html/wp-includes/default-filters.php
add_action( 'template_redirect', 'redirect_canonical' );
The actual broken code is in:
/var/www/html/wp-includes/canonical.php
=> $redirect_url is empty inside, so return;
And the broken code in it is:

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

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:

<?php
if (redirect_canonical($redirect_url, false) ) {

And not

<?php
if (!redirect_canonical($redirect_url, false) ) {

Change History (4)

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


6 months ago

#2 follow-up: @markparnell
6 months 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
6 months 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 6 months ago by SergeyBiryukov (next)

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


4 months ago

Note: See TracTickets for help on using tickets.