dev-middleware: Generalise URL rewriting, don't only rewrite for Android emulators (#47880)

Summary:

The previous diffs in this stack have aimed to make URL rewriting by inspector-proxy robust to any configuration of device->server, debugger->server and server->server connections.

Though rewriting was originally introduced to support Android emulator networking, we can now expand it to cover other use cases, like the device reaching the server over an internet address not reachable from the dev machine, or the debugger routing to the server through a tunnel on a different port, without needing CORS workarounds.

Changelog
[General][Fixed] dev-middleware: Rewrite URLs in the inspector proxy to cover all configurations, not just Android emulators.

Reviewed By: huntie

Differential Revision: D66247355

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

@@ -23,10 +23,11 @@ export class DebuggerAgent {
   #ws: ?WebSocket;
   #readyPromise: Promise<void>;
 
-  constructor(url: string, signal?: AbortSignal) {
+  constructor(url: string, signal?: AbortSignal, hostHeader?: ?string) {
     const ws = new WebSocket(url, {
       // The mock server uses a self-signed certificate.
       rejectUnauthorized: false,
+      ...(hostHeader != null ? {headers: {Host: hostHeader}} : {}),
     });
     this.#ws = ws;
     ws.on('message', data => {
@@ -115,8 +116,9 @@ export class DebuggerMock extends DebuggerAgent {
 export async function createDebuggerMock(
   url: string,
   signal: AbortSignal,
+  hostHeader?: ?string,
 ): Promise<DebuggerMock> {
-  const debuggerMock = new DebuggerMock(url, signal);
+  const debuggerMock = new DebuggerMock(url, signal, hostHeader);
   await debuggerMock.ready();
   return debuggerMock;
 }

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

@@ -127,11 +127,13 @@ export async function createAndConnectTarget(
   signal: AbortSignal,
   page: PageFromDevice,
   {
+    debuggerHostHeader = null,
     deviceId = null,
-    host = null,
+    deviceHostHeader = null,
   }: $ReadOnly<{
+    debuggerHostHeader?: ?string,
     deviceId?: ?string,
-    host?: ?string,
+    deviceHostHeader?: ?string,
   }> = {},
 ): Promise<{device: DeviceMock, debugger_: DebuggerMock}> {
   let device;
@@ -142,7 +144,7 @@ export async function createAndConnectTarget(
         deviceId ?? 'device' + Date.now()
       }&name=foo&app=bar`,
       signal,
-      host,
+      deviceHostHeader,
     );
     device.getPages.mockImplementation(() => [page]);
 
@@ -157,7 +159,11 @@ export async function createAndConnectTarget(
     const [{webSocketDebuggerUrl}] = pageList;
     expect(webSocketDebuggerUrl).toBeDefined();
 
-    debugger_ = await createDebuggerMock(webSocketDebuggerUrl, signal);
+    debugger_ = await createDebuggerMock(
+      webSocketDebuggerUrl,
+      signal,
+      debuggerHostHeader,
+    );
     await until(() => expect(device.connect).toBeCalled());
   } catch (e) {
     device?.close();

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

@@ -207,7 +207,7 @@ describe.each(['HTTP', 'HTTPS'])(
           vm: 'bar-vm',
         },
         {
-          host: '192.168.0.123:' + serverRef.port,
+          deviceHostHeader: '192.168.0.123:' + serverRef.port,
         },
       );
       try {
@@ -240,52 +240,7 @@ describe.each(['HTTP', 'HTTPS'])(
       }
     });
 
-    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'])(
+    describe.each(['10.0.2.2:8080', '[::1]', 'example.com:2000'])(
       '%s aliasing to and from localhost',
       sourceHost => {
         test('in source map fetching during Debugger.scriptParsed', async () => {
@@ -300,7 +255,7 @@ describe.each(['HTTP', 'HTTPS'])(
               vm: 'bar-vm',
             },
             {
-              host: sourceHost + ':' + serverRef.port,
+              deviceHostHeader: sourceHost,
             },
           );
           try {
@@ -311,9 +266,7 @@ describe.each(['HTTP', 'HTTPS'])(
               {
                 method: 'Debugger.scriptParsed',
                 params: {
-                  sourceMapURL: `${protocol.toLowerCase()}://${sourceHost}:${
-                    serverRef.port
-                  }/source-map`,
+                  sourceMapURL: `${protocol.toLowerCase()}://${sourceHost}/source-map`,
                 },
               },
             );
@@ -337,7 +290,8 @@ describe.each(['HTTP', 'HTTPS'])(
               vm: 'bar-vm',
             },
             {
-              host: sourceHost + ':' + serverRef.port,
+              debuggerHostHeader: 'localhost:' + serverRef.port,
+              deviceHostHeader: sourceHost,
             },
           );
           try {
@@ -348,9 +302,7 @@ describe.each(['HTTP', 'HTTPS'])(
               {
                 method: 'Debugger.scriptParsed',
                 params: {
-                  url: `${protocol.toLowerCase()}://${sourceHost}:${
-                    serverRef.port
-                  }/some/file.js`,
+                  url: `${protocol.toLowerCase()}://${sourceHost}/some/file.js`,
                 },
               },
             );
@@ -376,9 +328,7 @@ describe.each(['HTTP', 'HTTPS'])(
               },
             );
             expect(setBreakpointByUrlMessage.params.url).toEqual(
-              `${protocol.toLowerCase()}://${sourceHost}:${
-                serverRef.port
-              }/some/file.js`,
+              `${protocol.toLowerCase()}://${sourceHost}/some/file.js`,
             );
 
             const setBreakpointByUrlRegexMessage =
@@ -390,9 +340,20 @@ describe.each(['HTTP', 'HTTPS'])(
                   urlRegex: `localhost:${serverRef.port}|example.com:2000`,
                 },
               });
