Skip to content

Commit 3000aef

Browse files
authored
Request Handler: Serve static files if they exist in VFS. Don't guess. (#702)
Replaces approximate rules such as ```js if(path.startsWith('/wp-content')) { // serve from VFS } else { // serve from remote server } ``` With actual file existence check. ## Context When a static file is requested from Playground, the service worker doesn't know whether to serve it from VFS or playground.wordpress.net. It tries to guess that based on approximate rules such as `path.startsWith('/wp-content')`. This usually works, but is not very reliable. For example, when you store an entirely new WordPress instance in VFS, you would expect all the static assets to be served from VFS, but that is not what the request handler would do. This does not play well with previewing WordPress Pull Requests as we're actually replacing the entire WordPress instance in VFS with an arbitrary branch. Related to #700 Closes #109 ## Testing Instructions 1. Start Playground `npm run start` 2. Use it for a bit, go to wp-admin, confirm that static assets like CSS and JS files are loaded without issues 3. Upload something, confirm that worked too ## Breaking change The `isStaticFilePath` option is removed in this PR.. This is a breaking change in public API and may be relevant for wp-now CC @sejas @danielbachhuber. While that sounds scary, everything should "just work" without major changes required in wp-now, other than removing this usage of `isStaticFilePath`: https://github.com/WordPress/playground-tools/blob/d792d7f03323aa0ec34e5885e4aebd5741d96f24/packages/wp-now/src/wp-now.ts#L53-L65
1 parent 63e3b72 commit 3000aef

File tree

9 files changed

+91
-80
lines changed

9 files changed

+91
-80
lines changed

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ describe.each(SupportedPHPVersions)(
1616
php = new NodePHP(runtimeId);
1717
handler = new PHPRequestHandler(php, {
1818
documentRoot: '/',
19-
isStaticFilePath: (path) => !path.endsWith('.php'),
2019
});
2120
});
2221

@@ -57,6 +56,34 @@ describe.each(SupportedPHPVersions)(
5756
});
5857
});
5958

