Make WordPress Core

Opened 10 years ago

Last modified 2 months ago

#33821 new defect (bug)

redirect_canonical does not consider port in $compare_original

Reported by: willshouse's profile willshouse Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.3
Component: Canonical Keywords: has-patch needs-unit-tests needs-testing
Focuses: Cc:

Description (last modified by johnbillion)

In the wp-includes/canonical.php file the $requested_url is built starting at line 64. It combines is_ssl() for protocol, $_SERVER['HTTP_HOST'], and $_SERVER['REQUEST_URI'] - but it does not consider $_SERVER['SERVER_PORT']

This causes a redirect loop for us because we run HTTPS on port 8443.

I suggest checking to see if a port other than 80 or 443 is being used and adding that as part of the comparison - suggested patch attached.

Attachments (1)

add_canonical_port_check.patch (1.1 KB) - added by willshouse 10 years ago.
add canonical port check

Download all attachments as: .zip

Change History (11)

@willshouse
10 years ago

add canonical port check

#1 @johnbillion
10 years ago

  • Component changed from General to Canonical
  • Description modified (diff)
  • Keywords has-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 4.3 to 2.3

Thanks for the patch, willshouse.

This will need some unit tests.

#2 @swissspidy
9 years ago

#34672 was marked as a duplicate.

#3 @AbdealiJK
9 years ago

As previously mentioned in #34672 I find the following issue :

I am able to open wp-admin and everything works well.
When I open my home page, I get a redirect loop.
On further investigation, it seems redirect_canonical() in wp-includes/canonical.php is the culprit. It finds the :

And the extra :80 causes the condition $redirect_url == $requested_url to be false - causing the infinite redirect.

I tried the above patch for this issue, and it didn't seem to solve this issue.

#4 @johnbillion
9 years ago

#26625 was marked as a duplicate.

#5 @johnbillion
9 years ago

  • Keywords https added

This ticket was mentioned in Slack in #core-http by johnbillion. View the logs.


9 years ago

#7 @johnbillion
9 years ago

  • Keywords needs-testing added; https removed

#10 @wojtekn
2 months ago

I spotted a similar issue today. Doesn't the following code cause it?

	/*
	 * Ignore differences in host capitalization, as this can lead to infinite redirects.
	 * Only redirect no-www <=> yes-www.
	 */
	if ( $original_host_low === $redirect_host_low
		|| ( 'www.' . $original_host_low !== $redirect_host_low
			&& 'www.' . $redirect_host_low !== $original_host_low )
	) {
		$redirect['host'] = $original['host'];
	}

Let's assume the hosts look as follows:

Requested: http://localhost:8888
Canonical: http://some.custom.domain.dev

The variables look as follows:

$original_host_low = 'localhost';
$redirect_host_low = 'some.custom.domain.dev';

How conditions resolve:

  • 'localhost' === 'some.custom.domain.dev' -> false
  • 'www.' . 'localhost' !== 'some.custom.domain.dev' -> true
  • 'www.' . 'some.custom.domain.dev' !== 'localhost' -> true

As the second and third parts resolve to true, the whole condition resolves, and the redirect host is replaced with the original host.

Then, later in the flow, the user is redirected to http://localhost (without port) instead of http://some.custom.domain.dev.

Note: See TracTickets for help on using tickets.