Make WordPress Core

Opened 10 years ago

Last modified 5 months ago

#33821 reopened 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 has-test-info has-unit-tests
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 (2)

add_canonical_port_check.patch (1.1 KB) - added by willshouse 10 years ago.
add canonical port check
33821.tests.diff (4.5 KB) - added by SirLouen 5 months ago.

Download all attachments as: .zip

Change History (60)

@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
10 years ago

#34672 was marked as a duplicate.

#3 @AbdealiJK
10 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
10 years ago

#26625 was marked as a duplicate.

#5 @johnbillion
10 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
9 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.

#11 @SirLouen
6 months ago

  • Keywords needs-test-info added; needs-testing removed

Reproduction Report

Description

❌ This report can't validate that the issue can be reproduced.

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.4.6
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.4.6)
  • Browser: Chrome 136.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Reproduction Steps

  1. I'm using the WordPress-Develop env. I've created this patch to add SSL support just for testing purposes: https://github.com/WordPress/wordpress-develop/pull/8787
  2. Setup in wp-config.php the HTTPS version including the different port
    define( 'WP_SITEURL', 'https://localhost:8890' );
    define( 'WP_HOME', 'https://localhost:8890' );
    
  3. Navigate to the Administration page (/wp-admin)

Actual Results

  1. ❌ Error condition doesn't occur

Additional Notes

  • @wojtekn @willshouse can you provide more specific and detailed testing instructions to replicate this?

This ticket was mentioned in Slack in #core-test by azharderaiya. View the logs.


5 months ago

#13 @azharderaiya
5 months ago

  • Keywords needs-testing added
  • Resolution set to wontfix
  • Status changed from new to closed

I’ve reviewed and tested the issue on the latest WordPress version. The problem still persists—redirect_canonical() does not account for non-standard ports, leading to redirect loops in certain configurations. I attempted the patch provided, but it didn't resolve the issue in my setup.

This issue continues to affect sites running on non-default ports. It would be beneficial to revisit this ticket and consider incorporating a fix.

#14 @SirLouen
5 months ago

  • Keywords needs-testing removed
  • Resolution wontfix deleted
  • Status changed from closed to reopened

@azharderaiya wontfix is not for this purpose, only when the ticket is not useful and we won't work on it anymore.

#15 follow-up: @azharderaiya
5 months ago

@SirLouen I don't know that, so now I'm not able to change it.. can you please guide how to change it.

#16 in reply to: ↑ 15 @SirLouen
5 months ago

Replying to azharderaiya:

@SirLouen I don't know that, so now I'm not able to change it.. can you please guide how to change it.

Don't worry, I have already sorted it.

#17 @SirLouen
5 months ago

  • Keywords close added

At this moment I believe that this might has been patch. I would consider a worksforme, 10 years later form the original report without further instructions.

#18 follow-up: @willshouse
5 months ago

Thank you, everyone, for looking into this. It is indeed still an issue. As mentioned in the original issue, the code in canonical.php, starting at line 64 then, now line 70, does not consider a non-standard port in building the redirect:

https://i.postimg.cc/0rRPg6Cg/building-the-url.png

The code is building a complete URL for a redirect but the port is never considered.

My originally suggested patch should check for a port via the $_SERVER environment variable for on Apache2:

https://i.postimg.cc/ncpQQCJS/patch-notes.png

Logic

  • IF a server port is set
  • AND if there is not already a port in the hostname
  • AND if the port is a non-standard port
  • THEN explicitly add it to the URL that we are building

I'm comparing the canonical.php file from 2015 with the latest one and I don't see any changes to the section of code (starting at line 70) that is building the URL, so I'm unsure of how it could have been resolved? Other than perhaps a change in behavior where Apache would be adding the port into the HTTP_HOST variable - but the suggested patch does check for this, in order to avoid adding a port number twice.

Thanks again to everyone for considering the issue and working to make WordPress better.

#19 in reply to: ↑ 18 @siliconforks
5 months ago

Replying to willshouse:

Other than perhaps a change in behavior where Apache would be adding the port into the HTTP_HOST variable - but the suggested patch does check for this, in order to avoid adding a port number twice.

That's the part that doesn't really make sense here - the port should always be in HTTP_HOST when it's a non-standard port: it's required by the HTTP specification. So I don't see how it's possible to actually reproduce this (unless perhaps you use a buggy/malfunctioning HTTP client?)

