Skip to content

Add tunneling support #936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wojtekn merged 10 commits into from
Feb 14, 2025
Merged

Add tunneling support #936

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
67ff142
Enable https to avoid infinite loop when behind a reverse proxy
sejas Feb 14, 2025
e98835d
remove setting a siteurl to allow setting it at wp-config level
sejas Feb 14, 2025
743a363
restore siteurl virtual config to make new sites work
sejas Feb 14, 2025
c6171d1
read wp-config variables and set siturl only if not defined
sejas Feb 14, 2025
01e0120
read wp-config constants with clean runtime
sejas Feb 14, 2025
6118aff
fix absolute url displayed on studio header
sejas Feb 14, 2025
e9809a6
remove siteurls variables from wp-now
sejas Feb 14, 2025
2255b7f
Add comment for HTTP_X_FORWARDED_PROTO
sejas Feb 14, 2025
a9cb06e
Remove geWpConfigConstants which allowed Studio to grab WP_SITEURL us…
sejas Feb 14, 2025
f9c3930
Enable redirect to WP_SITEURL to avoid redirect to localhost without …
sejas Feb 14, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 40 additions & 22 deletions vendor/wp-now/src/wp-now.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,14 +250,14 @@ async function runIndexMode( php: PHP, { documentRoot, projectPath }: WPNowOptio

async function runWpContentMode(
php: PHP,
{ documentRoot, wordPressVersion, wpContentPath, projectPath, absoluteUrl }: WPNowOptions
{ documentRoot, wordPressVersion, wpContentPath, projectPath }: WPNowOptions
) {
const wordPressPath = path.join( getWordpressVersionsPath(), wordPressVersion );
await php.mount(
wordPressPath,
createNodeFsMountHandler( documentRoot ) as unknown as MountHandler
);
await initWordPress( php, wordPressVersion, documentRoot, absoluteUrl );
await initWordPress( php, wordPressVersion, documentRoot );
fs.ensureDirSync( wpContentPath );

await php.mount(
Expand All @@ -280,29 +280,25 @@ async function runWordPressDevelopMode(
} );
}

async function runWordPressMode(
php: PHP,
{ documentRoot, projectPath, absoluteUrl }: WPNowOptions
) {
async function runWordPressMode( php: PHP, { documentRoot, projectPath }: WPNowOptions ) {
php.mkdir( documentRoot );
await php.mount(
documentRoot,
createNodeFsMountHandler( projectPath ) as unknown as MountHandler
);

await initWordPress( php, 'user-provided', documentRoot, absoluteUrl );
await initWordPress( php, 'user-provided', documentRoot );
}

async function runPluginOrThemeMode(
php: PHP,
{ wordPressVersion, documentRoot, projectPath, wpContentPath, absoluteUrl, mode }: WPNowOptions
{ wordPressVersion, documentRoot, projectPath, wpContentPath, mode }: WPNowOptions
) {
const wordPressPath = path.join( getWordpressVersionsPath(), wordPressVersion );
await php.mount(
wordPressPath,
createNodeFsMountHandler( documentRoot ) as unknown as MountHandler
);
await initWordPress( php, wordPressVersion, documentRoot, absoluteUrl );
await initWordPress( php, wordPressVersion, documentRoot );

fs.ensureDirSync( wpContentPath );
fs.copySync(
Expand Down Expand Up @@ -344,14 +340,14 @@ async function runPluginOrThemeMode(

async function runWpPlaygroundMode(
php: PHP,
{ documentRoot, wordPressVersion, wpContentPath, absoluteUrl }: WPNowOptions
{ documentRoot, wordPressVersion, wpContentPath }: WPNowOptions
) {
const wordPressPath = path.join( getWordpressVersionsPath(), wordPressVersion );
await php.mount(
wordPressPath,
createNodeFsMountHandler( documentRoot ) as unknown as MountHandler
);
await initWordPress( php, wordPressVersion, documentRoot, absoluteUrl );
await initWordPress( php, wordPressVersion, documentRoot );

fs.ensureDirSync( wpContentPath );
fs.copySync(
Expand Down Expand Up @@ -418,12 +414,7 @@ async function login( php: PHP, options: WPNowOptions = {} ) {
* @param vfsDocumentRoot
* @param siteUrl
*/
async function initWordPress(
php: PHP,
wordPressVersion: string,
vfsDocumentRoot: string,
siteUrl: string
) {
async function initWordPress( php: PHP, wordPressVersion: string, vfsDocumentRoot: string ) {
let initializeDefaultDatabase = false;
if ( ! php.fileExists( `${ vfsDocumentRoot }/wp-config.php` ) ) {
php.writeFile(
Expand All @@ -433,10 +424,7 @@ async function initWordPress(
initializeDefaultDatabase = true;
}

const wpConfigConsts = {
WP_HOME: siteUrl,
WP_SITEURL: siteUrl,
};
Comment on lines -436 to -439

@sejas sejas Feb 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@wojtekn wojtekn Feb 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added one issue I've uncovered: #939

@fredrikekelund fredrikekelund Feb 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@fredrikekelund fredrikekelund Feb 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@wojtekn wojtekn Feb 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@wojtekn wojtekn Feb 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
const wpConfigConsts = {};

if ( wordPressVersion !== 'user-provided' ) {
wpConfigConsts[ 'WP_AUTO_UPDATE_CORE' ] = wordPressVersion === 'latest';
Expand Down Expand Up @@ -471,6 +459,36 @@ export function getThemeTemplate( projectPath: string ) {
async function mountInternalMuPlugins( php: PHP ) {
php.mkdir( PLAYGROUND_INTERNAL_MU_PLUGINS_FOLDER );

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

@fredrikekelund fredrikekelund Feb 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good on remembering to use POSIX paths 👍
`<?php
// See https://developer.wordpress.org/advanced-administration/security/https/#using-a-reverse-proxy
if( isset( $_SERVER['HTTP_X_FORWARDED_PROTO'] ) && strpos( $_SERVER['HTTP_X_FORWARDED_PROTO'], 'https') !== false ){
$_SERVER['HTTPS'] = 'on';
}
`
);

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;
}
});
`
);
Comment on lines +472 to +490

@sejas sejas Feb 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@fredrikekelund fredrikekelund Feb 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@wojtekn wojtekn Feb 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

php.writeFile(
path.posix.join( PLAYGROUND_INTERNAL_MU_PLUGINS_FOLDER, '0-allowed-redirect-hosts.php' ),
`<?php
Expand Down