From 218c33642928dcdbe3ff89cf8657ef582d0b867d Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sun, 17 Nov 2024 13:29:11 -0800 Subject: [PATCH] dev-middleware: Block Network.loadNetworkResource requests when nativeSourceCodeFetching == false (#47652) Summary: When the `nativeSourceCodeFetching` capability is disabled, `inspector-proxy` rewrites URLs exchanged over CDP between device and frontend so that URLs are addressable from CDT - in particular, when using an Android emulator `10.0.2.2` (host's address from within the emulator) is rewritten to and from `localhost` (the equivalent address reachable from the host). Previously - before we implemented `Network.loadNetworkResource`, or on old frontends that don't attempt to use that method - this worked reasonably well. A `fetch` from CDT to Metro would succeed on the rewritten URL. Since we implemented `Network.loadNetworkResource`, but disabled the `nativeSourceCodeFetching` capability, source fetching is broken under Android emulators. We're rewriting URLs to be frontend-relative, but then attempting to fetch them through the device, because as far as CDT is aware, `Network.loadNetworkResource` should still be tried first. When `Network.loadNetworkResource` responds with a CDP *error*, CDT falls back to a local fetch (which would work), but when it responds with a CDP *result* of `success: false`, there is no fallback. This diff adds an interception guarded behind `nativeSourceCodeFetching == false`, which rejects any calls to `Network.loadNetworkResource` with a CDP error. This restores the previous behaviour from before `Network.loadNetworkResource` was implemented at all. NOTE: An alternative approach would be to rewrite URLs back to device-relative for `Network.loadNetworkResource`, but IMO it's more correct for the frontend to respect that the device is asserting that it doesn't have that capability, and not to try to use it. Changelog: [Android][Fixed] RN DevTools: Fix source loading when using an Android emulator connecting to a dev server on the host. Differential Revision: D66074731 --- .../InspectorProxyCdpRewritingHacks-test.js | 63 +++++++++++++++++++ .../src/inspector-proxy/Device.js | 25 ++++++++ .../src/inspector-proxy/cdp-types/messages.js | 1 + 3 files changed, 89 insertions(+) diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js index bf7656843a2..d0cff55f83e 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js @@ -295,6 +295,42 @@ describe.each(['HTTP', 'HTTPS'])( debugger_.close(); } }); + + describe('Network.loadNetworkResource', () => { + test('should respond with an error without forwarding to the client', async () => { + const {device, debugger_} = await createAndConnectTarget( + serverRef, + autoCleanup.signal, + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + }, + ); + try { + const response = await debugger_.sendAndGetResponse({ + id: 1, + method: 'Network.loadNetworkResource', + params: { + url: 'http://example.com', + }, + }); + expect(response.result).toEqual( + expect.objectContaining({ + error: { + code: -32601, + message: + '[inspector-proxy]: Page lacks nativeSourceCodeFetching capability.', + }, + }), + ); + } finally { + device.close(); + debugger_.close(); + } + }); + }); }, ); @@ -581,6 +617,33 @@ describe.each(['HTTP', 'HTTPS'])( } }); }); + + describe('Network.loadNetworkResource', () => { + test('should forward event directly to client (does not rewrite url host)', async () => { + const {device, debugger_} = await createAndConnectTarget( + serverRef, + autoCleanup.signal, + pageDescription, + ); + try { + const message = { + id: 1, + method: 'Network.loadNetworkResource', + params: { + url: `${protocol.toLowerCase()}://10.0.2.2:${serverRef.port}`, + }, + }; + await sendFromDebuggerToTarget(debugger_, device, 'page1', message); + expect(device.wrappedEventParsed).toBeCalledWith({ + pageId: 'page1', + wrappedEvent: message, + }); + } finally { + device.close(); + debugger_.close(); + } + }); + }); }); }, ); diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index 88b82f5995f..54fe5397fce 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -799,6 +799,31 @@ export default class Device { // Sends response to debugger via side-effect this.#processDebuggerGetScriptSource(req, socket); return null; + case 'Network.loadNetworkResource': + // If we're rewriting URLs (to frontend-relative), we don't want to + // pass these URLs to the device, since it may try to fetch, return a + // CDP *result* (not error) with a network failure, and CDT + // will *not* then fall back to fetching locally. + // + // Instead, take the absence of a nativeSourceCodeFetching + // capability as a signal to never pass a loadNetworkResource request + // to the device. By returning a CDP error, the frontend should fetch. + const result = { + error: { + code: -32601, // Method not found + message: + '[inspector-proxy]: Page lacks nativeSourceCodeFetching capability.', + }, + }; + const response = {id: req.id, result}; + socket.send(JSON.stringify(response)); + const pageId = this.#debuggerConnection?.pageId ?? null; + this.#deviceEventReporter?.logResponse(response, 'proxy', { + pageId, + frontendUserAgent: this.#debuggerConnection?.userAgent ?? null, + prefersFuseboxFrontend: this.#isPageFuseboxFrontend(pageId), + }); + return null; default: return req; } diff --git a/packages/dev-middleware/src/inspector-proxy/cdp-types/messages.js b/packages/dev-middleware/src/inspector-proxy/cdp-types/messages.js index b019728d15d..67ffef60e6c 100644 --- a/packages/dev-middleware/src/inspector-proxy/cdp-types/messages.js +++ b/packages/dev-middleware/src/inspector-proxy/cdp-types/messages.js @@ -44,6 +44,7 @@ export type CDPClientMessage = | CDPRequest<'Debugger.getScriptSource'> | CDPRequest<'Debugger.scriptParsed'> | CDPRequest<'Debugger.setBreakpointByUrl'> + | CDPRequest<'Network.loadNetworkResource'> | CDPRequest<>; export type CDPServerMessage =