Add tunneling support #936

sejas · 2025-02-14T04:19:36Z

Related issues

Fixes https://github.com/Automattic/dotcom-forge/issues/9783

Proposed Changes

It enables users to add WP_SITEURL and WP_HOME constants in wp-config.php
It adds a mu-plugin to redirect from localhost:XXXX to WP_SITEURL if defined, in that case wp-admin autologin doesn't work.

Kudos to @wojtekn for suggesting this approach p1739469267935719-slack-C06DRMD6VPZ

Testing Instructions

Run npm start
Create a site
Test the new site and confirm it's working as expected.
Edit wp-config.php to add WP_SITEURL and WP_HOME constants at the top of the file
You can set http://whatever.localhost:8881 or run ngrok http 8881 and use the url returned by ngrok. 8881 is the port set by Studio and you can see it in the Settings tab.
Start Studio site
Observe the wp-admin and open site links open the defined URL and the site works as expected

Note

The autologin doesn't work if you are using Ngrok, and we don't plan to fix it in this PR.

studio-simple-solution-wp-config-constants.mp4

Pre-merge Checklist

Have you checked for TypeScript, React or other console errors?

vendor/wp-now/src/wp-now.ts

…er constant

…port and fix open site link

sejas · 2025-02-14T11:18:03Z

vendor/wp-now/src/wp-now.ts

+	php.writeFile(
+		path.posix.join( PLAYGROUND_INTERNAL_MU_PLUGINS_FOLDER, '0-redirect-to-siteurl-constant.php' ),
+		`<?php
+		// See https://core.trac.wordpress.org/ticket/33821#comment:10
+		add_action( 'init', function() {
+			if ( ! defined( 'WP_SITEURL' ) ) {
+				return;
+			}
+
+			$current_host = $_SERVER['HTTP_HOST'] ?? '';
+
+			if ( preg_match( '/^localhost:\\d+$/', $current_host ) ) {
+				$requested_uri = $_SERVER['REQUEST_URI'] ?? '/';
+				wp_redirect( rtrim( WP_SITEURL, '/' ) . $requested_uri, 302 );
+				exit;
+			}
+		});
+		`
+	);


On 01e0120 I explored a solution having a function geWpConfigConstants that exposed wp-config.php constants to Studio to display the correct links in wp-admin and open site.

After discussing it with @wojtekn , we decided to simplify the logic and relay in WordPress default redirect. Due to this issue https://core.trac.wordpress.org/ticket/33821#comment:10 the redirect doesn't work out of the box and we need this mu plugin.

The wp-admin link redirects correctly to the ngrok url defined in WP_SITEURL, but it doesn't perform the autologin.

To recap, is the issue here that redirect_canonical doesn't work properly when we request a URL that includes a port number?

@fredrikekelund correct. It seems it also doesn't handle non-ssl http://localhost:8888/ redirect to https://my-tunnel.com/ well. It would make sense to dedicate some time later to check what we could do to rely on the core redirect_canonical.

sejas · 2025-02-14T11:19:58Z

vendor/wp-now/src/wp-now.ts

-	const wpConfigConsts = {
-		WP_HOME: siteUrl,
-		WP_SITEURL: siteUrl,
-	};


Removing this constants can uncover some issues in sites where the option siteurl and home point to a different URL.
When starting those sites WordPress will redirect to the URL saved in the database which will lead to a "non-response" page.
We can fix this behaviour in a different PR where we update those variables in database when we create a site with an existing SQLite database that already contains a siteurl with a different port.

Added one issue I've uncovered: #939

Where do we actually define siteurl and home now..? Are those options just implied in the installation process?

Removing this constants can uncover some issues in sites where the option siteurl and home point to a different URL.

Isn't that normal behavior for WordPress, though?

Where do we actually define siteurl and home now..? Are those options just implied in the installation process?

This is correct. Studio calls wp-admin/install.php to install WordPress, which internally guesses the correct site url and saves it as option in the database. It happens in https://github.com/WordPress/WordPress/blob/c5cc9402b42939fd6a1370a7a463748ac9a2f595/wp-admin/includes/upgrade.php#L93

Isn't that normal behavior for WordPress, though?

