Skip to content

Commit 79e4f23

Browse files
authored
[Fizz] correctly track header length (#30327)
The interstital characters in our link header tracking are not contributing to the remaining capacity calculation so when a lot of inditidual links are present in the link header it can allow an overflowing link header to be included. This change corrects the math so it properly prevents overflow.
1 parent c5eca9b commit 79e4f23

File tree

2 files changed

+65
-8
lines changed

2 files changed

+65
-8
lines changed

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -431,9 +431,13 @@ export function createRenderState(
431431
fontPreloads: '',
432432
highImagePreloads: '',
433433
remainingCapacity:
434-
typeof maxHeadersLength === 'number'
434+
// We seed the remainingCapacity with 2 extra bytes because when we decrement the capacity
435+
// we always assume we are inserting an interstitial ", " however the first header does not actually
436+
// consume these two extra bytes.
437+
2 +
438+
(typeof maxHeadersLength === 'number'
435439
? maxHeadersLength
436-
: DEFAULT_HEADERS_CAPACITY_IN_UTF16_CODE_UNITS,
440+
: DEFAULT_HEADERS_CAPACITY_IN_UTF16_CODE_UNITS),
437441
}
438442
: null;
439443
const renderState: RenderState = {
@@ -2953,7 +2957,7 @@ function pushImg(
29532957
// make this behavior different between render and prerender since in the latter case
29542958
// we are less sensitive to the current requests runtime per and more sensitive to maximizing
29552959
// headers.
2956-
(headers.remainingCapacity -= header.length) >= 2)
2960+
(headers.remainingCapacity -= header.length + 2) >= 0)
29572961
) {
29582962
// If we postpone in the shell we will still emit this preload so we track
29592963
// it to make sure we don't reset it.
@@ -5393,7 +5397,7 @@ function prefetchDNS(href: string) {
53935397
// make this behavior different between render and prerender since in the latter case
53945398
// we are less sensitive to the current requests runtime per and more sensitive to maximizing
53955399
// headers.
5396-
(headers.remainingCapacity -= header.length) >= 2)
5400+
(headers.remainingCapacity -= header.length + 2) >= 0)
53975401
) {
53985402
// Store this as resettable in case we are prerendering and postpone in the Shell
53995403
renderState.resets.dns[key] = EXISTS;
@@ -5452,7 +5456,7 @@ function preconnect(href: string, crossOrigin: ?CrossOriginEnum) {
54525456
// make this behavior different between render and prerender since in the latter case
54535457
// we are less sensitive to the current requests runtime per and more sensitive to maximizing
54545458
// headers.
5455-
(headers.remainingCapacity -= header.length) >= 2)
5459+
(headers.remainingCapacity -= header.length + 2) >= 0)
54565460
) {
54575461
// Store this in resettableState in case we are prerending and postpone in the Shell
54585462
renderState.resets.connect[bucket][key] = EXISTS;
@@ -5518,7 +5522,7 @@ function preload(href: string, as: string, options?: ?PreloadImplOptions) {
55185522
// make this behavior different between render and prerender since in the latter case
55195523
// we are less sensitive to the current requests runtime per and more sensitive to maximizing
55205524
// headers.
5521-
(headers.remainingCapacity -= header.length) >= 2)
5525+
(headers.remainingCapacity -= header.length + 2) >= 0)
55225526
) {
55235527
// If we postpone in the shell we will still emit a preload as a header so we
55245528
// track this to make sure we don't reset it.
@@ -5633,7 +5637,7 @@ function preload(href: string, as: string, options?: ?PreloadImplOptions) {
56335637
// make this behavior different between render and prerender since in the latter case
56345638
// we are less sensitive to the current requests runtime per and more sensitive to maximizing
56355639
// headers.
5636-
(headers.remainingCapacity -= header.length) >= 2)
5640+
(headers.remainingCapacity -= header.length + 2) >= 0)
56375641
) {
56385642
// If we postpone in the shell we will still emit this preload so we
56395643
// track it here to prevent it from being reset.
@@ -6260,7 +6264,7 @@ export function emitEarlyPreloads(
62606264
// This means that a particularly long header might close out the header queue where later
62616265
// headers could still fit. We could in the future alter the behavior here based on prerender vs render
62626266
// since during prerender we aren't as concerned with pure runtime performance.
6263-
if ((headers.remainingCapacity -= header.length) >= 2) {
6267+
if ((headers.remainingCapacity -= header.length + 2) >= 0) {
62646268
renderState.resets.style[key] = PRELOAD_NO_CREDS;
62656269
if (linkHeader) {
62666270
linkHeader += ', ';

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3940,6 +3940,59 @@ describe('ReactDOMFizzServer', () => {
39403940
expect(getVisibleChildren(container)).toEqual(<div>hello</div>);
39413941
});
39423942

3943+
it('accounts for the length of the interstitial between links when computing the headers length', async () => {
3944+
let headers = null;
3945+
function onHeaders(x) {
3946+
headers = x;
3947+
}
3948+
3949+
function App() {
3950+
// 20 bytes
3951+
ReactDOM.preconnect('01');
3952+
// 42 bytes
3953+
ReactDOM.preconnect('02');
3954+
// 64 bytes
3955+
ReactDOM.preconnect('03');
3956+
// 86 bytes
3957+
ReactDOM.preconnect('04');
3958+
// 108 bytes
3959+
ReactDOM.preconnect('05');
3960+
// 130 bytes
3961+
ReactDOM.preconnect('06');
3962+
// 152 bytes
3963+
ReactDOM.preconnect('07');
3964+
// 174 bytes
3965+
ReactDOM.preconnect('08');
3966+
// 196 bytes
3967+
ReactDOM.preconnect('09');
3968+
// 218 bytes
3969+
ReactDOM.preconnect('10');
3970+
// 240 bytes
3971+
ReactDOM.preconnect('11');
3972+
// 262 bytes
3973+
ReactDOM.preconnect('12');
3974+
// 284 bytes
3975+
ReactDOM.preconnect('13');
3976+
// 306 bytes
3977+
ReactDOM.preconnect('14');
3978+
return (
3979+
<html>
3980+
<body>hello</body>
3981+
</html>
3982+
);
3983+
}
3984+
3985+
await act(() => {
3986+
renderToPipeableStream(<App />, {onHeaders, maxHeadersLength: 305});
3987+
});
3988+
expect(headers.Link.length).toBe(284);
3989+
3990+
await act(() => {
3991+
renderToPipeableStream(<App />, {onHeaders, maxHeadersLength: 306});
3992+
});
3993+
expect(headers.Link.length).toBe(306);
3994+
});
3995+
39433996
describe('error escaping', () => {
39443997
it('escapes error hash, message, and component stack values in directly flushed errors (html escaping)', async () => {
39453998
window.__outlet = {};

0 commit comments

Comments
 (0)