-            expect(setBreakpointByUrlRegexMessage.params.urlRegex).toEqual(
-              `${sourceHost.replaceAll('.', '\\.')}:${serverRef.port}|example.com:2000`,
-            );
+
+            // urlRegex rewriting is restricted to specific Android IPs that
+            // are well-known to route to the host. In this case we only
+            // replace hostname - longstanding behaviour.
+            if (sourceHost === '10.0.2.2:8080') {
+              expect(setBreakpointByUrlRegexMessage.params.urlRegex).toEqual(
+                `10\\.0\\.2\\.2:${serverRef.port}|example.com:2000`,
+              );
+            } else {
+              // Otherwise expect no change.
+              expect(setBreakpointByUrlRegexMessage.params.urlRegex).toEqual(
+                `localhost:${serverRef.port}|example.com:2000`,
+              );
+            }
           } finally {
             device.close();
             debugger_.close();
@@ -411,7 +372,7 @@ describe.each(['HTTP', 'HTTPS'])(
                 vm: 'bar-vm',
               },
               {
-                host: sourceHost + ':' + serverRef.port,
+                deviceHostHeader: sourceHost,
               },
             );
             try {
@@ -451,7 +412,7 @@ describe.each(['HTTP', 'HTTPS'])(
           vm: 'bar-vm',
         },
         {
-          host: '127.0.0.1:' + serverRef.port,
+          deviceHostHeader: '127.0.0.1:' + serverRef.port,
         },
       );
       try {

packages/dev-middleware/src/inspector-proxy/Device.js

@@ -37,22 +37,6 @@ const debug = require('debug')('Metro:InspectorProxy');
 
 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: $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
-  // fetching). Replacing with the more general 'localhost' should always be
-  // safe while also more compatible with IPv6-only setups.
-  '127.0.0.1',
-  // Android's stock emulator and other emulators such as genymotion use a
-  // 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.
 const FILE_PREFIX = 'file://';
@@ -60,10 +44,6 @@ const FILE_PREFIX = 'file://';
 type DebuggerConnection = {
   // Debugger web socket connection
   socket: WS,
-  // If we replaced a device-relative origin (like 'http://10.0.2.2:8082') with
-  // debugger-relative, we store the original address to reverse the operation
-  // on messages back from the frontend, such as setting breakpoints.
-  originalSourceURLOrigin?: string,
   prependedFilePrefix: boolean,
   pageId: string,
   userAgent: string | null,
@@ -675,6 +655,54 @@ export default class Device {
     }
   }
 
+  /**
+   * Given a URL from the debugger frontend, returns the equivalent URL
+   * reachable from the device.
+   */
+  #debuggerRelativeToDeviceRelativeUrl(
+    debuggerRelativeUrl: URL,
+    {debuggerRelativeBaseUrl}: DebuggerConnection,
+  ): URL {
+    const deviceRelativeUrl = new URL(debuggerRelativeUrl.href);
+    if (debuggerRelativeUrl.origin === debuggerRelativeBaseUrl.origin) {
+      deviceRelativeUrl.hostname = this.#deviceRelativeBaseUrl.hostname;
+      deviceRelativeUrl.port = this.#deviceRelativeBaseUrl.port;
+      deviceRelativeUrl.protocol = this.#deviceRelativeBaseUrl.protocol;
+    }
+    return deviceRelativeUrl;
+  }
+
+  /**
+   * Given a URL from the device, returns the equivalent URL reachable from
+   * the debugger frontend.
+   */
+  #deviceRelativeUrlToDebuggerRelativeUrl(
+    deviceRelativeUrl: URL,
+    {debuggerRelativeBaseUrl}: DebuggerConnection,
+  ): URL {
+    const debuggerRelativeUrl = new URL(deviceRelativeUrl.href);
+    if (deviceRelativeUrl.origin === this.#deviceRelativeBaseUrl.origin) {
+      debuggerRelativeUrl.hostname = debuggerRelativeBaseUrl.hostname;
+      debuggerRelativeUrl.port = debuggerRelativeBaseUrl.port;
+      debuggerRelativeUrl.protocol = debuggerRelativeUrl.protocol;
+    }
+    return debuggerRelativeUrl;
+  }
+
+  /**
+   * Given a URL from the device, returns the equivalent URL reachable from
+   * this proxy.
+   */
+  #deviceRelativeUrlToServerRelativeUrl(deviceRelativeUrl: URL): URL {
+    const debuggerRelativeUrl = new URL(deviceRelativeUrl.href);
+    if (deviceRelativeUrl.origin === this.#deviceRelativeBaseUrl.origin) {
+      debuggerRelativeUrl.hostname = this.#serverRelativeBaseUrl.hostname;
+      debuggerRelativeUrl.port = this.#serverRelativeBaseUrl.port;
+      debuggerRelativeUrl.protocol = this.#serverRelativeBaseUrl.protocol;
+    }
+    return debuggerRelativeUrl;
+  }
+
   // Allows to make changes in incoming message from device.
   async #processMessageFromDeviceLegacy(
     payload: CDPServerMessage,
