Route requests for missing static files using remote asset metadata - Second attempt (#1490)

Prior to this PR, we could not easily tell whether we should request a
missing static asset from the web or delegate a request for a missing
file to WordPress.

Related to #1365 where we need better information to judge whether to
handle a request for a missing static file as PHP.

This PR:
- Includes a list of remote asset paths with minified WP builds
- Updates request handling to only handle missing files as static when
they are explicitly listed as remote assets
(perhaps this should be split into multiple PRs)
- Updates request routing to behave more like a regular web server

By including a list of remote asset paths with minified WP builds, we
can judge which missing static file paths represent remote assets and
which are truly missing and need to be delegated to WordPress.

- CI tests
- Test manually with `npm run dev` and observe that Playground loads
normally with no unexpected errors in the console

Author: brandonpayton

packages/php-wasm/node/src/test/php-request-handler.spec.ts

@@ -39,6 +39,27 @@ describe.each(SupportedPHPVersions)(
 			});
 		});
 
+		it('should execute a non-default PHP file in a directory', async () => {
+			php.mkdirTree('/folder');
+			php.writeFile(
+				'/folder/some.php',
+				`<?php echo 'Some PHP file in a folder.';`
+			);
+			const response = await handler.request({
+				url: '/folder/some.php',
+			});
+			expect(response).toEqual({
+				httpStatusCode: 200,
+				headers: {
+					'content-type': ['text/html; charset=UTF-8'],
+					'x-powered-by': [expect.any(String)],
+				},
+				bytes: new TextEncoder().encode('Some PHP file in a folder.'),
+				errors: '',
+				exitCode: 0,
+			});
+		});
+
 		it('should serve a static file', async () => {
 			php.writeFile('/index.html', `Hello World`);
 			const response = await handler.request({
@@ -102,7 +123,8 @@ describe.each(SupportedPHPVersions)(
 			});
 		});
 
-		it('should yield x-file-type=static when a static file is not found', async () => {
+		it('should yield x-file-type=static when a static file is not found and is listed as a remote asset', async () => {
+			handler.addRemoteAssetPaths(['index.html']);
 			const response = await handler.request({
 				url: '/index.html',
 			});
@@ -117,6 +139,19 @@ describe.each(SupportedPHPVersions)(
 			});
 		});
 
+		it('should not yield x-file-type=static when a static file is not found and is not listed as a remote asset', async () => {
+			const response = await handler.request({
+				url: '/index.html',
+			});
+			expect(response).toEqual({
+				httpStatusCode: 404,
+				headers: {},
+				bytes: expect.any(Uint8Array),
+				errors: '',
+				exitCode: 0,
+			});
+		});
+
 		it('should not yield x-file-type=static when a PHP file is not found', async () => {
 			const response = await handler.request({
 				url: '/index.php',
@@ -130,6 +165,77 @@ describe.each(SupportedPHPVersions)(
 			});
 		});
 
+		it('should redirect to add trailing slash to existing dir', async () => {
+			php.mkdirTree('/folder');
+			const response = await handler.request({
+				url: '/folder',
+			});
+			expect(response).toEqual({
+				httpStatusCode: 301,
+				headers: {
+					Location: ['/folder/'],
+				},
+				bytes: expect.any(Uint8Array),
+				errors: '',
+				exitCode: 0,
+			});
+		});
+
+		it('should return 200 and pass query strings when a valid request is made to a folder', async () => {
+			php.mkdirTree('/folder');
+			php.writeFile('/folder/index.php', `<?php echo $_GET['key'];`);
+			const response = await handler.request({
+				url: '/folder/?key=value',
+			});
+			expect(response.httpStatusCode).toEqual(200);
+			expect(response.text).toEqual('value');
+		});
+
+		it('should default a folder request to index.html if it exists and index.php does not', async () => {
+			php.mkdirTree('/folder');
+			php.writeFile('/folder/index.html', `INDEX DOT HTML`);
+			const response = await handler.request({
+				url: '/folder/?key=value',
+			});
+			expect(response.httpStatusCode).toEqual(200);
+			expect(response.text).toEqual('INDEX DOT HTML');
+		});
+
+		it('should default a folder request to index.php when when both index.php and index.html exist', async () => {
+			php.mkdirTree('/folder');
+			php.writeFile('/folder/index.php', `INDEX DOT PHP`);
+			php.writeFile('/folder/index.html', `INDEX DOT HTML`);
+			const response = await handler.request({
+				url: '/folder/?key=value',
+			});
+			expect(response.httpStatusCode).toEqual(200);
+			expect(response.text).toEqual('INDEX DOT PHP');
+		});
+
+		it('should delegate request for non-existent PHP file to /index.php with query args', async () => {
+			php.writeFile(
+				'/index.php',
+				`<?php echo "DEFAULT with key={$_GET['key']}";`
+			);
+			const response = await handler.request({
+				url: '/non/existent/file.php?key=value',
+			});
+			expect(response.httpStatusCode).toEqual(200);
+			expect(response.text).toEqual('DEFAULT with key=value');
+		});
+
+		it('should delegate request for non-existent non-PHP file to /index.php with query args', async () => {
+			php.writeFile(
+				'/index.php',
+				`<?php echo "DEFAULT with key={$_GET['key']}";`
+			);
+			const response = await handler.request({
+				url: '/non/existent/file?key=value',
+			});
+			expect(response.httpStatusCode).toEqual(200);
+			expect(response.text).toEqual('DEFAULT with key=value');
+		});
+
 		it('should return httpStatus 500 if exit code is not 0', async () => {
 			php.writeFile(
 				'/index.php',
@@ -301,16 +407,6 @@ describe.each(SupportedPHPVersions)(
 			expect(response.httpStatusCode).toEqual(200);
 			expect(response.text).toEqual('value');
 		});
-
-		it('should return 200 and pass query strings when a valid request is made to a folder', async () => {
-			php.mkdirTree('/folder');
-			php.writeFile('/folder/index.php', `<?php echo $_GET['key'];`);
-			const response = await handler.request({
-				url: '/folder/?key=value',
-			});
-			expect(response.httpStatusCode).toEqual(200);
-			expect(response.text).toEqual('value');
-		});
 	}
 );
 

packages/php-wasm/node/src/test/php.spec.ts

@@ -934,6 +934,16 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => {
 			expect(php.isDir(testFilePath)).toEqual(false);
 		});
 
+		it('isFile() should correctly distinguish between a file and a directory', () => {
+			php.writeFile(testFilePath, 'Hello World!');
+			expect(php.fileExists(testFilePath)).toEqual(true);
+			expect(php.isFile(testFilePath)).toEqual(true);
+
+			php.mkdir(testDirPath);
+			expect(php.fileExists(testDirPath)).toEqual(true);
+			expect(php.isFile(testDirPath)).toEqual(false);
+		});
+
 		it('listFiles() should return a list of files in a directory', () => {
 			php.mkdir(testDirPath);
 			php.writeFile(testDirPath + '/test.txt', 'Hello World!');

packages/php-wasm/universal/src/lib/php-request-handler.ts

@@ -38,6 +38,13 @@ interface BaseConfiguration {
 	 * Rewrite rules
 	 */
 	rewriteRules?: RewriteRule[];
+
+	// TODO: Can we reduce coupling between the request handler and WP?
+	/**
+	 * Answers whether the given path is a known remote asset path
+	 * for a minified WP build.
+	 */
+	isKnownRemoteAssetPath?: (wpRelativePath: string) => boolean;
 }
 
 export type PHPRequestHandlerFactoryArgs = PHPFactoryOptions & {
@@ -137,6 +144,8 @@ export class PHPRequestHandler {
 	#cookieStore: HttpCookieStore;
 	rewriteRules: RewriteRule[];
 	processManager: PHPProcessManager;
+	// TODO: Can we reduce coupling between the request handler and WP?
+	isKnownRemoteAssetPath: (wpRelativePath: string) => boolean = () => false;
 
 	/**
 	 * The request handler needs to decide whether to serve a static asset or
@@ -194,6 +203,10 @@ export class PHPRequestHandler {
 			this.#PATHNAME,
 		].join('');
 		this.rewriteRules = rewriteRules;
+
+		if (config.isKnownRemoteAssetPath) {
+			this.isKnownRemoteAssetPath = config.isKnownRemoteAssetPath;
+		}
 	}
 
 	async getPrimaryPhp() {
@@ -306,14 +319,89 @@ export class PHPRequestHandler {
 			),
 			this.rewriteRules
 		);
-		const fsPath = joinPaths(this.#DOCROOT, normalizedRequestedPath);
-		if (!seemsLikeAPHPRequestHandlerPath(fsPath)) {
-			return this.#serveStaticFile(
-				await this.processManager.getPrimaryPhp(),
-				fsPath
-			);
+
+		const primaryPhp = await this.getPrimaryPhp();
+
+		let fsPath = joinPaths(this.#DOCROOT, normalizedRequestedPath);
+
+		if (primaryPhp.isDir(fsPath)) {
+			// Ensure directory URIs have a trailing slash. Otherwise,
+			// relative URIs in index.php or index.html files are relative
+			// to the next directory up.
+			//
+			// Example:
+			// For a request to "/wp-admin", the relative link "edit.php"
+			// resolves to "/edit.php" rather than "/wp-admin/edit.php".
+			//
+			// This is correct behavior for the browser:
+			// https://www.rfc-editor.org/rfc/rfc3986#section-5.2.3
+			//
+			// But the intent for `/wp-admin/index.php` is that its relative
+			// URIs are relative to `/wp-admin/`.
+			//
+			// In fact, WordPress also redirects like this when given a chance.
+			// - https://github.com/WordPress/wordpress-develop/blob/b6a3b9c7d1ce33cbeca6f95871a26c48141e524b/src/wp-includes/canonical.php#L696
+			// - https://github.com/WordPress/wordpress-develop/blob/b6a3b9c7d1ce33cbeca6f95871a26c48141e524b/src/wp-includes/canonical.php#L1036-L1045
+			// - https://github.com/WordPress/wordpress-develop/blob/b6a3b9c7d1ce33cbeca6f95871a26c48141e524b/src/wp-includes/link-template.php#L3558
+			if (!fsPath.endsWith('/')) {
+				return new PHPResponse(
+					301,
+					{ Location: [`${requestedUrl.pathname}/`] },
+					new Uint8Array(0)
+				);
+			}
+
+			// We can only satisfy requests for directories with a default file
+			// so let's first resolve to a default path when available.
+			const defaultFilePath = ['index.php', 'index.html']
+				.map((defaultFilename) => joinPaths(fsPath, defaultFilename))
+				.find((possibleDefaultPath) =>
+					primaryPhp.isFile(possibleDefaultPath)
+				);
+
+			if (defaultFilePath) {
+				fsPath = defaultFilePath;
+			}
+		}
+
+		if (fsPath.endsWith('.php')) {
+			if (primaryPhp.isFile(fsPath)) {
+				const effectiveRequest: PHPRequest = {
+					...request,
+					url: joinPaths(this.#ABSOLUTE_URL, fsPath),
+				};
+				return this.#spawnPHPAndDispatchRequest(
+					effectiveRequest,
+					requestedUrl
+				);
+			}
+		} else {
+			if (primaryPhp.isFile(fsPath)) {
+				return this.#serveStaticFile(primaryPhp, fsPath);
+			} else if (
+				// Make sure fsPath doesn't describe any other entity on the filesystem
+				!primaryPhp.fileExists(fsPath) &&
+				this.isKnownRemoteAssetPath(normalizedRequestedPath)
+			) {
+				// This path is listed as a remote asset. Mark it as a static file
+				// so the service worker knows it can issue a real fetch() to the server.
+				return new PHPResponse(
+					404,
+					{ 'x-file-type': ['static'] },
+					new TextEncoder().encode('404 File not found')
+				);
+			}
 		}
-		return this.#spawnPHPAndDispatchRequest(request, requestedUrl);
+
+		// TODO: Can we adjust this default to reduce coupling between the request handler and WP?
+		// Delegate unresolved requests to WordPress. This makes WP magic possible,
+		// like pretty permalinks and dynamically generated sitemaps.
+		const wpDefaultPath = joinPaths(this.#DOCROOT, 'index.php');
+		const effectiveRequest: PHPRequest = {
+			...request,
+			url: joinPaths(this.#ABSOLUTE_URL, wpDefaultPath),
+		};
+		return this.#spawnPHPAndDispatchRequest(effectiveRequest, requestedUrl);
 	}
 
 	/**
@@ -323,17 +411,6 @@ export class PHPRequestHandler {
 	 * @returns The response.
 	 */
 	#serveStaticFile(php: PHP, fsPath: string): PHPResponse {
-		if (!php.fileExists(fsPath)) {
-			return new PHPResponse(
-				404,
-				// Let the service worker know that no static file was found
-				// and that it's okay to issue a real fetch() to the server.
-				{
-					'x-file-type': ['static'],
-				},
-				new TextEncoder().encode('404 File not found')
-			);
-		}
 		const arrayBuffer = php.readFileAsBuffer(fsPath);
 		return new PHPResponse(
 			200,
@@ -503,35 +580,6 @@ function inferMimeType(path: string): string {
 	return mimeTypes[extension] || mimeTypes['_default'];
 }
 
-/**
- * Guesses whether the given path looks like a PHP file.
- *
- * @example
- * ```js
- * seemsLikeAPHPRequestHandlerPath('/index.php') // true
- * seemsLikeAPHPRequestHandlerPath('/index.php') // true
- * seemsLikeAPHPRequestHandlerPath('/index.php/foo/bar') // true
- * seemsLikeAPHPRequestHandlerPath('/index.html') // false
- * seemsLikeAPHPRequestHandlerPath('/index.html/foo/bar') // false
- * seemsLikeAPHPRequestHandlerPath('/') // true
- * ```
- *
- * @param  path The path to check.
- * @returns Whether the path seems like a PHP server path.
- */
-export function seemsLikeAPHPRequestHandlerPath(path: string): boolean {
-	return seemsLikeAPHPFile(path) || seemsLikeADirectoryRoot(path);
-}
-
-function seemsLikeAPHPFile(path: string) {
-	return path.endsWith('.php') || path.includes('.php/');
-}
-
-function seemsLikeADirectoryRoot(path: string) {
-	const lastSegment = path.split('/').pop();
-	return !lastSegment!.includes('.');
-}
-
 /**
  * Applies the given rewrite rules to the given path.
  *

packages/php-wasm/universal/src/lib/php-worker.ts

@@ -218,6 +218,11 @@ export class PHPWorker implements LimitedPHPApi {
 		return _private.get(this)!.php!.isDir(path);
 	}
 
+	/** @inheritDoc @php-wasm/universal!/PHP.isFile */
+	isFile(path: string): boolean {
+		return _private.get(this)!.php!.isFile(path);
+	}
+
 	/** @inheritDoc @php-wasm/universal!/PHP.fileExists */
 	fileExists(path: string): boolean {
 		return _private.get(this)!.php!.fileExists(path);

packages/php-wasm/universal/src/lib/php.ts

@@ -923,14 +923,24 @@ export class PHP implements Disposable {
 		return FSHelpers.isDir(this[__private__dont__use].FS, path);
 	}
 
-	/**
-	 * Checks if a file (or a directory) exists in the PHP filesystem.
-	 *
-	 * @param  path - The file path to check.
-	 * @returns True if the file exists, false otherwise.
-	 */
-	fileExists(path: string) {
-		return FSHelpers.fileExists(this[__private__dont__use].FS, path);
+	/** @inheritDoc */
+	isFile(path: string): boolean {
+		if (!this.fileExists(path)) {
+			return false;
+		}
+		return this[__private__dont__use].FS.isFile(
+			this[__private__dont__use].FS.lookupPath(path).node.mode
+		);
+	}
+
+	/** @inheritDoc */
+	fileExists(path: string): boolean {
+		try {
+			this[__private__dont__use].FS.lookupPath(path);
+			return true;
+		} catch (e) {
+			return false;
+		}
 	}
 
 	/**