diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js index 5c3536be64a..5e2396edb0f 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js @@ -268,5 +268,123 @@ describe.each(['HTTP', 'HTTPS'])( debugger2?.close(); } }); + + test('debugger connection to a nonexistent page is rejected', async () => { + let device, debugger_; + try { + device = await createDeviceMock( + `${serverRef.serverBaseWsUrl}/inspector/device?device=device1&name=foo&app=bar`, + autoCleanup.signal, + ); + // Set up a page. + device.getPages.mockImplementation(() => [ + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + }, + ]); + let pageList: Array = []; + await until(async () => { + pageList = (await fetchJson( + `${serverRef.serverBaseUrl}/json`, + // $FlowIgnore[unclear-type] + ): any); + expect(pageList).toHaveLength(1); + }); + const [{webSocketDebuggerUrl}] = pageList; + + // Connect the debugger to a nonexistent page. + debugger_ = await createDebuggerMock( + webSocketDebuggerUrl.replaceAll('page1', 'some-other-id'), + autoCleanup.signal, + ); + + // The debugger gets disconnected automatically. + await until(async () => { + expect([ + // CLOSING + 3, + // CLOSED + 4, + ]).toContain(debugger_.socket.readyState); + }); + + expect(device.connect).not.toHaveBeenCalled(); + expect(device.disconnect).not.toHaveBeenCalled(); + } finally { + device?.close(); + debugger_?.close(); + } + }); + + test('debugger connection to a nonexistent page does not kill the current debugger connection', async () => { + let device, debugger1, debugger2; + try { + device = await createDeviceMock( + `${serverRef.serverBaseWsUrl}/inspector/device?device=device1&name=foo&app=bar`, + autoCleanup.signal, + ); + // Set up a page. + device.getPages.mockImplementation(() => [ + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + }, + ]); + let pageList: Array = []; + await until(async () => { + pageList = (await fetchJson( + `${serverRef.serverBaseUrl}/json`, + // $FlowIgnore[unclear-type] + ): any); + expect(pageList).toHaveLength(1); + }); + const [{webSocketDebuggerUrl}] = pageList; + + // Connect the first debugger. + debugger1 = await createDebuggerMock( + webSocketDebuggerUrl, + autoCleanup.signal, + ); + + // Connect a second debugger to a nonexistent page. + debugger2 = await createDebuggerMock( + webSocketDebuggerUrl.replaceAll('page1', 'some-other-id'), + autoCleanup.signal, + ); + + // The second debugger gets disconnected automatically. + await until(async () => { + expect([ + // CLOSING + 3, + // CLOSED + 4, + ]).toContain(debugger2.socket.readyState); + }); + + // We can still send messages through the first debugger. + await sendFromDebuggerToTarget(debugger1, device, 'page1', { + method: 'Runtime.enable', + id: 0, + }); + + expect(device.connect).toHaveBeenCalledWith({ + event: 'connect', + payload: { + pageId: 'page1', + }, + }); + expect(device.disconnect).not.toHaveBeenCalled(); + } finally { + device?.close(); + debugger1?.close(); + debugger2?.close(); + } + }); }, ); diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index 7be749d07cd..4d058de8dd2 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -308,17 +308,30 @@ export default class Device { userAgent: string | null, }>, ) { + const page: ?Page = + pageId === REACT_NATIVE_RELOADABLE_PAGE_ID + ? this.#createSyntheticPage() + : this.#pages.get(pageId); + + if (!page) { + debug( + `Got new debugger connection for page ${pageId} of ${this.#name}, but no such page exists`, + ); + socket.close(); + return; + } + // Clear any commands we were waiting on. this.#deviceEventReporter?.logDisconnection('debugger'); + // Disconnect current debugger if we already have debugger connected. + this.#terminateDebuggerConnection(); + this.#deviceEventReporter?.logConnection('debugger', { pageId, frontendUserAgent: metadata.userAgent, }); - // Disconnect current debugger if we already have debugger connected. - this.#terminateDebuggerConnection(); - const debuggerInfo = { socket, prependedFilePrefix: false, @@ -327,18 +340,11 @@ export default class Device { customHandler: null, }; - // TODO(moti): Handle null case explicitly, e.g. refuse to connect to - // unknown pages. - const page: ?Page = - pageId === REACT_NATIVE_RELOADABLE_PAGE_ID - ? this.#createSyntheticPage() - : this.#pages.get(pageId); - this.#debuggerConnection = debuggerInfo; debug(`Got new debugger connection for page ${pageId} of ${this.#name}`); - if (page && this.#debuggerConnection && this.#createCustomMessageHandler) { + if (this.#debuggerConnection && this.#createCustomMessageHandler) { this.#debuggerConnection.customHandler = this.#createCustomMessageHandler( { page, @@ -405,7 +411,7 @@ export default class Device { return; } - if (!page || !this.#pageHasCapability(page, 'nativeSourceCodeFetching')) { + if (!this.#pageHasCapability(page, 'nativeSourceCodeFetching')) { processedReq = this.#interceptClientMessageForSourceFetching( debuggerRequest, debuggerInfo,