[wp-trac] [WordPress Trac] #33821: redirect_canonical does not consider port in $compare_original

[WordPress Trac]

#33821: redirect_canonical does not consider port in $compare_original
-------------------------------------------------+-------------------------
 Reporter:  willshouse                           |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  reopened
 Priority:  normal                               |   Milestone:  Future
                                                 |  Release
Component:  Canonical                            |     Version:  2.3
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch needs-unit-tests has-      |     Focuses:
  test-info                                      |
-------------------------------------------------+-------------------------
Changes (by SirLouen):

 * keywords:  has-patch needs-unit-tests changes-requested 2nd-opinion =>
     has-patch needs-unit-tests has-test-info


Comment:

 = 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 [https://theboroer.github.io/localtunnel-www
 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

 2. Depending on your testing environment, check which is the local port
 for an HTTP connection (not HTTPS)
 3. For example, for `wordpress-environment` use 8889, this is `lt --port
 8889`
 4. 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', 'https://flkajsfljas.loca.lt' );
 define( 'WP_SITEURL', 'http://flkajsfljas.loca.lt' );
 }}}
 5. Now go into http://localhost:8889
 6. 🐞 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
 [https://github.com/WordPress/wordpress-develop/pull/8977 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.
 3. Now run we will be running NGROK but with port 8890 `ngrok http
 https://localhost:8890` (or whatever HTTPS port you use)
 4. Go into https://localhost:8890
 5. 🐞 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 so the page should not be throwing errors as in the
 first 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 [https://github.com/SirLouen/wordpress-
 develop/blob/trunk/src/wp-includes/canonical.php#L601 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.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/33821#comment:44>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform