Skip to content

Commit

Permalink
Restore Content-Length header in inspector-proxy JSON responses
Browse files Browse the repository at this point in the history
Summary:
Changelog: [General][Fixed] Re-enable listing Hermes debugger targets in chrome://inspect, broken in 0.74 RC

Fixes #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
  • Loading branch information
motiz88 authored and facebook-github-bot committed Mar 1, 2024
1 parent 64b0c9c commit 4cfac8e
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<JsonPagesListResponse>(
`${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();
}
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 4cfac8e

Please sign in to comment.