From b8e42ea7b4f5281339d8c75f2fd5d384ace82da4 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Thu, 14 Sep 2023 06:08:57 -0700 Subject: [PATCH] Build client-accessible frontend URLs in /open-debugger (#39423) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39423 D49158227 made the CDP WebSocket URLs returned from `/json` proxy-safe by using the Host header (etc) from the HTTP request. This did *not* fully fix the related `/open-debugger` endpoint, because the original headers were being lost in the internal `fetch('/json')` call. Here, we eliminate that `fetch` call, and instead give the `/open-debugger` handler direct access to the method on `InspectorProxy` that backs the `/json` endpoint. This allows us to pass in an appropriate base URL derived from the headers seen by `/open-debugger`. Changelog: [Internal] Reviewed By: huntie Differential Revision: D49229545 fbshipit-source-id: 9036ab295721e0d1fd3cdb608d0a7cc07b8f2eeb --- .../dev-middleware/src/createDevMiddleware.js | 3 +- .../src/inspector-proxy/InspectorProxy.js | 52 ++++++++++++------- .../src/middleware/openDebuggerMiddleware.js | 16 ++++-- .../src/utils/queryInspectorTargets.js | 40 -------------- 4 files changed, 48 insertions(+), 63 deletions(-) delete mode 100644 packages/dev-middleware/src/utils/queryInspectorTargets.js diff --git a/packages/dev-middleware/src/createDevMiddleware.js b/packages/dev-middleware/src/createDevMiddleware.js index 23fdef131f..9391a7fe47 100644 --- a/packages/dev-middleware/src/createDevMiddleware.js +++ b/packages/dev-middleware/src/createDevMiddleware.js @@ -56,10 +56,11 @@ export default function createDevMiddleware({ .use( '/open-debugger', openDebuggerMiddleware({ - logger, browserLauncher: unstable_browserLauncher, eventReporter: unstable_eventReporter, experiments, + inspectorProxy, + logger, }), ) .use( diff --git a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js index 18a15004b7..6a00308dca 100644 --- a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js +++ b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js @@ -34,10 +34,18 @@ const PAGES_LIST_JSON_VERSION_URL = '/json/version'; const INTERNAL_ERROR_CODE = 1011; +export interface InspectorProxyQueries { + getPageDescriptions( + options: $ReadOnly<{ + wsPublicBaseUrl: string, + }>, + ): Array; +} + /** * Main Inspector Proxy class that connects JavaScript VM inside Android/iOS apps and JS debugger. */ -export default class InspectorProxy { +export default class InspectorProxy implements InspectorProxyQueries { // Root of the project used for relative to absolute source path conversion. _projectRoot: string; @@ -62,6 +70,25 @@ export default class InspectorProxy { this._experiments = experiments; } + getPageDescriptions({ + wsPublicBaseUrl, + }: $ReadOnly<{ + wsPublicBaseUrl: string, + }>): Array { + // Build list of pages from all devices. + let result: Array = []; + Array.from(this._devices.entries()).forEach(([deviceId, device]) => { + result = result.concat( + device + .getPagesList() + .map((page: Page) => + this._buildPageDescription(deviceId, device, page, wsPublicBaseUrl), + ), + ); + }); + return result; + } + // Process HTTP request sent to server. We only respond to 2 HTTP requests: // 1. /json/version returns Chrome debugger protocol version that we use // 2. /json and /json/list returns list of page descriptions (list of inspectable apps). @@ -76,24 +103,13 @@ export default class InspectorProxy { request.url === PAGES_LIST_JSON_URL_2 ) { const wsPublicBaseUrl = getDevServerUrl(request, 'public', 'ws'); - // Build list of pages from all devices. - let result: Array = []; - Array.from(this._devices.entries()).forEach(([deviceId, device]) => { - result = result.concat( - device - .getPagesList() - .map((page: Page) => - this._buildPageDescription( - deviceId, - device, - page, - wsPublicBaseUrl, - ), - ), - ); - }); - this._sendJsonResponse(response, result); + this._sendJsonResponse( + response, + this.getPageDescriptions({ + wsPublicBaseUrl, + }), + ); } else if (request.url === PAGES_LIST_JSON_VERSION_URL) { this._sendJsonResponse(response, { Browser: 'Mobile JavaScript', diff --git a/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js b/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js index 610f04dea4..f8b9bc8adb 100644 --- a/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js +++ b/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js @@ -11,6 +11,7 @@ import type {NextHandleFunction} from 'connect'; import type {IncomingMessage, ServerResponse} from 'http'; +import type {InspectorProxyQueries} from '../inspector-proxy/InspectorProxy'; import type {BrowserLauncher, LaunchedBrowser} from '../types/BrowserLauncher'; import type {EventReporter} from '../types/EventReporter'; import type {Experiments} from '../types/Experiments'; @@ -19,7 +20,6 @@ import type {Logger} from '../types/Logger'; import url from 'url'; import getDevServerUrl from '../utils/getDevServerUrl'; import getDevToolsFrontendUrl from '../utils/getDevToolsFrontendUrl'; -import queryInspectorTargets from '../utils/queryInspectorTargets'; const debuggerInstances = new Map(); @@ -28,6 +28,7 @@ type Options = $ReadOnly<{ logger?: Logger, eventReporter?: EventReporter, experiments: Experiments, + inspectorProxy: InspectorProxyQueries, }>; /** @@ -42,6 +43,7 @@ export default function openDebuggerMiddleware({ browserLauncher, eventReporter, experiments, + inspectorProxy, logger, }: Options): NextHandleFunction { return async ( @@ -53,9 +55,15 @@ export default function openDebuggerMiddleware({ const {query} = url.parse(req.url, true); const {appId} = query; - const targets = await queryInspectorTargets( - getDevServerUrl(req, 'local'), - ); + const targets = inspectorProxy + .getPageDescriptions({ + wsPublicBaseUrl: getDevServerUrl(req, 'public', 'ws'), + }) + .filter( + // Only use targets with better reloading support + app => + app.title === 'React Native Experimental (Improved Chrome Reloads)', + ); let target; if (typeof appId === 'string') { diff --git a/packages/dev-middleware/src/utils/queryInspectorTargets.js b/packages/dev-middleware/src/utils/queryInspectorTargets.js deleted file mode 100644 index 3aa0f72443..0000000000 --- a/packages/dev-middleware/src/utils/queryInspectorTargets.js +++ /dev/null @@ -1,40 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow strict - * @format - * @oncall react_native - */ - -import fetch from 'node-fetch'; - -type ReactNativeCDPTarget = { - id: string, - description: string, - title: string, - type: string, - devtoolsFrontendUrl: string, - webSocketDebuggerUrl: string, - vm: string, - deviceName?: string, -}; - -/** - * Get the list of available debug targets from the React Native dev server. - * - * @see https://chromedevtools.github.io/devtools-protocol/ - */ -export default async function queryInspectorTargets( - devServerUrl: string, -): Promise { - const res = await fetch(`${devServerUrl}/json/list`); - const apps = (await res.json(): Array); - - // Only use targets with better reloading support - return apps.filter( - app => app.title === 'React Native Experimental (Improved Chrome Reloads)', - ); -}