@@ -695,36 +723,14 @@ 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);
-
           // Rewrite device-relative URLs to de debugger-relative URLs for the
           // frontend.
-          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.host =
-              debuggerInfo.debuggerRelativeBaseUrl.host;
-            debuggerRelativeURL.protocol =
-              debuggerInfo.debuggerRelativeBaseUrl.protocol;
-            serverRelativeUrl.host = this.#serverRelativeBaseUrl.host;
-            serverRelativeUrl.protocol = this.#serverRelativeBaseUrl.protocol;
-            debuggerInfo.originalSourceURLOrigin =
-              this.#deviceRelativeBaseUrl.origin;
-            payload.params.sourceMapURL = debuggerRelativeURL.href;
-          }
+          payload.params.sourceMapURL =
+            this.#deviceRelativeUrlToDebuggerRelativeUrl(
+              sourceMapURL,
+              debuggerInfo,
+            ).href;
 
           // Some debug clients do not support fetching HTTP URLs. If the
           // message headed to the debug client identifies the source map with
@@ -732,7 +738,9 @@ export default class Device {
           // Data URL (which is more widely supported) before passing the
           // message to the debug client.
           try {
-            const sourceMap = await this.#fetchText(serverRelativeUrl);
+            const sourceMap = await this.#fetchText(
+              this.#deviceRelativeUrlToServerRelativeUrl(sourceMapURL),
+            );
             payload.params.sourceMapURL =
               'data:application/json;charset=utf-8;base64,' +
               Buffer.from(sourceMap).toString('base64');
@@ -746,31 +754,17 @@ export default class Device {
       if ('url' in params) {
         let serverRelativeUrl = params.url;
         const parsedUrl = this.#tryParseHTTPURL(params.url);
-        // 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.host = debuggerInfo.debuggerRelativeBaseUrl.host;
-          parsedUrl.protocol = debuggerInfo.debuggerRelativeBaseUrl.protocol;
-          payload.params.url = parsedUrl.href;
-          debuggerInfo.originalSourceURLOrigin =
-            this.#deviceRelativeBaseUrl.origin;
+        if (parsedUrl) {
+          // Rewrite device-relative URLs pointing to the server so that they're
+          // reachable from the frontend.
+          payload.params.url = this.#deviceRelativeUrlToDebuggerRelativeUrl(
+            parsedUrl,
+            debuggerInfo,
+          ).href;
 
           // Determine the server-relative URL.
-          parsedUrl.host = this.#serverRelativeBaseUrl.host;
-          parsedUrl.protocol = this.#serverRelativeBaseUrl.protocol;
-          serverRelativeUrl = parsedUrl.href;
+          serverRelativeUrl =
+            this.#deviceRelativeUrlToServerRelativeUrl(parsedUrl).href;
         }
 
         // Chrome doesn't download source maps if URL param is not a valid
@@ -883,28 +877,23 @@ export default class Device {
     debuggerInfo: DebuggerConnection,
   ): CDPRequest<'Debugger.setBreakpointByUrl'> {
     // If we replaced Android emulator's address to localhost we need to change it back.
-    const {
-      debuggerRelativeBaseUrl,
-      originalSourceURLOrigin,
-      prependedFilePrefix,
-    } = debuggerInfo;
-    const processedReq = {...req, params: {...req.params}};
-    if (originalSourceURLOrigin != null && processedReq.params.url != null) {
-      processedReq.params.url = processedReq.params.url.replace(
-        debuggerRelativeBaseUrl.origin,
-        originalSourceURLOrigin,
-      );
+    const {debuggerRelativeBaseUrl, prependedFilePrefix} = debuggerInfo;
 
-      if (
-        processedReq.params.url &&
-        processedReq.params.url.startsWith(FILE_PREFIX) &&
+    const processedReq = {...req, params: {...req.params}};
+    if (processedReq.params.url != null) {
+      const originalUrlParam = processedReq.params.url;
+      const httpUrl = this.#tryParseHTTPURL(originalUrlParam);
+      if (httpUrl) {
+        processedReq.params.url = this.#debuggerRelativeToDeviceRelativeUrl(
+          httpUrl,
+          debuggerInfo,
+        ).href;
+      } else if (
+        originalUrlParam.startsWith(FILE_PREFIX) &&
         prependedFilePrefix
       ) {
         // Remove fake URL prefix if we modified URL in #processMessageFromDeviceLegacy.
-        // $FlowFixMe[incompatible-use]
-        processedReq.params.url = processedReq.params.url.slice(
-          FILE_PREFIX.length,
-        );
+        processedReq.params.url = originalUrlParam.slice(FILE_PREFIX.length);
       }
     }
 
@@ -919,8 +908,11 @@ export default class Device {
     // `file://` source URLs. It can be removed when we drop support for
     // legacy targets, if not sooner.
     if (
-      REWRITE_HOSTS_TO_LOCALHOST.has(this.#deviceRelativeBaseUrl.hostname) &&
-      this.#deviceRelativeBaseUrl.port === debuggerRelativeBaseUrl.port &&
+      // Android's stock emulator and other emulators such as genymotion use a
+      // standard localhost alias.
+      new Set(['10.0.2.2', '10.0.3.2']).has(
+        this.#deviceRelativeBaseUrl.hostname,
+      ) &&
       debuggerRelativeBaseUrl.hostname === 'localhost' &&
       processedReq.params.urlRegex != null
     ) {