dev-middleware: Only rewrite hostnames if they match device connection hosts (#47685)

Summary:

Currently, we assume any URL with a hostname of `10.0.2.2` or `10.0.3.2` (device-relative) is eligible for rewriting to `localhost` (frontend-relative), because we assume the device is an Android emulator. We rewrite these URLs between device and dev machine so that the rewritten URLs are reachable from the dev machine.

This diff narrows this logic so that we'll only rewrite URLs where the hostname matches the pre-existing list *and* this matches the host the device is actually connected on, according to its headers from the original connection.

The main motivation for this change is to unblock removing assumptions about device-reachable vs server-reachable hosts. Later in the stack we'll drop the hardcoded listing of `10.0.2.2` etc in favour of identifying URLs that target the dev server, from whatever network.

There's also an edge case fix here that `10.0.2.2` etc might actually refer to a remote LAN server, and not be an Android emulator's alias for for an emulator host.

Changelog:
[General][Fixed] RN DevTools: Don't assume 10.0.2.2 is an alias for localhost unless it's used to establish a connection to the server

Reviewed By: huntie

Differential Revision: D66058704

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<T: JSONSerializable>(url: string): Promise<T> {
  * 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<Parameters<typeof fetch>, ReturnType<typeof fetch>>,
+  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(() => {

packages/dev-middleware/src/__tests__/InspectorDeviceUtils.js

@@ -27,10 +27,17 @@ export class DeviceAgent {
   #ws: ?WebSocket;
   #readyPromise: Promise<void>;
 
-  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<DeviceMock> {
-  const device = new DeviceMock(url, signal);
+  const device = new DeviceMock(url, signal, host);
   await device.ready();
   return device;
 }

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]);
 

packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js

@@ -33,12 +33,18 @@ jest.useRealTimers();
 
 jest.setTimeout(10000);
 
+const fetchOriginal = fetch;
+const fetchSpy: JestMockFn<
+  Parameters<typeof fetch>,
+  ReturnType<typeof fetch>,
+> = 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(

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<string> = [
+const REWRITE_HOSTS_TO_LOCALHOST: $ReadOnlySet<string> = 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<string> = [
   // 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<string> = 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

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', () => {