diff --git a/packages/dev-middleware/src/__tests__/FetchUtils.js b/packages/dev-middleware/src/__tests__/FetchUtils.js index 650ce613943..794c669cfeb 100644 --- a/packages/dev-middleware/src/__tests__/FetchUtils.js +++ b/packages/dev-middleware/src/__tests__/FetchUtils.js @@ -14,8 +14,6 @@ import type {RequestOptions} from 'undici'; import {Agent, request} from 'undici'; -declare var globalThis: $FlowFixMe; - /** * A version of `fetch` that is usable with the HTTPS server created in * ServerUtils (which uses a self-signed certificate). @@ -66,27 +64,26 @@ export async function fetchJson(url: string): Promise { * Change the global fetch dispatcher to allow self-signed certificates. * This runs with Jest's `beforeAll` and `afterAll`, and restores the original dispatcher. */ -export function withFetchSelfSignedCertsForAllTests() { - const fetchOriginal = globalThis.fetch; +export function withFetchSelfSignedCertsForAllTests( + fetchSpy: JestMockFn, ReturnType>, + fetchOriginal: typeof fetch, +) { const selfSignedCertDispatcher = new Agent({ connect: { rejectUnauthorized: false, }, }); - let fetchSpy; - beforeAll(() => { // For some reason, setting the `selfSignedCertDispatcher` with `setGlobalDispatcher` doesn't work. // Instead of using `setGlobalDispatcher`, we'll use a spy to intercept the fetch calls and add the dispatcher. - fetchSpy = jest - .spyOn(globalThis, 'fetch') - .mockImplementation((url, options) => - fetchOriginal(url, { - ...options, - dispatcher: options?.dispatcher ?? selfSignedCertDispatcher, - }), - ); + fetchSpy.mockImplementation((url, options) => + fetchOriginal(url, { + ...options, + // $FlowFixMe[prop-missing]: dispatcher + dispatcher: options?.dispatcher ?? selfSignedCertDispatcher, + }), + ); }); afterAll(() => { diff --git a/packages/dev-middleware/src/__tests__/InspectorDeviceUtils.js b/packages/dev-middleware/src/__tests__/InspectorDeviceUtils.js index 2f4d5d69d8a..92cd8793e5d 100644 --- a/packages/dev-middleware/src/__tests__/InspectorDeviceUtils.js +++ b/packages/dev-middleware/src/__tests__/InspectorDeviceUtils.js @@ -27,10 +27,17 @@ export class DeviceAgent { #ws: ?WebSocket; #readyPromise: Promise; - constructor(url: string, signal?: AbortSignal) { + constructor(url: string, signal?: AbortSignal, host?: ?string) { const ws = new WebSocket(url, { // The mock server uses a self-signed certificate. rejectUnauthorized: false, + ...(host != null + ? { + headers: { + Host: host, + }, + } + : {}), }); this.#ws = ws; ws.on('message', data => { @@ -160,8 +167,9 @@ export class DeviceMock extends DeviceAgent { export async function createDeviceMock( url: string, signal: AbortSignal, + host?: ?string, ): Promise { - const device = new DeviceMock(url, signal); + const device = new DeviceMock(url, signal, host); await device.ready(); return device; } diff --git a/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js b/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js index f3ab1e3bfcc..69040c32c49 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js +++ b/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js @@ -126,7 +126,13 @@ export async function createAndConnectTarget( }>, signal: AbortSignal, page: PageFromDevice, - deviceId: ?string = null, + { + deviceId = null, + host = null, + }: $ReadOnly<{ + deviceId?: ?string, + host?: ?string, + }> = {}, ): Promise<{device: DeviceMock, debugger_: DebuggerMock}> { let device; let debugger_; @@ -136,6 +142,7 @@ export async function createAndConnectTarget( deviceId ?? 'device' + Date.now() }&name=foo&app=bar`, signal, + host, ); device.getPages.mockImplementation(() => [page]); diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js index d0cff55f83e..8c8a6fc345f 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js @@ -33,12 +33,18 @@ jest.useRealTimers(); jest.setTimeout(10000); +const fetchOriginal = fetch; +const fetchSpy: JestMockFn< + Parameters, + ReturnType, +> = jest.spyOn(globalThis, 'fetch'); + describe.each(['HTTP', 'HTTPS'])( 'inspector proxy CDP rewriting hacks over %s', protocol => { // Inspector proxy tests are using a self-signed certificate for HTTPS tests. if (protocol === 'HTTPS') { - withFetchSelfSignedCertsForAllTests(); + withFetchSelfSignedCertsForAllTests(fetchSpy, fetchOriginal); } const serverRef = withServerForEachTest({ @@ -46,6 +52,7 @@ describe.each(['HTTP', 'HTTPS'])( projectRoot: __dirname, secure: protocol === 'HTTPS', }); + const autoCleanup = withAbortSignalForEachTest(); afterEach(() => { jest.clearAllMocks(); @@ -188,6 +195,96 @@ describe.each(['HTTP', 'HTTPS'])( } }); + test("does not rewrite urls in Debugger.scriptParsed that don't match the device connection host", async () => { + serverRef.app.use('/source-map', serveStaticJson({version: 3})); + const {device, debugger_} = await createAndConnectTarget( + serverRef, + autoCleanup.signal, + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + }, + { + host: '192.168.0.123:' + serverRef.port, + }, + ); + try { + let fetchCalledWithURL; + fetchSpy.mockImplementationOnce(async url => { + fetchCalledWithURL = url instanceof URL ? url : null; + throw new Error('Unreachable'); + }); + const sourceMapURL = `${protocol.toLowerCase()}://127.0.0.1:${ + serverRef.port + }/source-map`; + const scriptParsedMessage = await sendFromTargetToDebugger( + device, + debugger_, + 'page1', + { + method: 'Debugger.scriptParsed', + params: { + sourceMapURL, + }, + }, + ); + expect(fetchCalledWithURL?.href).toEqual(sourceMapURL); + expect(scriptParsedMessage.params.sourceMapURL).toEqual( + `${protocol.toLowerCase()}://127.0.0.1:${serverRef.port}/source-map`, + ); + } finally { + device.close(); + debugger_.close(); + } + }); + + test('does not rewrite urls in Debugger.scriptParsed that match the device connection host but are not allowlisted for rewriting', async () => { + serverRef.app.use('/source-map', serveStaticJson({version: 3})); + const {device, debugger_} = await createAndConnectTarget( + serverRef, + autoCleanup.signal, + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + }, + { + host: '192.168.0.123:' + serverRef.port, + }, + ); + try { + let fetchCalledWithURL; + fetchSpy.mockImplementationOnce(url => { + fetchCalledWithURL = url instanceof URL ? url : null; + throw new Error('Unreachable'); + }); + const sourceMapURL = `${protocol.toLowerCase()}://192.168.0.123:${ + serverRef.port + }/source-map`; + const scriptParsedMessage = await sendFromTargetToDebugger( + device, + debugger_, + 'page1', + { + method: 'Debugger.scriptParsed', + params: { + sourceMapURL, + }, + }, + ); + expect(fetchCalledWithURL?.href).toEqual(sourceMapURL); + expect(scriptParsedMessage.params.sourceMapURL).toEqual( + `${protocol.toLowerCase()}://192.168.0.123:${serverRef.port}/source-map`, + ); + } finally { + device.close(); + debugger_.close(); + } + }); + describe.each(['10.0.2.2', '10.0.3.2', '127.0.0.1'])( '%s aliasing to and from localhost', sourceHost => { @@ -202,6 +299,9 @@ describe.each(['HTTP', 'HTTPS'])( title: 'bar-title', vm: 'bar-vm', }, + { + host: sourceHost + ':' + serverRef.port, + }, ); try { const scriptParsedMessage = await sendFromTargetToDebugger( @@ -236,6 +336,9 @@ describe.each(['HTTP', 'HTTPS'])( title: 'bar-title', vm: 'bar-vm', }, + { + host: sourceHost + ':' + serverRef.port, + }, ); try { const scriptParsedMessage = await sendFromTargetToDebugger( @@ -284,11 +387,11 @@ describe.each(['HTTP', 'HTTPS'])( method: 'Debugger.setBreakpointByUrl', params: { lineNumber: 1, - urlRegex: 'localhost:1000|localhost:2000', + urlRegex: `localhost:${serverRef.port}|example.com:2000`, }, }); expect(setBreakpointByUrlRegexMessage.params.urlRegex).toEqual( - `${sourceHost}:1000|${sourceHost}:2000`, + `${sourceHost}:${serverRef.port}|example.com:2000`, ); } finally { device.close(); @@ -307,6 +410,9 @@ describe.each(['HTTP', 'HTTPS'])( title: 'bar-title', vm: 'bar-vm', }, + { + host: sourceHost + ':' + serverRef.port, + }, ); try { const response = await debugger_.sendAndGetResponse({ @@ -344,6 +450,9 @@ describe.each(['HTTP', 'HTTPS'])( title: 'bar-title', vm: 'bar-vm', }, + { + host: '127.0.0.1:' + serverRef.port, + }, ); try { const scriptParsedMessage = await sendFromTargetToDebugger( diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index 5ba6f2cd01e..977519a17b0 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -40,7 +40,7 @@ const PAGES_POLLING_INTERVAL = 1000; // Replace hosts appearing in the `url` and `sourceMapURL` fields of // `Debugger.scriptParsed`, and back again in messages from the debugger, // to account for device/debugger/proxy running on different networks. -const REWRITE_HOSTS_TO_LOCALHOST: Array = [ +const REWRITE_HOSTS_TO_LOCALHOST: $ReadOnlySet = new Set([ // A device may retrieve a bundle through 127.0.0.1 via a (SSH) tunnel, but // the (remote) Metro server may be on a host without an IPv4 loopback, so // 127.0.0.1 may not be addressible locally for (e.g., for source map @@ -51,7 +51,7 @@ const REWRITE_HOSTS_TO_LOCALHOST: Array = [ // standard localhost alias. '10.0.2.2', '10.0.3.2', -]; +]); // Prefix for script URLs that are alphanumeric IDs. See comment in #processMessageFromDeviceLegacy method for // more details. @@ -79,6 +79,7 @@ export type DeviceOptions = $ReadOnly<{ projectRoot: string, eventReporter: ?EventReporter, createMessageMiddleware: ?CreateCustomMessageHandlerFn, + deviceRelativeBaseUrl: URL, serverRelativeBaseUrl: URL, }>; @@ -136,6 +137,10 @@ export default class Device { #connectedPageIds: Set = new Set(); + // A base HTTP(S) URL to this server, reachable from the device. Derived from + // the http request that created the connection. + #deviceRelativeBaseUrl: URL; + // A base HTTP(S) URL to the server, relative to this server. #serverRelativeBaseUrl: URL; @@ -152,6 +157,7 @@ export default class Device { eventReporter, createMessageMiddleware, serverRelativeBaseUrl, + deviceRelativeBaseUrl, }: DeviceOptions) { this.#id = id; this.#name = name; @@ -159,6 +165,7 @@ export default class Device { this.#deviceSocket = socket; this.#projectRoot = projectRoot; this.#serverRelativeBaseUrl = serverRelativeBaseUrl; + this.#deviceRelativeBaseUrl = deviceRelativeBaseUrl; this.#deviceEventReporter = eventReporter ? new DeviceEventReporter(eventReporter, { deviceId: id, @@ -676,19 +683,33 @@ export default class Device { const params = payload.params; if ('sourceMapURL' in params) { const sourceMapURL = this.#tryParseHTTPURL(params.sourceMapURL); + if (sourceMapURL) { + // This URL will be used to fetch from the server, and will be + // mutated if necessary from device-relative to server-relative. + // This is not exposed to the debugger. const serverRelativeUrl = new URL(sourceMapURL.href); - for (const hostToRewrite of REWRITE_HOSTS_TO_LOCALHOST) { - if (params.sourceMapURL.includes(hostToRewrite)) { - payload.params.sourceMapURL = params.sourceMapURL.replace( - hostToRewrite, - 'localhost', - ); - debuggerInfo.originalSourceURLAddress = hostToRewrite; - serverRelativeUrl.host = this.#serverRelativeBaseUrl.host; - serverRelativeUrl.protocol = this.#serverRelativeBaseUrl.protocol; - } + // Rewrite device-relative URLs to localhost-relative URLs for the + // debugger. + // TODO: Fix the assumption that localhost:[same port] is correct. + if ( + // sourceMapURL is a device-relative url to the server. + // May or may not be reachable from the frontend. + sourceMapURL.origin === this.#deviceRelativeBaseUrl.origin && + // For a specific set of IPs (eg 10.0.2.2) it's relatively safe to + // assume the frontend can reach the server on localhost. + // TODO: Fix the assumption that localhost:[same port] is correct + // and remove this check. + REWRITE_HOSTS_TO_LOCALHOST.has(this.#deviceRelativeBaseUrl.hostname) + ) { + const debuggerRelativeURL = new URL(sourceMapURL.href); + debuggerRelativeURL.hostname = 'localhost'; + serverRelativeUrl.host = this.#serverRelativeBaseUrl.host; + serverRelativeUrl.protocol = this.#serverRelativeBaseUrl.protocol; + debuggerInfo.originalSourceURLAddress = + this.#deviceRelativeBaseUrl.hostname; + payload.params.sourceMapURL = debuggerRelativeURL.href; } // Some debug clients do not support fetching HTTP URLs. If the @@ -712,21 +733,30 @@ export default class Device { const originalParamsUrl = params.url; let serverRelativeUrl = originalParamsUrl; const parsedUrl = this.#tryParseHTTPURL(originalParamsUrl); - if (parsedUrl) { - for (const hostToRewrite of REWRITE_HOSTS_TO_LOCALHOST) { - if (parsedUrl.hostname === hostToRewrite) { - // URL is device-relative and points to the host - rewrite it to - // use localhost. - parsedUrl.hostname = 'localhost'; - payload.params.url = parsedUrl.href; - debuggerInfo.originalSourceURLAddress = hostToRewrite; + // Rewrite device-relative URLs pointing to the server so that they're + // reachable from the frontend. + if ( + parsedUrl && + // url is a device-relative url to the server. + // May or may not be reachable from the frontend. + parsedUrl.origin === this.#deviceRelativeBaseUrl.origin && + // For a specific set of IPs (eg 10.0.2.2) it's relatively safe to + // assume the frontend can reach the server on localhost. + // TODO: Fix the assumption that localhost:[same port] is correct and + // remove this check. + REWRITE_HOSTS_TO_LOCALHOST.has(this.#deviceRelativeBaseUrl.hostname) + ) { + // URL is device-relative and points to the host - rewrite it to + // use localhost. + parsedUrl.hostname = 'localhost'; + payload.params.url = parsedUrl.href; + debuggerInfo.originalSourceURLAddress = + this.#deviceRelativeBaseUrl.hostname; - // Determine the server-relative URL. - parsedUrl.host = this.#serverRelativeBaseUrl.host; - parsedUrl.protocol = this.#serverRelativeBaseUrl.protocol; - serverRelativeUrl = parsedUrl.href; - } - } + // Determine the server-relative URL. + parsedUrl.host = this.#serverRelativeBaseUrl.host; + parsedUrl.protocol = this.#serverRelativeBaseUrl.protocol; + serverRelativeUrl = parsedUrl.href; } // Chrome doesn't download source maps if URL param is not a valid diff --git a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js index 119357db78c..23a9f7b2d1f 100644 --- a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js +++ b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js @@ -226,7 +226,11 @@ export default class InspectorProxy implements InspectorProxyQueries { const deviceName = query.name || 'Unknown'; const appName = query.app || 'Unknown'; + const deviceRelativeBaseUrl = + getBaseUrlFromRequest(req) ?? this.#serverBaseUrl; + const oldDevice = this.#devices.get(deviceId); + let newDevice; const deviceOptions: DeviceOptions = { id: deviceId, @@ -236,6 +240,7 @@ export default class InspectorProxy implements InspectorProxyQueries { projectRoot: this.#projectRoot, eventReporter: this.#eventReporter, createMessageMiddleware: this.#customMessageHandler, + deviceRelativeBaseUrl, serverRelativeBaseUrl: this.#serverBaseUrl, }; @@ -249,7 +254,7 @@ export default class InspectorProxy implements InspectorProxyQueries { this.#devices.set(deviceId, newDevice); debug( - `Got new connection: name=${deviceName}, app=${appName}, device=${deviceId}`, + `Got new connection: name=${deviceName}, app=${appName}, device=${deviceId}, via=${deviceRelativeBaseUrl.origin}`, ); socket.on('close', () => {