From c340b0d34c361abe39a40dc5b3a66c2b7a57ac13 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Date: Tue, 9 Apr 2024 13:06:26 +0200 Subject: [PATCH] Catch DNS errors to avoid unhandled exceptions (#1215) Catches potential DNS errors produced in outbound network requests to avoid unhandled exceptions. ## What problem is it solving? Fixes the problem described in https://github.com/WordPress/wordpress-playground/issues/535. ## How is the problem addressed? The PR introduces a `try-catch` statement to catch errors produced when invoking `lookup` to resolve the target host address. When this happens, it sends empty binary data as a notification that the connection initiated but soon after, it closes the connection with error code `3000`. ## Testing Instructions The [original issue](https://github.com/WordPress/wordpress-playground/issues/535) references `wp-now`, so it's recommended to test the changes using a local build of `wp-now` from the [`playground-tools` repository](https://github.com/WordPress/playground-tools). 1. In `wordpress-playground` repository, run `nx build php-wasm-node`. 2. Check out [`playground-tools` repository](https://github.com/WordPress/playground-tools). 3. In `playground-rools` repository, update the `@php-wasm/node` dependency to point to the local path of the `wordpress-playground` repository. **Example:** ```patch diff --git forkSrcPrefix/package.json forkDstPrefix/package.json index cc623dc57810509691383fa068cc8f921c957f48..ac84b7f467c2f7d23b9fa8c5f9487cafdb4644e6 100644 --- forkSrcPrefix/package.json +++ forkDstPrefix/package.json @@ -32,7 +32,7 @@ "@codemirror/state": "6.2.0", "@codemirror/theme-one-dark": "6.1.1", "@codemirror/view": "6.9.3", - "@php-wasm/node": "0.6.10", + "@php-wasm/node": "file:../wordpress-playground/dist/packages/php-wasm/node", "@php-wasm/progress": "0.6.10", "@php-wasm/universal": "0.6.10", "@php-wasm/web": "0.6.10", ``` 4. Run `npm i`. 5. Run `nx build wp-now`. 6. Run `node dist/packages/wp-now/cli.js start`. 7. Turn off internet connection. 8. Navigate to the site's WP admin (`/wp-admin`). 9. Observe no errors are logged and that `wp-now` doesn't crash. 10. Navigate to `/wp-admin/plugin-install.php`. 11. Observe no errors are logged and that `wp-now` doesn't crash. 12. Observe that the page shows an error message due to being offline. 13. Navigate to `/wp-admin/update-core.php` 14. Observe no errors are logged and that `wp-now` doesn't crash. > [!NOTE] > It's recommended to disable the network cache in your browser via the DevTools Network tab. --- .../lib/networking/outbound-ws-to-tcp-proxy.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/php-wasm/node/src/lib/networking/outbound-ws-to-tcp-proxy.ts b/packages/php-wasm/node/src/lib/networking/outbound-ws-to-tcp-proxy.ts index cf89e08b86..d35782fa37 100644 --- a/packages/php-wasm/node/src/lib/networking/outbound-ws-to-tcp-proxy.ts +++ b/packages/php-wasm/node/src/lib/networking/outbound-ws-to-tcp-proxy.ts @@ -185,7 +185,9 @@ async function onWsConnect(client: any, request: http.IncomingMessage) { clientLog( 'WebSocket client disconnected: ' + code + ' [' + reason + ']' ); - target.end(); + if (target) { + target.end(); + } } as any); client.on('error', function (a: string | Buffer) { clientLog('WebSocket client error: ' + a); @@ -196,9 +198,17 @@ async function onWsConnect(client: any, request: http.IncomingMessage) { let reqTargetIp; if (net.isIP(reqTargetHost) === 0) { clientLog('resolving ' + reqTargetHost + '... '); - const resolution = await lookup(reqTargetHost); - reqTargetIp = resolution.address; - clientLog('resolved ' + reqTargetHost + ' -> ' + reqTargetIp); + try { + const resolution = await lookup(reqTargetHost); + reqTargetIp = resolution.address; + clientLog('resolved ' + reqTargetHost + ' -> ' + reqTargetIp); + } catch (e) { + clientLog("can't resolve " + reqTargetHost + ' due to:', e); + // Send empty binary data to notify requester that connection was initiated + client.send([]); + client.close(3000); + return; + } } else { reqTargetIp = reqTargetHost; }