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

robhogan · facebook-github-bot · commit 74995bc90aa0 · 2024-11-22T10:26:43.000-08:00
Summary: Pull Request resolved: #47880 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 fbshipit-source-id: e9201ebc1f7f5fe2119c71cd4d7b4ca895645404
diff --git a/packages/dev-middleware/src/__tests__/InspectorDebuggerUtils.js b/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;
 }
diff --git a/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js b/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();
diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js b/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 {
diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js
