Repository: facebook/react
Pull request: [Fizz] correctly track header length

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.

Changed files:
filename: packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
patch: @@ -431,9 +431,13 @@ export function createRenderState(
         fontPreloads: '',
         highImagePreloads: '',
         remainingCapacity:
-          typeof maxHeadersLength === 'number'
+          // We seed the remainingCapacity with 2 extra bytes because when we decrement the capacity
+          // we always assume we are inserting an interstitial ", " however the first header does not actually
+          // consume these two extra bytes.
+          2 +
+          (typeof maxHeadersLength === 'number'
             ? maxHeadersLength
-            : DEFAULT_HEADERS_CAPACITY_IN_UTF16_CODE_UNITS,
+            : DEFAULT_HEADERS_CAPACITY_IN_UTF16_CODE_UNITS),
       }
     : null;
   const renderState: RenderState = {
@@ -2953,7 +2957,7 @@ function pushImg(
         // make this behavior different between render and prerender since in the latter case
         // we are less sensitive to the current requests runtime per and more sensitive to maximizing
         // headers.
-        (headers.remainingCapacity -= header.length) >= 2)
+        (headers.remainingCapacity -= header.length + 2) >= 0)
       ) {
         // If we postpone in the shell we will still emit this preload so we track
         // it to make sure we don't reset it.
@@ -5393,7 +5397,7 @@ function prefetchDNS(href: string) {
         // make this behavior different between render and prerender since in the latter case
         // we are less sensitive to the current requests runtime per and more sensitive to maximizing
         // headers.
-        (headers.remainingCapacity -= header.length) >= 2)
+        (headers.remainingCapacity -= header.length + 2) >= 0)
       ) {
         // Store this as resettable in case we are prerendering and postpone in the Shell
         renderState.resets.dns[key] = EXISTS;
@@ -5452,7 +5456,7 @@ function preconnect(href: string, crossOrigin: ?CrossOriginEnum) {
         // make this behavior different between render and prerender since in the latter case
         // we are less sensitive to the current requests runtime per and more sensitive to maximizing
         // headers.
-        (headers.remainingCapacity -= header.length) >= 2)
+        (headers.remainingCapacity -= header.length + 2) >= 0)
       ) {
         // Store this in resettableState in case we are prerending and postpone in the Shell
         renderState.resets.connect[bucket][key] = EXISTS;
@@ -5518,7 +5522,7 @@ function preload(href: string, as: string, options?: ?PreloadImplOptions) {
           // make this behavior different between render and prerender since in the latter case
           // we are less sensitive to the current requests runtime per and more sensitive to maximizing
           // headers.
-          (headers.remainingCapacity -= header.length) >= 2)
+          (headers.remainingCapacity -= header.length + 2) >= 0)
         ) {
           // If we postpone in the shell we will still emit a preload as a header so we
           // track this to make sure we don't reset it.
@@ -5633,7 +5637,7 @@ function preload(href: string, as: string, options?: ?PreloadImplOptions) {
           // make this behavior different between render and prerender since in the latter case
           // we are less sensitive to the current requests runtime per and more sensitive to maximizing
           // headers.
-          (headers.remainingCapacity -= header.length) >= 2)
+          (headers.remainingCapacity -= header.length + 2) >= 0)
         ) {
           // If we postpone in the shell we will still emit this preload so we
           // track it here to prevent it from being reset.
@@ -6260,7 +6264,7 @@ export function emitEarlyPreloads(
             // This means that a particularly long header might close out the header queue where later
             // headers could still fit. We could in the future alter the behavior here based on prerender vs render
             // since during prerender we aren't as concerned with pure runtime performance.
-            if ((headers.remainingCapacity -= header.length) >= 2) {
+            if ((headers.remainingCapacity -= header.length + 2) >= 0) {
               renderState.resets.style[key] = PRELOAD_NO_CREDS;
               if (linkHeader) {
                 linkHeader += ', ';

filename: packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
patch: @@ -3940,6 +3940,59 @@ describe('ReactDOMFizzServer', () => {
     expect(getVisibleChildren(container)).toEqual(<div>hello</div>);
   });
 
+  it('accounts for the length of the interstitial between links when computing the headers length', async () => {
+    let headers = null;
+    function onHeaders(x) {
+      headers = x;
+    }
+
+    function App() {
+      // 20 bytes
+      ReactDOM.preconnect('01');
+      // 42 bytes
+      ReactDOM.preconnect('02');
+      // 64 bytes
+      ReactDOM.preconnect('03');
+      // 86 bytes
+      ReactDOM.preconnect('04');
+      // 108 bytes
+      ReactDOM.preconnect('05');
+      // 130 bytes
+      ReactDOM.preconnect('06');
+      // 152 bytes
+      ReactDOM.preconnect('07');
+      // 174 bytes
+      ReactDOM.preconnect('08');
+      // 196 bytes
+      ReactDOM.preconnect('09');
+      // 218 bytes
+      ReactDOM.preconnect('10');
+      // 240 bytes
+      ReactDOM.preconnect('11');
+      // 262 bytes
+      ReactDOM.preconnect('12');
+      // 284 bytes
+      ReactDOM.preconnect('13');
+      // 306 bytes
+      ReactDOM.preconnect('14');
+      return (
+        <html>
+          <body>hello</body>
+        </html>
+      );
+    }
+
+    await act(() => {
+      renderToPipeableStream(<App />, {onHeaders, maxHeadersLength: 305});
+    });
+    expect(headers.Link.length).toBe(284);
+
+    await act(() => {
+      renderToPipeableStream(<App />, {onHeaders, maxHeadersLength: 306});
+    });
+    expect(headers.Link.length).toBe(306);
+  });
+
   describe('error escaping', () => {
     it('escapes error hash, message, and component stack values in directly flushed errors (html escaping)', async () => {
       window.__outlet = {};