From 4cfac8eea63129059559f0a65c038dfe95e12d7c Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Fri, 1 Mar 2024 02:38:48 -0800 Subject: [PATCH] Restore Content-Length header in inspector-proxy JSON responses Summary: Changelog: [General][Fixed] Re-enable listing Hermes debugger targets in chrome://inspect, broken in 0.74 RC Fixes https://github.com/facebook/react-native/issues/43259. Reverts D52958725 and fixes the original `Content-Length` Unicode bug using a different approach. Reviewed By: fabriziocucci Differential Revision: D54409847 fbshipit-source-id: ed5bb464ab67f37535947646b124814d8bbf797c --- .../__tests__/InspectorProxyHttpApi-test.js | 66 ++++++++++++++++++- .../src/inspector-proxy/InspectorProxy.js | 1 + 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js index e601f219f06324..e5501b8e3ceb66 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js @@ -14,11 +14,13 @@ import type { JsonVersionResponse, } from '../inspector-proxy/types'; -import {fetchJson} from './FetchUtils'; +import {fetchJson, fetchLocal} from './FetchUtils'; import {createDeviceMock} from './InspectorDeviceUtils'; import {withAbortSignalForEachTest} from './ResourceUtils'; import {withServerForEachTest} from './ServerUtils'; +import nullthrows from 'nullthrows'; + // Must be greater than or equal to PAGES_POLLING_INTERVAL in `InspectorProxy.js`. const PAGES_POLLING_DELAY = 1000; @@ -309,5 +311,67 @@ describe('inspector proxy HTTP API', () => { } }); }); + + test('handles Unicode data safely', async () => { + const device = await createDeviceMock( + `${serverRef.serverBaseWsUrl}/inspector/device?device=device1&name=foo&app=bar`, + autoCleanup.signal, + ); + try { + device.getPages.mockImplementation(() => [ + { + app: 'bar-app 📱', + id: 'page1 🛂', + title: 'bar-title 📰', + vm: 'bar-vm 🤖', + }, + ]); + + jest.advanceTimersByTime(PAGES_POLLING_DELAY); + + const json = await fetchJson( + `${serverRef.serverBaseUrl}${endpoint}`, + ); + expect(json).toEqual([ + expect.objectContaining({ + description: 'bar-app 📱', + deviceName: 'foo', + id: 'device1-page1 🛂', + title: 'bar-title 📰', + vm: 'bar-vm 🤖', + }), + ]); + } finally { + device.close(); + } + }); + + test('includes a valid Content-Length header', async () => { + // NOTE: This test is needed because chrome://inspect's HTTP client is picky + // and doesn't accept responses without a Content-Length header. + const device = await createDeviceMock( + `${serverRef.serverBaseWsUrl}/inspector/device?device=device1&name=foo&app=bar`, + autoCleanup.signal, + ); + try { + device.getPages.mockImplementation(() => [ + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + }, + ]); + + jest.advanceTimersByTime(PAGES_POLLING_DELAY); + + const response = await fetchLocal( + `${serverRef.serverBaseUrl}${endpoint}`, + ); + expect(response.headers.get('Content-Length')).not.toBeNull(); + } finally { + device.close(); + } + }); }); }); diff --git a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js index 066fa44cc3d536..0f4146d5e01884 100644 --- a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js +++ b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js @@ -167,6 +167,7 @@ export default class InspectorProxy implements InspectorProxyQueries { response.writeHead(200, { 'Content-Type': 'application/json; charset=UTF-8', 'Cache-Control': 'no-cache', + 'Content-Length': Buffer.byteLength(data).toString(), Connection: 'close', }); response.end(data);