It is, and it should be done that way in the first place. Removing that now can uncover some issues, though. With those constants being set by Studio in a brute force way, it was masking flows in which the siteurl/home ended up being incorrect. One of those is an issue I linked above.

fredrikekelund

LGTM 👍 I tested this with Jurassic Tube and it worked great.

I left a couple of questions that would be good to have answers for, but they shouldn't block merging.

fredrikekelund · 2025-02-14T13:48:28Z

vendor/wp-now/src/wp-now.ts

 	php.mkdir( PLAYGROUND_INTERNAL_MU_PLUGINS_FOLDER );

+	php.writeFile(
+		path.posix.join( PLAYGROUND_INTERNAL_MU_PLUGINS_FOLDER, '0-https-for-reverse-proxy.php' ),


Good on remembering to use POSIX paths 👍

fredrikekelund · 2025-02-14T13:55:57Z

vendor/wp-now/src/wp-now.ts

+	php.writeFile(
+		path.posix.join( PLAYGROUND_INTERNAL_MU_PLUGINS_FOLDER, '0-redirect-to-siteurl-constant.php' ),
+		`<?php
+		// See https://core.trac.wordpress.org/ticket/33821#comment:10
+		add_action( 'init', function() {
+			if ( ! defined( 'WP_SITEURL' ) ) {
+				return;
+			}
+
+			$current_host = $_SERVER['HTTP_HOST'] ?? '';
+
+			if ( preg_match( '/^localhost:\\d+$/', $current_host ) ) {
+				$requested_uri = $_SERVER['REQUEST_URI'] ?? '/';
+				wp_redirect( rtrim( WP_SITEURL, '/' ) . $requested_uri, 302 );
+				exit;
+			}
+		});
+		`
+	);


To recap, is the issue here that redirect_canonical doesn't work properly when we request a URL that includes a port number?

fredrikekelund · 2025-02-14T13:59:32Z

vendor/wp-now/src/wp-now.ts

-	const wpConfigConsts = {
-		WP_HOME: siteUrl,
-		WP_SITEURL: siteUrl,
-	};


Where do we actually define siteurl and home now..? Are those options just implied in the installation process?

sejas added 6 commits February 14, 2025 01:04

Enable https to avoid infinite loop when behind a reverse proxy

67ff142

remove setting a siteurl to allow setting it at wp-config level

e98835d

restore siteurl virtual config to make new sites work

743a363

read wp-config variables and set siturl only if not defined

c6171d1

read wp-config constants with clean runtime

01e0120

fix absolute url displayed on studio header

6118aff

sejas self-assigned this Feb 14, 2025

sejas requested a review from a team February 14, 2025 04:19

sejas commented Feb 14, 2025

View reviewed changes

vendor/wp-now/src/wp-now.ts Outdated Show resolved Hide resolved

sejas added 4 commits February 14, 2025 04:41

remove siteurls variables from wp-now

e9809a6

Add comment for HTTP_X_FORWARDED_PROTO

2255b7f

Remove geWpConfigConstants which allowed Studio to grab WP_SITEURL us…

a9cb06e

…er constant

Enable redirect to WP_SITEURL to avoid redirect to localhost without …

f9c3930

…port and fix open site link

sejas commented Feb 14, 2025

View reviewed changes

sejas added the Reviewer can merge label Feb 14, 2025

wojtekn mentioned this pull request Feb 14, 2025

Search and replace doesn't work correctly during the import #939

Closed

fredrikekelund approved these changes Feb 14, 2025

View reviewed changes

wojtekn approved these changes Feb 14, 2025

View reviewed changes

wojtekn merged commit 402362f into trunk Feb 14, 2025
11 checks passed

wojtekn deleted the add/tunneling-support branch February 14, 2025 15:12

wojtekn mentioned this pull request Feb 17, 2025

Customize Studio site URL #755

Closed

2 tasks

Add tunneling support #936

Add tunneling support #936

Uh oh!

Conversation

sejas commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issues

Proposed Changes

Testing Instructions

Pre-merge Checklist

Uh oh!

Uh oh!

sejas Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wojtekn Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sejas Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredrikekelund left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sejas commented Feb 14, 2025 •

edited

Loading

sejas Feb 14, 2025 •

edited

Loading

wojtekn Feb 14, 2025 •

edited

Loading

sejas Feb 14, 2025 •

edited

Loading