59+
it('should yield x-file-type=static when a static file is not found', async () => {
60+
const response = await handler.request({
61+
url: '/index.html',
62+
});
63+
expect(response).toEqual({
64+
httpStatusCode: 404,
65+
headers: {
66+
'x-file-type': ['static'],
67+
},
68+
bytes: expect.any(Uint8Array),
69+
errors: '',
70+
exitCode: 0,
71+
});
72+
});
73+
74+
it('should not yield x-file-type=static when a PHP file is not found', async () => {
75+
const response = await handler.request({
76+
url: '/index.php',
77+
});
78+
expect(response).toEqual({
79+
httpStatusCode: 404,
80+
headers: {},
81+
bytes: expect.any(Uint8Array),
82+
errors: '',
83+
exitCode: 0,
84+
});
85+
});
86+
6087
it('should only handle a single PHP request at a time', async () => {
6188
php.writeFile(
6289
'/index.php',
@@ -171,8 +198,6 @@ describe.each(SupportedPHPVersions)(
171198
php.mkdirTree('/var/www');
172199
handler = new PHPRequestHandler(php, {
173200
documentRoot: '/var/www',
174-
// Treat all files as dynamic
175-
isStaticFilePath: () => false,
176201
});
177202
});
178203

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

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,6 @@ export interface PHPRequestHandlerConfiguration {
2424
* Request Handler URL. Used to populate $_SERVER details like HTTP_HOST.
2525
*/
2626
absoluteUrl?: string;
27-
/**
28-
* Callback used by the PHPRequestHandler to decide whether
29-
* the requested path refers to a PHP file or a static file.
30-
*/
31-
isStaticFilePath?: (path: string) => boolean;
3227
}
3328

3429
/** @inheritDoc */
@@ -46,7 +41,6 @@ export class PHPRequestHandler implements RequestHandler {
4641
* The PHP instance
4742
*/
4843
php: BasePHP;
49-
#isStaticFilePath: (path: string) => boolean;
5044

5145
/**
5246
* @param php - The PHP instance.
@@ -57,11 +51,9 @@ export class PHPRequestHandler implements RequestHandler {
5751
const {
5852
documentRoot = '/www/',
5953
absoluteUrl = typeof location === 'object' ? location?.href : '',
60-
isStaticFilePath = () => false,
6154
} = config;
6255
this.php = php;
6356
this.#DOCROOT = documentRoot;
64-
this.#isStaticFilePath = isStaticFilePath;
6557

6658
const url = new URL(absoluteUrl);
6759
this.#HOSTNAME = url.hostname;
@@ -122,29 +114,32 @@ export class PHPRequestHandler implements RequestHandler {
122114
isAbsolute ? undefined : DEFAULT_BASE_URL
123115
);
124116

125-
const normalizedRelativeUrl = removePathPrefix(
117+
const normalizedRequestedPath = removePathPrefix(
126118
requestedUrl.pathname,
127119
this.#PATHNAME
128120
);
129-
if (this.#isStaticFilePath(normalizedRelativeUrl)) {
130-
return this.#serveStaticFile(normalizedRelativeUrl);
121+
const fsPath = `${this.#DOCROOT}${normalizedRequestedPath}`;
122+
if (seemsLikeAPHPRequestHandlerPath(fsPath)) {
123+
return await this.#dispatchToPHP(request, requestedUrl);
131124
}
132-
return await this.#dispatchToPHP(request, requestedUrl);
125+
return this.#serveStaticFile(fsPath);
133126
}
134127

135128
/**
136129
* Serves a static file from the PHP filesystem.
137130
*
138-
* @param path - The requested static file path.
131+
* @param fsPath - Absolute path of the static file to serve.
139132
* @returns The response.
140133
*/
141-
#serveStaticFile(path: string): PHPResponse {
142-
const fsPath = `${this.#DOCROOT}${path}`;
143-
134+
#serveStaticFile(fsPath: string): PHPResponse {
144135
if (!this.php.fileExists(fsPath)) {
145136
return new PHPResponse(
146137
404,
147-
{},
138+
// Let the service worker know that no static file was found
139+
// and that it's okay to issue a real fetch() to the server.
140+
{
141+
'x-file-type': ['static'],
142+
},
148143
new TextEncoder().encode('404 File not found')
149144
);
150145
}
@@ -394,3 +389,32 @@ function inferMimeType(path: string): string {
394389
return 'application-octet-stream';
395390
}
396391
}
392+
393+
/**
394+
* Guesses whether the given path looks like a PHP file.
395+
*
396+
* @example
397+
* ```js
398+
* seemsLikeAPHPRequestHandlerPath('/index.php') // true
399+
* seemsLikeAPHPRequestHandlerPath('/index.php') // true
400+
* seemsLikeAPHPRequestHandlerPath('/index.php/foo/bar') // true
401+
* seemsLikeAPHPRequestHandlerPath('/index.html') // false
402+
* seemsLikeAPHPRequestHandlerPath('/index.html/foo/bar') // false
403+
* seemsLikeAPHPRequestHandlerPath('/') // true
404+
* ```
405+
*
406+
* @param path The path to check.
407+
* @returns Whether the path seems like a PHP server path.
408+
*/
409+
export function seemsLikeAPHPRequestHandlerPath(path: string): boolean {
410+
return seemsLikeAPHPFile(path) || seemsLikeADirectoryRoot(path);
411+
}
412+
413+
function seemsLikeAPHPFile(path: string) {
414+
return path.endsWith('.php') || path.includes('.php/');
415+
}
416+
417+
function seemsLikeADirectoryRoot(path: string) {
418+
const lastSegment = path.split('/').pop();
419+
return !lastSegment!.includes('.');
420+
}

packages/php-wasm/web-service-worker/src/initialize-service-worker.ts

Lines changed: 10 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,17 @@ export function initializeServiceWorker(config: ServiceWorkerConfiguration) {
6161
async function defaultRequestHandler(event: FetchEvent) {
6262
event.preventDefault();
6363
const url = new URL(event.request.url);
64-
const unscopedUrl = removeURLScope(url);
65-
if (!seemsLikeAPHPRequestHandlerPath(unscopedUrl.pathname)) {
66-
return fetch(
67-
await cloneRequest(event.request, {
68-
url,
69-
})
70-
);
64+
const workerResponse = await convertFetchEventToPHPRequest(event);
65+
if (
66+
workerResponse.status === 404 &&
67+
workerResponse.headers.get('x-file-type') === 'static'
68+
) {
69+
const request = await cloneRequest(event.request, {
70+
url,
71+
});
72+
return fetch(request);
7173
}
72-
return convertFetchEventToPHPRequest(event);
74+
return workerResponse;
7375
}
7476

7577
export async function convertFetchEventToPHPRequest(event: FetchEvent) {
@@ -192,35 +194,6 @@ interface ServiceWorkerConfiguration {
192194
handleRequest?: (event: FetchEvent) => Promise<Response> | undefined;
193195
}
194196

195-
/**
196-
* Guesses whether the given path looks like a PHP file.
197-
*
198-
* @example
199-
* ```js
200-
* seemsLikeAPHPRequestHandlerPath('/index.php') // true
201-
* seemsLikeAPHPRequestHandlerPath('/index.php') // true
202-
* seemsLikeAPHPRequestHandlerPath('/index.php/foo/bar') // true
203-
* seemsLikeAPHPRequestHandlerPath('/index.html') // false
204-
* seemsLikeAPHPRequestHandlerPath('/index.html/foo/bar') // false
205-
* seemsLikeAPHPRequestHandlerPath('/') // true
206-
* ```
207-
*
208-
* @param path The path to check.
209-
* @returns Whether the path seems like a PHP server path.
210-
*/
211-
export function seemsLikeAPHPRequestHandlerPath(path: string): boolean {
212-
return seemsLikeAPHPFile(path) || seemsLikeADirectoryRoot(path);
213-
}
214-
215-
function seemsLikeAPHPFile(path: string) {
216-
return path.endsWith('.php') || path.includes('.php/');
217-
}
218-
219-
function seemsLikeADirectoryRoot(path: string) {
220-
const lastSegment = path.split('/').pop();
221-
return !lastSegment!.includes('.');
222-
}
223-
224197
async function rewritePost(request: Request) {
225198
const contentType = request.headers.get('content-type')!;
226199
if (request.method !== 'POST') {

packages/playground/blueprints/src/lib/compile.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ describe('Blueprints', () => {
1616
php = await NodePHP.load(phpVersion, {
1717
requestHandler: {
1818
documentRoot: '/',
19-
isStaticFilePath: (path) => !path.endsWith('.php'),
2019
},
2120
});
2221
});

packages/playground/blueprints/src/lib/steps/install-plugin.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ describe('Blueprint step installPlugin', () => {
88
php = await NodePHP.load(phpVersion, {
99
requestHandler: {
1010
documentRoot: '/wordpress',
11-
isStaticFilePath: (path) => !path.endsWith('.php'),
1211
},
1312
});
1413
});

packages/playground/blueprints/src/lib/steps/install-theme.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ describe('Blueprint step installTheme', () => {
88
php = await NodePHP.load(phpVersion, {
99
requestHandler: {
1010
documentRoot: '/wordpress',
11-
isStaticFilePath: (path) => !path.endsWith('.php'),
1211
},
1312
});
1413
});

packages/playground/remote/service-worker.ts

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,9 @@ import {
77
awaitReply,
88
convertFetchEventToPHPRequest,
99
initializeServiceWorker,
10-
seemsLikeAPHPRequestHandlerPath,
1110
cloneRequest,
1211
broadcastMessageExpectReply,
1312
} from '@php-wasm/web-service-worker';
14-
import { isUploadedFilePath } from './src/lib/is-uploaded-file-path';
1513

1614
if (!(self as any).document) {
1715
// Workaround: vite translates import.meta.url
@@ -40,23 +38,21 @@ initializeServiceWorker({
4038
}
4139
event.preventDefault();
4240
async function asyncHandler() {
43-
const { staticAssetsDirectory, defaultTheme } =
44-
await getScopedWpDetails(scope!);
41+
const { staticAssetsDirectory } = await getScopedWpDetails(scope!);
42+
const workerResponse = await convertFetchEventToPHPRequest(event);
4543
if (
46-
(seemsLikeAPHPRequestHandlerPath(unscopedUrl.pathname) ||
47-
isUploadedFilePath(unscopedUrl.pathname)) &&
48-
!unscopedUrl.pathname.startsWith(
49-
`/wp-content/themes/${defaultTheme}`
50-
)
44+
workerResponse.status === 404 &&
45+
workerResponse.headers.get('x-file-type') === 'static'
5146
) {
52-
const response = await convertFetchEventToPHPRequest(event);
53-
return response;
47+
// If we get a 404 for a static file, try to fetch it from
48+
// the from the static assets directory at the remote server.
49+
const request = await rewriteRequest(
50+
event.request,
51+
staticAssetsDirectory
52+
);
53+
return fetch(request);
5454
}
55-
const request = await rewriteRequest(
56-
event.request,
57-
staticAssetsDirectory
58-
);
59-
return fetch(request);
55+
return workerResponse;
6056
}
6157
return asyncHandler();
6258
},

packages/playground/remote/src/lib/is-uploaded-file-path.ts

Lines changed: 0 additions & 2 deletions
This file was deleted.

packages/playground/remote/src/lib/worker-thread.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { WebPHP, WebPHPEndpoint, exposeAPI } from '@php-wasm/web';
22
import { EmscriptenDownloadMonitor } from '@php-wasm/progress';
33
import { setURLScope } from '@php-wasm/scopes';
44
import { DOCROOT, wordPressSiteUrl } from './config';
5-
import { isUploadedFilePath } from './is-uploaded-file-path';
65
import {
76
getWordPressModule,
87
LatestSupportedWordPressVersion,
@@ -85,7 +84,6 @@ const { php, phpReady } = WebPHP.loadSync(phpVersion, {
8584
requestHandler: {
8685
documentRoot: DOCROOT,
8786
absoluteUrl: scopedSiteUrl,
88-
isStaticFilePath: isUploadedFilePath,
8987
},
9088
// We don't yet support loading specific PHP extensions one-by-one.
9189
// Let's just indicate whether we want to load all of them.

0 commit comments

Comments
 (0)