From 7f9f495de598f60cf3bbd8feba3651e00839acf8 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Tue, 28 Jan 2025 08:04:31 -0800 Subject: [PATCH] don't inline sourceMapURL in Debugger.scriptParsed (#49001) Summary: In {D42973408}, `Debugger.scriptParsed` was tweaked to be intercepted in `inspector-proxy`, which: 1. Rewrote the `sourceMapURL` to be relative to debugger. 2. Attempted to fetch the contents of the source map from `sourceMapURL` after re-writing again to a server-relative URL, and if successful replaced `sourceMapURL` with a base64 data URL. 1 is still needed until we have `Network.loadNetworkResource`, but 2 was only needed for frontends that did not support http fetch, and is not needed with Fusebox. Changelog: [Feature] `Debugger.scriptParsed` now includes the field `sourceMapURL` as a (rewritten) remote url as opposed to base64 data url Reviewed By: robhogan Differential Revision: D68708899 --- .../src/__tests__/InspectorProtocolUtils.js | 9 - .../InspectorProxyCdpRewritingHacks-test.js | 155 +----------------- .../src/inspector-proxy/Device.js | 18 -- 3 files changed, 7 insertions(+), 175 deletions(-) diff --git a/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js b/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js index cddba100863..fb148bd56b2 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js +++ b/packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js @@ -109,15 +109,6 @@ export async function sendFromDebuggerToTarget( return receivedMessage.wrappedEvent; } -export function parseJsonFromDataUri(uri: string): T { - expect(uri).toMatch(/^data:/); - const parsedUri = dataUriToBuffer(uri); - expect(parsedUri.type).toBe('application/json'); - return JSON.parse( - new TextDecoder(parsedUri.charset).decode(parsedUri.buffer), - ); -} - export async function createAndConnectTarget( serverRef: $ReadOnly<{ serverBaseUrl: string, diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js index 09b5ebf2c01..3b1860b7e15 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js @@ -12,7 +12,6 @@ import {withFetchSelfSignedCertsForAllTests} from './FetchUtils'; import { createAndConnectTarget, - parseJsonFromDataUri, sendFromDebuggerToTarget, sendFromTargetToDebugger, } from './InspectorProtocolUtils'; @@ -89,9 +88,9 @@ describe.each(['HTTP', 'HTTPS'])( }, }, ); - expect( - parseJsonFromDataUri(scriptParsedMessage.params.sourceMapURL), - ).toEqual({version: 3, file: '\u2757.js'}); + expect(scriptParsedMessage.params.sourceMapURL).toEqual( + `${serverRef.serverBaseUrl}/source-map`, + ); } finally { device.close(); debugger_.close(); @@ -132,7 +131,7 @@ describe.each(['HTTP', 'HTTPS'])( expect(debugger_.handle).toHaveBeenNthCalledWith(1, { method: 'Debugger.scriptParsed', params: { - sourceMapURL: expect.stringMatching(/^data:/), + sourceMapURL: `${serverRef.serverBaseUrl}/source-map`, }, }); expect(debugger_.handle).toHaveBeenNthCalledWith(2, { @@ -144,57 +143,6 @@ describe.each(['HTTP', 'HTTPS'])( } }); - test('handling of failure to fetch source map', async () => { - const {device, debugger_} = await createAndConnectTarget( - serverRef, - autoCleanup.signal, - { - app: 'bar-app', - id: 'page1', - title: 'bar-title', - vm: 'bar-vm', - }, - ); - try { - const scriptParsedMessage = await sendFromTargetToDebugger( - device, - debugger_, - 'page1', - { - method: 'Debugger.scriptParsed', - params: { - sourceMapURL: `${serverRef.serverBaseUrl}/source-map-missing`, - }, - }, - ); - - // We don't rewrite the message in this case. - expect(scriptParsedMessage.params.sourceMapURL).toEqual( - `${serverRef.serverBaseUrl}/source-map-missing`, - ); - - // We send an error through to the debugger as a console message. - expect(debugger_.handle).toBeCalledWith( - expect.objectContaining({ - method: 'Runtime.consoleAPICalled', - params: { - args: [ - { - type: 'string', - value: expect.stringMatching('Failed to fetch source map'), - }, - ], - executionContextId: 0, - type: 'error', - }, - }), - ); - } finally { - device.close(); - debugger_.close(); - } - }); - 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( @@ -211,11 +159,6 @@ describe.each(['HTTP', 'HTTPS'])( }, ); 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`; @@ -230,7 +173,6 @@ describe.each(['HTTP', 'HTTPS'])( }, }, ); - expect(fetchCalledWithURL?.href).toEqual(sourceMapURL); expect(scriptParsedMessage.params.sourceMapURL).toEqual( `${protocol.toLowerCase()}://127.0.0.1:${serverRef.port}/source-map`, ); @@ -270,9 +212,9 @@ describe.each(['HTTP', 'HTTPS'])( }, }, ); - expect( - parseJsonFromDataUri(scriptParsedMessage.params.sourceMapURL), - ).toEqual({version: 3}); + expect(scriptParsedMessage.params.sourceMapURL).toEqual( + `${serverRef.serverBaseUrl}/source-map`, + ); } finally { device.close(); debugger_.close(); @@ -538,89 +480,6 @@ describe.each(['HTTP', 'HTTPS'])( debugger_.close(); } }); - - test.each(['url', 'file'])( - 'reports %s fetch error back to debugger', - async resourceType => { - const {device, debugger_} = await createAndConnectTarget( - serverRef, - autoCleanup.signal, - { - app: 'bar-app', - id: 'page1', - title: 'bar-title', - vm: 'bar-vm', - }, - ); - try { - await sendFromTargetToDebugger(device, debugger_, 'page1', { - method: 'Debugger.scriptParsed', - params: { - scriptId: 'script1', - url: - resourceType === 'url' - ? `${serverRef.serverBaseUrl}/source-missing` - : '__fixtures__/mock-source-file.does-not-exist', - startLine: 0, - endLine: 0, - startColumn: 0, - endColumn: 0, - hash: createHash('sha256').update('foo').digest('hex'), - }, - }); - const response = await debugger_.sendAndGetResponse({ - id: 1, - method: 'Debugger.getScriptSource', - params: { - scriptId: 'script1', - }, - }); - - // We mark the request as failed. - expect(response).toEqual({ - id: 1, - result: { - error: { - message: expect.stringMatching( - `Failed to fetch source ${resourceType}`, - ), - }, - }, - }); - - // We also send an error through to the debugger as a console message. - expect(debugger_.handle).toBeCalledWith( - expect.objectContaining({ - method: 'Runtime.consoleAPICalled', - params: { - args: [ - { - type: 'string', - value: expect.stringMatching( - `Failed to fetch source ${resourceType}`, - ), - }, - ], - executionContextId: 0, - type: 'error', - }, - }), - ); - - // The device does not receive the getScriptSource request, since it - // is handled by the proxy. - expect(device.wrappedEventParsed).not.toBeCalledWith({ - pageId: 'page1', - wrappedEvent: expect.objectContaining({ - method: 'Debugger.getScriptSource', - }), - }); - } finally { - device.close(); - debugger_.close(); - } - }, - ); }); describe("disabled when target has 'nativeSourceCodeFetching' capability flag", () => { diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index 69bf668c06c..97e785312c8 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -737,24 +737,6 @@ export default class Device { 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 - // an HTTP URL, fetch the content here and convert the content to a - // Data URL (which is more widely supported) before passing the - // message to the debug client. - try { - const sourceMap = await this.#fetchText( - this.#deviceRelativeUrlToServerRelativeUrl(sourceMapURL), - ); - payload.params.sourceMapURL = - 'data:application/json;charset=utf-8;base64,' + - Buffer.from(sourceMap).toString('base64'); - } catch (exception) { - this.#sendErrorToDebugger( - `Failed to fetch source map ${params.sourceMapURL}: ${exception.message}`, - ); - } } } if ('url' in params) {