Reject debugger connections to unknown page IDs (#45148)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45148

Changelog: [General][Fixed] Reconnecting to an unknown inspector page no longer results in a zombie connection

TSIA

Reviewed By: hoxyq

Differential Revision: D58954759

fbshipit-source-id: 99c5caccc3cc917e0691e94326c7a35874f9a385
This commit is contained in:
Moti Zilberman
2024-06-24 12:59:53 -07:00
committed by Facebook GitHub Bot
parent 6daccf75da
commit a7adfef0bb
2 changed files with 136 additions and 12 deletions
@@ -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<PageDescription> = [];
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<PageDescription> = [];
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();
}
});
},
);
@@ -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,