I think in order to resolve this issue, it will be necessary to know the exact conditions under which it occurs, including (but not necessarily limited to) the following information:

  1. What browser/client is being used?
  2. What server is being used? (Apache?)
  3. Is there a reverse proxy server being used?
  4. What version of HTTP is being used? (HTTP 1.1? HTTP 2? ...)
  5. Does this happen only with HTTPS? Or can this issue also be observed with unencrypted HTTP on a non-standard port?
  6. What are the values of siteurl and home in the wp_options table? (Or the values of the WP_SITEURL and WP_HOME constants, if using these)
  7. What URL is being accessed?
  8. What URL is it being redirected to? (Because this is supposed to cause a loop, I assume this is the same as the answer to question 7 above? Unless it's a case of URL A redirecting to URL B, and then B redirecting back to A...)
  9. Anything else you can think of that might be necessary to reproduce this

#20 @wojtekn
5 months ago

That's the part that doesn't really make sense here - the port should always be in HTTP_HOST when it's a non-standard port: it's required by the HTTP specification. So I don't see how it's possible to actually reproduce this (unless perhaps you use a buggy/malfunctioning HTTP client?)

@siliconforks in the comment above (https://core.trac.wordpress.org/ticket/33821#comment:10), I showed how those end up being compared without port, and how the redirect host is replaced with the original host.

To share more context, I initially reproduced it in WordPress Studio, and we "solved" it by installing custom mu-plugin to handle this specific redirect (see https://github.com/Automattic/studio/pull/936/commits/f9c393052282fd86dd176bedf91f0049d328c6a5)

The issue was happening when the site was accessed via a non-SSL URL with port (e.g. http://localhost:8888), but the canonical URL was set to a custom domain without a port (http://some.custom.domain.dev).

I would expect that the users will be redirected from http://localhost:8888 to http://some.custom.domain.dev, but in reality they were redirected to http://localhost.

#21 @willshouse
5 months ago

When originally reported I was using an Apache2 server with the reverse proxy module. For me it was solved by the suggested patch.

#22 @siliconforks
5 months ago

I'm still not sure I understand the configuration that would trigger this issue, but if you're using a reverse proxy, note that it is not expected that WordPress will simply work out-of-the-box with a reverse proxy. It will probably be necessary to add code to wp-config.php in order to make it work. See #8593, #9235, #15733, #19337, #24394, #57260...

#23 @SirLouen
5 months ago

Same here, I can't get a clear step by step reproduction instructions with the comment from @wojtekn I understand the solution, but can't get like a straight path to get the problem

I can configure whatever you tell me to configure. Throw me some server confs if needed. I would like the exact setup with all explicit configs possible.

Sometimes I understand that you find a problem, you go into the code and say: "It's clear why is failing". The problem is that ideally it should be infinitely and easily replicable, not only for solving the problem, but for testing the patches by 3rd parties and more importantly, for documentation purposes.

#24 follow-up: @willshouse
5 months ago

I suppose this occurs so seldom that we just choose not to fix it. After all, @siliconforks what mentions about the HTTP specification and how it "should" always include a non-standard port number is true. RFC 7230, Section 5.4 "Host" and from Section 2.7.1 "http URI"

I found some caveats when looking into this-

  • Some misconfigured clients, proxies, or old browsers may omit the port even when it's non-standard
  • PHP's $_SERVER[SERVER_PORT] gives you the actual port the server is listening on, regardless of the Host header

Seems exactly like what's happening. My patch was for fixing these edge cases using the SERVER_PORT parameter.

But maybe since the request is not technically up to specifications we mark this issue as "wontfix" per @azharderaiya

#25 in reply to: ↑ 24 @SirLouen
5 months ago

Replying to willshouse:

I suppose this occurs so seldom that we just choose not to fix it. After all, @siliconforks what mentions about the HTTP specification and how it "should" always include a non-standard port number is true. RFC 7230, Section 5.4 "Host" and from Section 2.7.1 "http URI"

Vouching for this issue, I must say that I'm currently pushing a weird ticket on HTTP1.0 specification for an issue with empty Host Header. A very difficult scenario that I've been able to reproduce consistently but not easily (with any good server like Nginx this is impossible, needed to go into Apache2 casting http1.0 requests). This #63316

I mean: if someone is able to reproduce it, as I've been able to reproduce that ticket, this ball can move on.

Last edited 5 months ago by SirLouen (previous) (diff)

#26 follow-up: @wojtekn
5 months ago

@SirLouen, thanks for being open to help with that issue! My bad, I haven't provided clear steps to reproduce earlier.

I can't reproduce it in WordPress Studio as we fixed this issue with a mu-plugin. The most straightforward steps to reproduce using Local:

  1. Start Local
  2. Ensure to have 'Router mode' set to localhost (Preferences -> Advanced)
  3. Create a new site and start it
  4. Note site's host, e.g. localhost:10003
  5. Start the ngrok tunnel pointing to the site:
ngrok http 10003
  1. Update wp-config.php and set canonical URLs:
define( 'WP_HOME', 'https://MY-NGROK-URL.ngrok-free.app');
define( 'WP_SITEURL', 'https://MY-NGROK-URL.ngrok-free.app');
  1. Open http://localhost:10003/ in a browser or test using curl:
curl -I http://localhost:10003

Expected behavior: User is redirected to https://MY-NGROK-URL.ngrok-free.app
Current behavior: User is redirected to http://localhost

#27 in reply to: ↑ 26 @SirLouen
5 months ago

  • Keywords changes-requested needs-refresh added; needs-test-info close removed

Test Report

Description

❌This report can't validate that the indicated patch works as expected.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/33821/add_canonical_port_check.patch

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Three 1.6
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Testing Info

  • I've followed these instructions.
  • Instead of Local, I'm using the wordpress-develop build (Port 8889)
  • 🐞 I can reproduce the issue

Expected results

  • The port is respected

Actual Results

  1. ❌ Issue not resolved with patch.

Additional Notes

Replying to wojtekn:

Expected behavior: User is redirected to https://MY-NGROK-URL.ngrok-free.app
Current behavior: User is redirected to http://localhost

I've been able to reproduce this, using the wordpress-develop build

HTTP/1.1 301 Moved Permanently
Server: nginx/1.27.5
Date: Fri, 13 Jun 2025 12:04:24 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.28
X-Redirect-By: WordPress
Location: http://localhost/

But I've applied the patch, and nothing seems to be changing here.

During debug I've noted that $_SERVERSERVER_PORT? is 80 at the point, while requested $_SERVERHTTP_HOST? is http://localhost:8889

At some point, the port is stripped but not where the patch expects. Here

	if ( strpos( $_SERVER['HTTP_HOST'], ':' ) == false ) {

The condition is not applying, because as I say the HTTP_HOST has already the port as expected.

I think the problem could be around L616

$user_home = parse_url( home_url() );

if ( ! empty( $user_home['host'] ) ) {
	$redirect['host'] = $user_home['host'];
}

if ( empty( $user_home['path'] ) ) {
	$user_home['path'] = '/';
}
// Handle ports.
if ( ! empty( $user_home['port'] ) ) {
	$redirect['port'] = $user_home['port'];
} else {
	unset( $redirect['port'] );
}

Here it doesnt matter which port is coming as long as the home url doesnt have port, it will be stripped.

Last edited 5 months ago by SirLouen (previous) (diff)

This ticket was mentioned in PR #8977 on WordPress/wordpress-develop by @wojtekn.


5 months ago
#28

  • Keywords needs-refresh removed

Fix redirect comparison to account for port difference to ensure redirects from a requested URL that uses port to a canonical URL that doesn't use port work correctly.

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

#29 follow-up: @wojtekn
5 months ago

@SirLouen I opened a PR with a patch that fixes the issue from my testing steps.

#30 in reply to: ↑ 29 @SirLouen
5 months ago

Replying to wojtekn:

@SirLouen I opened a PR with a patch that fixes the issue from my testing steps.

I've been playing around. This brought me some memory of some tests I did some months ago with ngrok and wordpress-develop. I was trying to set up a remote test site and I had to install a plugin that managed some canonical redirects I can't remember now which and why.

The thing here is that if I set up my HTTPS environment in port 8890
https://github.com/WordPress/wordpress-develop/pull/8787

And then I run ngrok with something like ngrok http https://localhost:8890
Everything works well within WP (without patch and without any other mods, straight out of the box).

Now I'm testing this, with regular http connection and with https

Without the patch:

> curl -I http://localhost:8889
HTTP/1.1 301 Moved Permanently
Server: nginx/1.27.5
Date: Fri, 13 Jun 2025 15:09:21 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.28
X-Redirect-By: WordPress
Location: http://localhost/

With the patch:

> curl -I http://localhost:8889
HTTP/1.1 301 Moved Permanently
Server: nginx/1.27.5
Date: Fri, 13 Jun 2025 15:09:46 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.28
X-Redirect-By: WordPress
Location: http://2854-80-102-99-22.ngrok-free.app/

With my patch and without your patch:

> curl -k -I https://localhost:8890
HTTP/1.1 301 Moved Permanently
Server: nginx/1.27.5
Date: Fri, 13 Jun 2025 15:29:08 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.28
X-Redirect-By: WordPress
Location: https://localhost/

With my patch and your patch:

> curl -k -I https://localhost:8890
HTTP/1.1 301 Moved Permanently
Server: nginx/1.27.5
Date: Fri, 13 Jun 2025 15:28:17 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.28
X-Redirect-By: WordPress
Location: https://ba15-80-102-99-22.ngrok-free.app/

True that the location is wrong without patch. And note that theoretically the canonical redirection should be pointing to https regardless of curling the http or https version of localhost, as we set this in the defines right?

The thing is that If I go into https://ba15-80-102-99-22.ngrok-free.app I can't see any problems
Even the rel=canonical are pointing to the right place

The problem is that I don't know exactly what problems I should be experiencing.

#31 @wojtekn
5 months ago

The problem is that I don't know exactly what problems I should be experiencing.

@SirLouen as far as I see, you confirmed the problem - when you requested such a configured site using http://localhost:8889 without the patch, you were incorrectly redirected to http://localhost/. With the patch, you were redirected to the correct canonical URL http://2854-80-102-99-22.ngrok-free.app/.

That case was more visible in Studio/Local, as those local dev environment apps serve the site under the http://localhost:8889 URL and have the UX that allows opening the site using that URL.

When using Ngrok with such a site, it's required to configure the Ngrok URL as the canonical URL, as in other cases, all links are pointing to http://localhost:8889.

Then, when the user clicks 'Open site' in the Studio or Local:

  1. Local dev env app opens the http://localhost:8889 in the browser
  2. WordPress processes redirect to canonical URL
  3. With bug in place, user ends up at http://localhost
  4. With fix in place, user ends up at correct Ngrok URL

#32 @SirLouen
5 months ago

@wojtekn yes I understand the concern. But, the strange thing is that running this both in my browser and on my phone I was never redirected. Only in curl I could see such. Wondering what's going on

#33 follow-up: @wojtekn
5 months ago

@SirLouen maybe the browser cache? In my case, I need to open a URL in a browser tab with Dev Tools open, and "Disable cache" in the Network tab checked. In other cases, browser caches redirect most of the time.

#34 in reply to: ↑ 33 @SirLouen
5 months ago

Replying to wojtekn:

@SirLouen maybe the browser cache? In my case, I need to open a URL in a browser tab with Dev Tools open, and "Disable cache" in the Network tab checked. In other cases, browser caches redirect most of the time.

I commented you that I could access via mobile for a reason: there is no cache involved in this
Check this video
https://f003.backblazeb2.com/file/wordpress-videos/wp-videos/2025/06/33821.mp4

As you can see, I'm browsing with a Private Firefox tab (I don't even use Firefox regularly)
Here you can see that I can access the site without redirects
But on the curl there is a clear Redirect with the wrong location

We might need to understand a little better what's going on, before proceeding. I'm going to put again precisely my testing instructions, in case you want to reproduce. I also provide some env vars, like ngrok version and so, just for a reference.

Environment

  • WordPress: 6.9-alpha-60093-src
  • ngrok: 3.22.1
  • curl: 8.9.1
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Firefox 139.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Testing Instructions

  1. First get the wordpress-develop build
  2. Patch it with my PR 8787
  3. In the .env file, set the port 8890 with LOCAL_HTTPS_PORT=8890
  4. Run ngrok with ngrok http https://localhost:8890
  5. Get the URL and set it in wp-config.php like:
    define( 'WP_SITEURL', 'https://e708-80-102-99-22.ngrok-free.app' );
    define( 'WP_HOME', 'https://e708-80-102-99-22.ngrok-free.app' );
    
  1. Now get to access https://e708-80-102-99-22.ngrok-free.app from wherever you want and see its fully working without redirets or anything
  2. Try to do the curl -k -I https://localhost:8890 to see the Location https://localhost/

#35 follow-ups: @wojtekn
5 months ago

@SirLouen thanks for sharing the detailed steps. The difference I see between those and mine is that I have used non-SSL, and in your case, you used SSL. Could you try without SSL?

#36 in reply to: ↑ 35 @SirLouen
5 months ago

Replying to wojtekn:

@SirLouen thanks for sharing the detailed steps. The difference I see between those and mine is that I have used non-SSL, and in your case, you used SSL. Could you try without SSL?

I've also tried with HTTP, things are very different

In this case issuing:

ngrok http 8889
It creates the following tunnel URL (which is HTTPS always, by default)
https://d4d5-80-102-99-22.ngrok-free.app/wp-admin/

Problem with ngrok-free.app is that the HSTS forces https by default.
And as I told you, the protocol is not upgrading, only the URL

So when I go into https://d4d5-80-102-99-22.ngrok-free.app/wp-admin/

It will create an infinite loop. With your patch and without your patch.
You can check such infinite loop here

If I had access to ngrok.pizza which doesn't have HSTS policies, I could test going into
http://d4d5-80-102-99-22.ngrok.pizza/wp-admin/

To check how it goes.

So as I told you here, I think that the redirects are not because of the trouble with the host, but because of the protocol redirection (and as I say, your PR 8977 is not solvingthis for me

PS: Remember you can test wordpress-develop by yourself and see what I'm seeing and tell me your experience. I know that you are maintaining WordPress Studio, but we might have to settle a middle ground here for tests.

#37 in reply to: ↑ 35 ; follow-up: @wojtekn
5 months ago

PS: Remember you can test wordpress-develop by yourself and see what I'm seeing and tell me your experience. I know that you are maintaining WordPress Studio, but we might have to settle a middle ground here for tests.

Yes, I'm testing with clean wordpress-develop.

@SirLouen, now I see that my initial fix was addressing the host and port part, but there was still an issue with losing the schema. Still, everything worked well for me, thanks to the double redirect.

With the issue in place:

% curl -I http://localhost:10013
HTTP/1.1 301 Moved Permanently
Server: nginx/1.26.1
Date: Mon, 16 Jun 2025 19:27:15 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.27
X-Redirect-By: WordPress
Location: http://localhost/

With my initial patch:

% curl -I http://localhost:10013
HTTP/1.1 301 Moved Permanently
Server: nginx/1.26.1
Date: Mon, 16 Jun 2025 19:29:43 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.27
X-Redirect-By: WordPress
Location: http://f261-79-117-215-65.ngrok-free.app/

The host and port are correct, but the https scheme set for the home URL is still not preserved.

However, there was another redirect to the correct scheme at this time:

% curl -I http://f261-79-117-215-65.ngrok-free.app/ 
HTTP/1.1 307 Temporary Redirect
Content-Type: text/html; charset=utf-8
Location: https://f261-79-117-215-65.ngrok-free.app/
Date: Mon, 16 Jun 2025 19:31:11 GMT

It looks like the scheme is being lost when elements are being copied from home URL to redirect URL - the series of conditions copies host, path, and port from homeurl, but they miss the scheme:

https://github.com/WordPress/wordpress-develop/blob/3410c205da5564cdb44bd9110c3b27f294583cf0/src/wp-includes/canonical.php#L601

Those look like two different issues.

I updated my patch to cover the scheme part, and now it works fine - it correctly redirects to homeurl and doesn't miss the scheme:

% curl -I http://localhost:10013
HTTP/1.1 301 Moved Permanently
Server: nginx/1.26.1
Date: Mon, 16 Jun 2025 19:39:19 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.27
X-Redirect-By: WordPress
Location: https://f261-79-117-215-65.ngrok-free.app/

What do you think?

#38 in reply to: ↑ 37 @SirLouen
5 months ago

Replying to wojtekn:

What do you think?

I find that this is still problematic

I've been playing around a bit with all this. I've found this service, very simple to configure, similar to ngrok with the peculiarity, that they provide a domain without HSTS, which means, that I can access directly to the HTTP version, without redirect.

Exactly as I was expecting

> curl -I http://localhost:8889
HTTP/1.1 301 Moved Permanently
Server: nginx/1.27.5
Date: Mon, 16 Jun 2025 22:29:41 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.28
X-Redirect-By: WordPress
Location: http://localhost/

The redirection is there, but it works perfectly without the patch. It seems that the problem is exclusively in the scheme not in the host. I have not really inspected what its going under the hood.

Yes, I'm testing with clean wordpress-develop.

I was asking, because your ports look rare to me, wondering why you are not using standard 8889 for wordpres-develop?

Now lets move back into Ngrok.

If I run ngrok http 8889

And I define:

define( 'WP_SITEURL', 'http://2eaa-80-102-99-22.ngrok-free.app' );
define( 'WP_HOME', 'http://2eaa-80-102-99-22.ngrok-free.app' );

It goes well (although pretty useless because all assets are being served over http, while the HSTS upgrade to HTTPS render them unusable). As I said ideally, testing with the other service without HSTS it makes a lot of sense because it avoids the HTTPS upgrade, hence, everything looks as it should (no downgraded assets)

But on the bright side I can say that technically it works well (without styles or anything like that)

Now the problem comes when I define

define( 'WP_SITEURL', 'https://a7d6-80-102-99-22.ngrok-free.app' );
define( 'WP_HOME', 'https://a7d6-80-102-99-22.ngrok-free.app' );

In this scenario, if I move into https://a7d6-80-102-99-22.ngrok-free.app/wp-admin/, it creates a full 302 loop

The interesting thing is that with your patch, all these reproduction steps are completely identical.

Nothing changes for me. Basically, what I can say is that all those changes in canonical.php are not doing a thing regarding this topic (yes in CURL i can see the changes but not in the Firefox/Chrome incognito mode). You could argue that its a problem of cache, but as I say, I'm using incognito mode for everything.

I can't really see a change.

Last edited 5 months ago by SirLouen (previous) (diff)

#39 @wojtekn
5 months ago

@SirLouen, I think we may be testing and fixing different bugs.

I will review the case with the redirect loop you shared in more detail. However, I don't get this part:

Exactly as I was expecting

curl -I http://localhost:8889
HTTP/1.1 301 Moved Permanently
Server: nginx/1.27.5
Date: Mon, 16 Jun 2025 22:29:41 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-Powered-By: PHP/8.2.28
X-Redirect-By: WordPress
Location: http://localhost/

The redirection is there, but it works perfectly without the patch. It seems that the problem is exclusively in the scheme not in the host. I have not really inspected what its going under the hood.

You say this redirection works perfectly, but this redirection is a bug I'm trying to address. 🤔

#40 @wojtekn
5 months ago

@SirLouen okay, I think you meant this redirect loop in WP Admin:

% curl -I https://1c3f-79-117-215-65.ngrok-free.app/wp-admin/
HTTP/2 302 
cache-control: no-cache, must-revalidate, max-age=0, no-store, private
content-type: text/html; charset=UTF-8
date: Tue, 17 Jun 2025 07:52:48 GMT
expires: Wed, 11 Jan 1984 05:00:00 GMT
location: https://f261-79-117-215-65.ngrok-free.app/wp-login.php?redirect_to=https%3A%2F%2F1c3f-79-117-215-65.ngrok-free.app%2Fwp-admin%2F&reauth=1
ngrok-agent-ips: 79.117.215.65
server: nginx/1.26.1
x-powered-by: PHP/8.2.27
x-redirect-by: WordPress

I can reproduce it now with and without my patch, but to confirm, I wasn't trying to fix this particular issue.

This 302 redirect doesn't look like coming from redirect_canonical() but auth_redirect().

#41 @wojtekn
5 months ago

@SirLouen, in the test from the previous comment I used the incorrect site by mistake.

When I access https://1c3f-79-117-215-65.ngrok-free.app/wp-admin/ as guest, I'm redirected to auth form. After I authenticate, I can access https://1c3f-79-117-215-65.ngrok-free.app/wp-admin/ without a redirect loop. It works well with and without my patch.

#42 @SirLouen
5 months ago

  • Keywords 2nd-opinion added

@wojtekn OK, lets focus on one thing at a time. For now, we are trying to fix the HTTP part.

For this reason, i dont think that using ngrok is ideal because of the HTTPS redirections that have been mixing everything up (unless you are also thinking on the http->https upgrade scenario)
Anyway I find that upgrading from http->https is useless because you leave all assets without https, hence, they will not load for cross security troubles.

This said, focusing on the HTTP part:
If I use the tunnel I suggested I still, can't replicate the problem.

I run wordpress-develop in port 8889 then setup

define( 'WP_SITEURL', 'http://strong-cases-smash.loca.lt' );
define( 'WP_HOME', 'http://strong-cases-smash.loca.lt' );

And if I go into the browser http://strong-cases-smash.loca.lt, I can't really see any problems. I can browse all the pages without issues (without the patch)

It's true that if I issue curl -I http://localhost:8889
The Location is localhost

But this is not affecting the WP site at all (unless you have spotted something failing because of this).

So this is my main question: What is the concern of leaving the Location localhost if the site works well.

#43 @wojtekn
5 months ago

So this is my main question: What is the concern of leaving the Location localhost if the site works well.

The site doesn't work well in this case - the redirect from the requested URL to the canonical is broken.

As a user, I want to do the following:

  1. Start Local (or Studio after removing our fix for that bug from the codebase)
  2. Ensure to have 'Router mode' set to localhost (Preferences -> Advanced)
  3. Create a new site and start it
  4. Note site's host, e.g. localhost:10018
  5. Click 'Open site' in the app UI
  6. Confirm that http://localhost:10018/ opens correctly and serves the WP site

Now, I want to use this site with Ngrok and have the 'Open site' link in the app still working:

  1. Start the ngrok tunnel pointing to the site:
    ngrok http 10018
    
  2. Update wp-config.php and set canonical URLs:
    define( 'WP_HOME', 'https://MY-NGROK-URL.ngrok-free.app');
    define( 'WP_SITEURL', 'https://MY-NGROK-URL.ngrok-free.app');
    
  3. Click 'Open site' in the app UI
  4. http://localhost:10018/ now incorrectly redirects to http://localhost instead of Ngrok URL, which is configured in wp-config.php as a canonical URL

I can access the site using the Ngrok URL directly, but to have great UX I would like to be able to use the 'Open site' link in local environment app. The local environment app serves the site under localhost:port, but as I configured the canonical URL in the config file, I expected WordPress will redirect such a request correctly.

Trying to define the issue in the shortest words:

"WordPress incorrectly redirects requested URL with port to the canonical URL"

#44 @SirLouen
5 months ago

  • Keywords has-test-info added; changes-requested 2nd-opinion removed

Combined Bug Reproduction and Patch Test Report

Description

✅ This report validate that the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8977.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Firefox 139.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Testing Instructions

Test Setup

  1. Install some Local tunnelling] software that doesn't have HTTPS redirection with HSTS, like Localtunnel. If you plan to use ngrok, you will need a paid plan to use their non HSTS option (for ngrok.pizza hostnames). We will be using this for the 1st and 2nd test.
  2. Also install ngrok for the 3rd test.

First Test

  1. Depending on your testing environment, check which is the local port for an HTTP connection (not HTTPS)
  2. For example, for wordpress-environment use 8889, this is lt --port 8889
  3. Get the assigned host. Lets say its https://flkajsfljas.loca.lt and add it to wp-config.php (remember to change the HTTPS to HTTP)
    define( 'WP_HOME', 'http://flkajsfljas.loca.lt' );
    define( 'WP_SITEURL', 'http://flkajsfljas.loca.lt' );
    
  4. Now go into http://localhost:8889
  5. 🐞 You get redirected to http://localhost and the connection fails

Second Test

  1. Now change the wp-config.php to:
    define( 'WP_HOME', 'https://flkajsfljas.loca.lt' );
    define( 'WP_SITEURL', 'https://flkajsfljas.loca.lt' );
    
  2. Go into http://localhost:8889
  3. 🐞 You get redirected to https://localhost and the connection fails

Third Test

  1. Now we need to upgrade the connection to HTTPS. With some environment like LocalWP you can do this by default, but if you are working with wordpress-developer you might need a patch to upgrade. Patch with this PR 8787.
  2. If you are using the wordpress-develop build, you will need to add in your .env file this line:
    LOCAL_HTTPS_PORT=8890
    

Otherwise, you can simply get the HTTPS port of your local environment.

  1. Now run we will be running NGROK but with port 8890 ngrok http https://localhost:8890 (or whatever HTTPS port you use)
  2. Change wp-config.php to:
    define( 'WP_HOME', 'https://bc24-80-102-99-22.ngrok-free.app' );
    define( 'WP_SITEURL', 'https://bc24-80-102-99-22.ngrok-free.app' );
    
  3. Go into https://localhost:8890
  4. 🐞 You getg redirected to https://localhost/ and the connection fails

Expected Results

  1. First Test: Given that the canonical URL assigned in WP_HOME and WP_SITEURL is http://flkajsfljas.loca.lt you should be redirected there.
  2. Second Test. Same to first test, you should be redirected to https://flkajsfljas.loca.lt. But given that there is a scheme update from HTTP to HTTPS the page will be throwing errors all over the place, so this is a marginal and irrelevant scenario that we can simply ignore.
  3. Third Test. Same to second test, but now we have upgraded the scheme from HTTP to HTTPS and should be redirected to https://bc24-80-102-99-22.ngrok-free.app so the page should not be throwing errors as in the second test.

Actual Results

  1. ✅ All 3 scenarios are resolved with the patch.

Additional Notes

  • @wojtekn I got you now. I was looking in an entirely wrong direction. Anyway, I think that maybe the auth_redirect for the second test could be considered for another patch. Although, if I'm sincere, this is so irrelevant because after all it's a "useless" upgrade.

needs-code-review

  • Currently, canonical redirects are not respecting WP_HOME settings for scheme. Furthermore, under certain circumstances, host is being rewritten unnecessarily and this line is not being respected.

Providing more context, we must dig into changeset [6097] and ticket #4773 and why this line was introduced. I'm not confident if we could be provoking a regression by ignoring this line with the new patch. This report was already fixed into the 2.3 version, but it did not provide the exact patches that could be causing the trouble. Once we better sort this out and check if there could be no collisions, we can move it forward.

  • Finally, we must consider adding some unit-tests for this.
Last edited 5 months ago by SirLouen (previous) (diff)

#45 follow-up: @wojtekn
5 months ago

I got you now. I was looking in an entirely wrong direction. Anyway, I think that maybe the auth_redirect for the second test could be considered for another patch. Although, if I'm sincere, this is so irrelevant because after all it's a "useless" upgrade.

Thanks for dedicating time to check it more.

Finally, we must consider adding some unit-tests for this.

I've added tests for both issues (preserving the canonical URL scheme and avoiding changes to the host when ports differ).

Currently, canonical redirects are not respecting WP_HOME settings for scheme. Furthermore, under certain circumstances, host is being rewritten unnecessarily and this line is not being respected.
Providing more context, we must dig into changeset [6097] and ticket #4773 and why this line was introduced.

The changeset [6097] added a condition that prevents from redirecting to canonical if request domain and canonical differ only in letter casing. It was reported in #4773.

This condition was further improved by loosening the condition by adding part that allows to redirect from www to non-www and vice versa. (#5089). Looking into that ticket, it's not clear why this change was added there, as the steps described in the ticket don't even tackle www and non-www redirects. I tried removing this condition, and the www to non-www redirects still work fine.

I'm not confident if we could be provoking a regression by ignoring this line with the new patch. This report was already fixed into the 2.3 version, but it did not provide the exact patches that could be causing the trouble. Once we've better sorted this out and c if there could be no collisions, we can move it forward.

What if we added a few more unit tests to confirm that behavior for those cases doesn't change:

  • Example.com → example.com (should not redirect)
  • www.example.com → example.com (should redirect)
  • example.com → www.example.com (should redirect)

#46 in reply to: ↑ 45 @SirLouen
5 months ago

Replying to wojtekn:

What if we added a few more unit tests to confirm that behavior for those cases doesn't change:

  • Example.com → example.com (should not redirect)
  • www.example.com → example.com (should redirect)
  • example.com → www.example.com (should redirect)

Yes, that could be useful.

But here,

/*
	 * 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 )
	) {
		if ( $original_port === $redirect_port ) {
			$redirect['host'] = $original['host'];
		}
	}

What I can interpret is:
If we redirect
Example.com:8080 => Example.com:18889
But then can I get redirected to => example.com:18889
Leading into a redirect loop? (because there is host strtolower normalization)

In case there isn't such risk also from a code review, shouldn't we integrate and document it?

/*
 * Ignore differences in host capitalization, as this can lead to infinite redirects.
 * Only redirect no-www <=> yes-www.
 * Only when ports are identical, we would rather not rewrite ports.
 */
if ( $original_host_low === $redirect_host_low
	|| ( 'www.' . $original_host_low !== $redirect_host_low
	&& 'www.' . $redirect_host_low !== $original_host_low )
	&& ( $original_port === $redirect_port ) {
) {
		$redirect['host'] = $original['host'];
	}
}

So basically in case there could be no problems with the double redirection and a future loop, ideally some unit tests covering several examples for each scenario would be great to prove this right and conclude this ticket.

#47 follow-up: @wojtekn
5 months ago

What I can interpret is:
If we redirect
Example.com:8080 => Example.com:18889
But then can I get redirected to => example.com:18889
Leading into a redirect loop? (because there is host strtolower normalization)

The case normalization is done only for comparison purposes. However, there is one case that we should preserve:

  • Example.com:10200 → example.com:8080 (should redirect but without changing domain case)

I added this as test_different_host_casing_with_port_redirect_without_host_change test, and confirmed that the test goes through fine in trunk, but fails with my patch applied. Then, I applied the change you proposed and confirmed that this test, along with other tests, goes through. Finally, I added another test to explicitly test the port redirect for the same host.

#48 in reply to: ↑ 47 @SirLouen
5 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Replying to wojtekn:

What I can interpret is:
If we redirect
Example.com:8080 => Example.com:18889
But then can I get redirected to => example.com:18889
Leading into a redirect loop? (because there is host strtolower normalization)

The case normalization is done only for comparison purposes. However, there is one case that we should preserve:

  • Example.com:10200 → example.com:8080 (should redirect but without changing domain case)

I added this as test_different_host_casing_with_port_redirect_without_host_change test, and confirmed that the test goes through fine in trunk, but fails with my patch applied. Then, I applied the change you proposed and confirmed that this test, along with other tests, goes through. Finally, I added another test to explicitly test the port redirect for the same host.

I can't think in more scenarios now. At first glance it looks good to me. I've added some extra code review things in the PR (mostly docs guides)

#49 @SirLouen
5 months ago

By the way, I'm noticing that all the 7 tests are identical
I think that it will be better to use just one function + a data provider. Give 6 parameters (home_url, site_url, request_host, request_url and expected_url, a failure_msg, as the array parameters, and then pass each element)

Also, after each assert, you can add the failure_msg.

I attach here the patch for reference so you can integrate it in your PR.

#50 follow-up: @wojtekn
5 months ago

Thanks for proposing the refactor. I applied those changes and further refined the code.

#51 in reply to: ↑ 50 @SirLouen
5 months ago

  • Keywords needs-testing added

Replying to wojtekn:

Thanks for proposing the refactor. I applied those changes and further refined the code.

Ok, I think its time to move this forward.

#52 @wojtekn
5 months ago

Thanks @SirLouen !

Separately from the above efforts, I tried to look into the exact issue reported by @willshouse. The description points to the case in which $requested_url is not provided to redirect_canonical(), and then this function assembles it based on the $_SERVER array values.

I'm happy to look into this one, too. As there were comments pointing out that it might be related to incorrect server configuration, I would appreciate sharing the test that targets redirect_canonical() and allows to reproduce the issue. I tried this one, but couldn't cause it to fail:

	public function test_domain_and_port_redirections_ssl() {
		update_option( 'home', 'https://example.com:8443' );
		update_option( 'siteurl', 'https://example.com:8443' );

		// Simulate a request to a non-canonical domain.
		$_SERVER['HTTP_HOST']   = 'example.com';
		$_SERVER['REQUEST_URI'] = '/';

		$redirect = redirect_canonical( null, false );

		$this->assertSame( 'https://example.com:8443/', $redirect, 'Could not redirect' );
	}

#53 @SirLouen
5 months ago

@wojtekn I think he suggested that he wasn't able to reproduce it consistently anymore. At that point I was already looking to a different point, so I'm not sure if its worth to dig there anymore (unless someone could report again, with precise instructions).

If you happen to find them, maybe you could open a new ticket (unless you think this can easily be integrated within the current flow).

For now, I'm passing this for testing with the current state of patch. If it happens to have some traction before you are able to reproduce it, then I recommend you to open a new ticket, and if you want, reference it here.

#54 @wojtekn
5 months ago

Sounds like a great plan @SirLouen !

This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.


5 months ago

#57 @nikunj8866
5 months ago

  • Keywords needs-testing removed

Test Report

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8977

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 138.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0
    • WP Mail Logging 1.14.0

Steps to Reproduce or Test

Test Setup(1st and 2nd Test)

  1. In Docker Desktop, start wordpress-develop project.
  2. Setup Localtunnel by npm install -g localtunnel.

First Test

  1. run lt --port 8889. It gives me https://two-crews-kick.loca.lt.
  2. Added this URL in wp-config.php of wordpress-develop.
    define( 'WP_HOME', 'http://two-crews-kick.loca.lt' );
    define( 'WP_SITEURL', 'http://two-crews-kick.loca.lt' );
    
  3. Go to http://localhost:8889
  4. 🐞 Redirected to http://localhost and the connection fails (https://prnt.sc/QTEJiqATwfRr)

Second Test

  1. Changed the HOME_URL and SITE_URL in wp-config.php to
    define( 'WP_HOME', 'https://two-crews-kick.loca.lt' );
    define( 'WP_SITEURL', 'https://two-crews-kick.loca.lt' );
    
  2. Go to http://localhost:8889
  3. 🐞 Redirected to http://localhost and the connection fails (https://prnt.sc/QTEJiqATwfRr)

Third Test

  1. Install ngrok by choco install ngrok.
  2. Sign up for the ngrok.
  3. Added ngrok token in my system ngrok config add-authtoken YOUR_TOKEN
  4. Applied patch https://github.com/WordPress/wordpress-develop/pull/8787
  5. Added LOCAL_HTTPS_PORT=8890 in .env file.
  6. run 'ngrok http https://localhost:8890'. Gets https://7640-2405-201-202e-b0f6-d19e-3557-142e-3caf.ngrok-free.app url.
  7. Added this URL in wp-config.php of wordpress-develop.
    define( 'WP_HOME', 'https://7640-2405-201-202e-b0f6-d19e-3557-142e-3caf.ngrok-free.app' );
    define( 'WP_SITEURL', 'https://7640-2405-201-202e-b0f6-d19e-3557-142e-3caf.ngrok-free.app' );
    
  8. Go to http://localhost:8889
  9. 🐞 Redirected to http://localhost and the connection fails (https://prnt.sc/QTEJiqATwfRr)

Expected Results

When testing a patch to validate it works as expected:

When reproducing a bug:

  • ❌ Error condition occurs.

Screenshots

Before Patch Screencast:
https://screenrec.com/share/wAIuiJZ0jO

After Patch Screencast:
https://screenrec.com/share/k1G5qXz9o8
https://screenrec.com/share/3SfbCQOgDe

This ticket was mentioned in Slack in #core-test by nikunj8866. View the logs.


5 months ago

Note: See TracTickets for help on using tickets.