From 6a9e2ad5db3d15a5f018ad2c8dbc6d3b1189ad26 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Thu, 14 May 2026 22:03:52 +0200 Subject: [PATCH] [19.2.x][FlightReply] Don't drop FormData entries in `decodeReplyFromBusboy` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a regression from #36425 where referenced `FormData` entries can be dropped by `decodeReplyFromBusboy` when files are interleaved with text fields in the payload. `decodeReplyFromBusboy` queues text fields that arrive while a file is being streamed and flushes them after the last file's `'end'`, working around busboy emitting `'end'` deferred relative to subsequent `'field'` events. With multiple files interleaved with text, this loses the relative order of the affected text entries. The reorder was a long-standing but invisible issue — entries came back in the wrong order but were all present — until #36425 tightened how referenced FormData entries are collected from the backing store to rely on them being contiguous. With that assumption violated, referenced FormDatas can now come back with some entries dropped. The pattern is most easily surfaced through `useActionState` actions that return the submitted `FormData` as part of their state. This replaces the tail-flush with a linked list of pending files. Text fields that arrive while a file is in flight are queued on the tail file's `queuedFields`; fields that arrive when the list is empty resolve immediately. `flush()` walks from the head, resolving each completed file followed by its queued fields, and stops at the first file that hasn't ended yet. The backing FormData now matches the payload's order, restoring the contiguity assumption (and fixing the long-standing reorder as a side effect). The same change is applied to all five copies in `react-server-dom-{webpack,turbopack,parcel,esm,unbundled}`. Two new tests cover the multi-file interleave. --- package.json | 1 + .../src/server/ReactFlightDOMServerNode.js | 112 ++++++++++--- .../src/server/ReactFlightDOMServerNode.js | 112 ++++++++++--- .../src/server/ReactFlightDOMServerNode.js | 112 ++++++++++--- .../src/server/ReactFlightDOMServerNode.js | 112 ++++++++++--- .../__tests__/ReactFlightDOMReplyNode-test.js | 149 ++++++++++++++++++ .../src/server/ReactFlightDOMServerNode.js | 112 ++++++++++--- yarn.lock | 12 ++ 8 files changed, 617 insertions(+), 105 deletions(-) create mode 100644 packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReplyNode-test.js diff --git a/package.json b/package.json index 1a52907f21ed..42002a998d43 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "art": "0.10.1", "babel-plugin-syntax-hermes-parser": "^0.32.0", "babel-plugin-syntax-trailing-function-commas": "^6.5.0", + "busboy": "^1.6.0", "chalk": "^3.0.0", "cli-table": "^0.3.1", "coffee-script": "^1.12.7", diff --git a/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js index 4faa867420d2..63bfa7827122 100644 --- a/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js @@ -62,6 +62,7 @@ import { } from 'react-client/src/ReactFlightClientStreamConfigNode'; import type {TemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; +import type {FileHandle} from 'react-server/src/ReactFlightReplyServer'; export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; @@ -329,6 +330,17 @@ function prerenderToNodeStream( }); } +type PendingFile = { + name: string, + file: FileHandle, + complete: boolean, + // Lazily allocated when a text field arrives after this file's 'file' + // event but before its (deferred) 'end' event. Stored as flat + // [name1, value1, name2, value2, ...] pairs. + queuedFields: null | Array, + next: null | PendingFile, +}; + function decodeReplyFromBusboy( busboyStream: Busboy, moduleBasePath: ServerManifest, @@ -344,14 +356,55 @@ function decodeReplyFromBusboy( undefined, options ? options.arraySizeLimit : undefined, ); - let pendingFiles = 0; - const queuedFields: Array = []; + + // Linked list of pending files in arrival (payload) order. Text fields that + // arrive while a file is in flight are queued on the tail file's + // `queuedFields` so they can be resolved together when that file completes. + // Fields that arrive while the list is empty bypass it and resolve + // immediately. This makes the backing FormData's insertion order match the + // payload's entry order. + let head: null | PendingFile = null; + let tail: null | PendingFile = null; + let bodyFinished = false; + let closed = false; + + function flush() { + while (head !== null) { + const current = head; + if (!current.complete) { + // This file is still streaming. Hold later files and fields until it + // completes so the backing FormData reflects payload order. + return; + } + try { + resolveFileComplete(response, current.name, current.file); + const queuedFields = current.queuedFields; + if (queuedFields !== null) { + for (let i = 0; i < queuedFields.length; i += 2) { + resolveField(response, queuedFields[i], queuedFields[i + 1]); + } + } + } catch (error) { + busboyStream.destroy(error); + return; + } + head = current.next; + } + tail = null; + if (bodyFinished && !closed) { + closed = true; + close(response); + } + } + busboyStream.on('field', (name, value) => { - if (pendingFiles > 0) { - // Because the 'end' event fires two microtasks after the next 'field' - // we would resolve files and fields out of order. To handle this properly - // we queue any fields we receive until the previous file is done. - queuedFields.push(name, value); + if (tail !== null) { + // A file is in flight; queue the field on the tail (most recent) pending + // file so it resolves after that file, preserving payload order. + if (tail.queuedFields === null) { + tail.queuedFields = []; + } + tail.queuedFields.push(name, value); } else { try { resolveField(response, name, value); @@ -371,29 +424,46 @@ function decodeReplyFromBusboy( ); return; } - pendingFiles++; const file = resolveFileInfo(response, name, filename, mimeType); + const pendingFile: PendingFile = { + name, + file, + complete: false, + queuedFields: null, + next: null, + }; + if (tail === null) { + head = pendingFile; + } else { + tail.next = pendingFile; + } + tail = pendingFile; value.on('data', chunk => { - resolveFileChunk(response, file, chunk); - }); - value.on('end', () => { try { - resolveFileComplete(response, name, file); - pendingFiles--; - if (pendingFiles === 0) { - // Release any queued fields - for (let i = 0; i < queuedFields.length; i += 2) { - resolveField(response, queuedFields[i], queuedFields[i + 1]); - } - queuedFields.length = 0; - } + resolveFileChunk(response, file, chunk); } catch (error) { busboyStream.destroy(error); } }); + value.on('error', error => { + busboyStream.destroy(error); + }); + value.on('end', () => { + pendingFile.complete = true; + flush(); + }); }); busboyStream.on('finish', () => { - close(response); + bodyFinished = true; + flush(); + if (!closed) { + // Invariant: busboy delays 'finish' until every file's 'end' event has + // fired, so the flush above should always close the response. + reportGlobalError( + response, + new Error('Reply finished with incomplete file part.'), + ); + } }); busboyStream.on('error', err => { reportGlobalError( diff --git a/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js index 8dbdfa7f1dac..f87ee6268c95 100644 --- a/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js @@ -75,6 +75,7 @@ import { import {textEncoder} from 'react-server/src/ReactServerStreamConfigNode'; import type {TemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; +import type {FileHandle} from 'react-server/src/ReactFlightReplyServer'; export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; @@ -560,6 +561,17 @@ export function registerServerActions(manifest: ServerManifest) { serverManifest = manifest; } +type PendingFile = { + name: string, + file: FileHandle, + complete: boolean, + // Lazily allocated when a text field arrives after this file's 'file' + // event but before its (deferred) 'end' event. Stored as flat + // [name1, value1, name2, value2, ...] pairs. + queuedFields: null | Array, + next: null | PendingFile, +}; + export function decodeReplyFromBusboy( busboyStream: Busboy, options?: { @@ -574,14 +586,55 @@ export function decodeReplyFromBusboy( undefined, options ? options.arraySizeLimit : undefined, ); - let pendingFiles = 0; - const queuedFields: Array = []; + + // Linked list of pending files in arrival (payload) order. Text fields that + // arrive while a file is in flight are queued on the tail file's + // `queuedFields` so they can be resolved together when that file completes. + // Fields that arrive while the list is empty bypass it and resolve + // immediately. This makes the backing FormData's insertion order match the + // payload's entry order. + let head: null | PendingFile = null; + let tail: null | PendingFile = null; + let bodyFinished = false; + let closed = false; + + function flush() { + while (head !== null) { + const current = head; + if (!current.complete) { + // This file is still streaming. Hold later files and fields until it + // completes so the backing FormData reflects payload order. + return; + } + try { + resolveFileComplete(response, current.name, current.file); + const queuedFields = current.queuedFields; + if (queuedFields !== null) { + for (let i = 0; i < queuedFields.length; i += 2) { + resolveField(response, queuedFields[i], queuedFields[i + 1]); + } + } + } catch (error) { + busboyStream.destroy(error); + return; + } + head = current.next; + } + tail = null; + if (bodyFinished && !closed) { + closed = true; + close(response); + } + } + busboyStream.on('field', (name, value) => { - if (pendingFiles > 0) { - // Because the 'end' event fires two microtasks after the next 'field' - // we would resolve files and fields out of order. To handle this properly - // we queue any fields we receive until the previous file is done. - queuedFields.push(name, value); + if (tail !== null) { + // A file is in flight; queue the field on the tail (most recent) pending + // file so it resolves after that file, preserving payload order. + if (tail.queuedFields === null) { + tail.queuedFields = []; + } + tail.queuedFields.push(name, value); } else { try { resolveField(response, name, value); @@ -601,29 +654,46 @@ export function decodeReplyFromBusboy( ); return; } - pendingFiles++; const file = resolveFileInfo(response, name, filename, mimeType); + const pendingFile: PendingFile = { + name, + file, + complete: false, + queuedFields: null, + next: null, + }; + if (tail === null) { + head = pendingFile; + } else { + tail.next = pendingFile; + } + tail = pendingFile; value.on('data', chunk => { - resolveFileChunk(response, file, chunk); - }); - value.on('end', () => { try { - resolveFileComplete(response, name, file); - pendingFiles--; - if (pendingFiles === 0) { - // Release any queued fields - for (let i = 0; i < queuedFields.length; i += 2) { - resolveField(response, queuedFields[i], queuedFields[i + 1]); - } - queuedFields.length = 0; - } + resolveFileChunk(response, file, chunk); } catch (error) { busboyStream.destroy(error); } }); + value.on('error', error => { + busboyStream.destroy(error); + }); + value.on('end', () => { + pendingFile.complete = true; + flush(); + }); }); busboyStream.on('finish', () => { - close(response); + bodyFinished = true; + flush(); + if (!closed) { + // Invariant: busboy delays 'finish' until every file's 'end' event has + // fired, so the flush above should always close the response. + reportGlobalError( + response, + new Error('Reply finished with incomplete file part.'), + ); + } }); busboyStream.on('error', err => { reportGlobalError( diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js index 614fe040137d..5de2b05fdb56 100644 --- a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js @@ -68,6 +68,7 @@ import { import {textEncoder} from 'react-server/src/ReactServerStreamConfigNode'; import type {TemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; +import type {FileHandle} from 'react-server/src/ReactFlightReplyServer'; export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; @@ -551,6 +552,17 @@ function prerender( }); } +type PendingFile = { + name: string, + file: FileHandle, + complete: boolean, + // Lazily allocated when a text field arrives after this file's 'file' + // event but before its (deferred) 'end' event. Stored as flat + // [name1, value1, name2, value2, ...] pairs. + queuedFields: null | Array, + next: null | PendingFile, +}; + function decodeReplyFromBusboy( busboyStream: Busboy, turbopackMap: ServerManifest, @@ -566,14 +578,55 @@ function decodeReplyFromBusboy( undefined, options ? options.arraySizeLimit : undefined, ); - let pendingFiles = 0; - const queuedFields: Array = []; + + // Linked list of pending files in arrival (payload) order. Text fields that + // arrive while a file is in flight are queued on the tail file's + // `queuedFields` so they can be resolved together when that file completes. + // Fields that arrive while the list is empty bypass it and resolve + // immediately. This makes the backing FormData's insertion order match the + // payload's entry order. + let head: null | PendingFile = null; + let tail: null | PendingFile = null; + let bodyFinished = false; + let closed = false; + + function flush() { + while (head !== null) { + const current = head; + if (!current.complete) { + // This file is still streaming. Hold later files and fields until it + // completes so the backing FormData reflects payload order. + return; + } + try { + resolveFileComplete(response, current.name, current.file); + const queuedFields = current.queuedFields; + if (queuedFields !== null) { + for (let i = 0; i < queuedFields.length; i += 2) { + resolveField(response, queuedFields[i], queuedFields[i + 1]); + } + } + } catch (error) { + busboyStream.destroy(error); + return; + } + head = current.next; + } + tail = null; + if (bodyFinished && !closed) { + closed = true; + close(response); + } + } + busboyStream.on('field', (name, value) => { - if (pendingFiles > 0) { - // Because the 'end' event fires two microtasks after the next 'field' - // we would resolve files and fields out of order. To handle this properly - // we queue any fields we receive until the previous file is done. - queuedFields.push(name, value); + if (tail !== null) { + // A file is in flight; queue the field on the tail (most recent) pending + // file so it resolves after that file, preserving payload order. + if (tail.queuedFields === null) { + tail.queuedFields = []; + } + tail.queuedFields.push(name, value); } else { try { resolveField(response, name, value); @@ -593,29 +646,46 @@ function decodeReplyFromBusboy( ); return; } - pendingFiles++; const file = resolveFileInfo(response, name, filename, mimeType); + const pendingFile: PendingFile = { + name, + file, + complete: false, + queuedFields: null, + next: null, + }; + if (tail === null) { + head = pendingFile; + } else { + tail.next = pendingFile; + } + tail = pendingFile; value.on('data', chunk => { - resolveFileChunk(response, file, chunk); - }); - value.on('end', () => { try { - resolveFileComplete(response, name, file); - pendingFiles--; - if (pendingFiles === 0) { - // Release any queued fields - for (let i = 0; i < queuedFields.length; i += 2) { - resolveField(response, queuedFields[i], queuedFields[i + 1]); - } - queuedFields.length = 0; - } + resolveFileChunk(response, file, chunk); } catch (error) { busboyStream.destroy(error); } }); + value.on('error', error => { + busboyStream.destroy(error); + }); + value.on('end', () => { + pendingFile.complete = true; + flush(); + }); }); busboyStream.on('finish', () => { - close(response); + bodyFinished = true; + flush(); + if (!closed) { + // Invariant: busboy delays 'finish' until every file's 'end' event has + // fired, so the flush above should always close the response. + reportGlobalError( + response, + new Error('Reply finished with incomplete file part.'), + ); + } }); busboyStream.on('error', err => { reportGlobalError( diff --git a/packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js index 8d4715a34199..9b99764dd7b5 100644 --- a/packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js @@ -68,6 +68,7 @@ import { import {textEncoder} from 'react-server/src/ReactServerStreamConfigNode'; import type {TemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; +import type {FileHandle} from 'react-server/src/ReactFlightReplyServer'; export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; @@ -551,6 +552,17 @@ function prerender( }); } +type PendingFile = { + name: string, + file: FileHandle, + complete: boolean, + // Lazily allocated when a text field arrives after this file's 'file' + // event but before its (deferred) 'end' event. Stored as flat + // [name1, value1, name2, value2, ...] pairs. + queuedFields: null | Array, + next: null | PendingFile, +}; + function decodeReplyFromBusboy( busboyStream: Busboy, webpackMap: ServerManifest, @@ -566,14 +578,55 @@ function decodeReplyFromBusboy( undefined, options ? options.arraySizeLimit : undefined, ); - let pendingFiles = 0; - const queuedFields: Array = []; + + // Linked list of pending files in arrival (payload) order. Text fields that + // arrive while a file is in flight are queued on the tail file's + // `queuedFields` so they can be resolved together when that file completes. + // Fields that arrive while the list is empty bypass it and resolve + // immediately. This makes the backing FormData's insertion order match the + // payload's entry order. + let head: null | PendingFile = null; + let tail: null | PendingFile = null; + let bodyFinished = false; + let closed = false; + + function flush() { + while (head !== null) { + const current = head; + if (!current.complete) { + // This file is still streaming. Hold later files and fields until it + // completes so the backing FormData reflects payload order. + return; + } + try { + resolveFileComplete(response, current.name, current.file); + const queuedFields = current.queuedFields; + if (queuedFields !== null) { + for (let i = 0; i < queuedFields.length; i += 2) { + resolveField(response, queuedFields[i], queuedFields[i + 1]); + } + } + } catch (error) { + busboyStream.destroy(error); + return; + } + head = current.next; + } + tail = null; + if (bodyFinished && !closed) { + closed = true; + close(response); + } + } + busboyStream.on('field', (name, value) => { - if (pendingFiles > 0) { - // Because the 'end' event fires two microtasks after the next 'field' - // we would resolve files and fields out of order. To handle this properly - // we queue any fields we receive until the previous file is done. - queuedFields.push(name, value); + if (tail !== null) { + // A file is in flight; queue the field on the tail (most recent) pending + // file so it resolves after that file, preserving payload order. + if (tail.queuedFields === null) { + tail.queuedFields = []; + } + tail.queuedFields.push(name, value); } else { try { resolveField(response, name, value); @@ -593,29 +646,46 @@ function decodeReplyFromBusboy( ); return; } - pendingFiles++; const file = resolveFileInfo(response, name, filename, mimeType); + const pendingFile: PendingFile = { + name, + file, + complete: false, + queuedFields: null, + next: null, + }; + if (tail === null) { + head = pendingFile; + } else { + tail.next = pendingFile; + } + tail = pendingFile; value.on('data', chunk => { - resolveFileChunk(response, file, chunk); - }); - value.on('end', () => { try { - resolveFileComplete(response, name, file); - pendingFiles--; - if (pendingFiles === 0) { - // Release any queued fields - for (let i = 0; i < queuedFields.length; i += 2) { - resolveField(response, queuedFields[i], queuedFields[i + 1]); - } - queuedFields.length = 0; - } + resolveFileChunk(response, file, chunk); } catch (error) { busboyStream.destroy(error); } }); + value.on('error', error => { + busboyStream.destroy(error); + }); + value.on('end', () => { + pendingFile.complete = true; + flush(); + }); }); busboyStream.on('finish', () => { - close(response); + bodyFinished = true; + flush(); + if (!closed) { + // Invariant: busboy delays 'finish' until every file's 'end' event has + // fired, so the flush above should always close the response. + reportGlobalError( + response, + new Error('Reply finished with incomplete file part.'), + ); + } }); busboyStream.on('error', err => { reportGlobalError( diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReplyNode-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReplyNode-test.js new file mode 100644 index 000000000000..0924bafbdcd0 --- /dev/null +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReplyNode-test.js @@ -0,0 +1,149 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + * @jest-environment node + */ + +'use strict'; + +let webpackServerMap; +let busboy; +let ReactServerDOMServer; +let ReactServerDOMClient; + +describe('ReactFlightDOMReplyNode', () => { + beforeEach(() => { + jest.resetModules(); + // Simulate the condition resolution + jest.mock('react', () => require('react/react.react-server')); + jest.mock('react-server-dom-webpack/server', () => + require('react-server-dom-webpack/server.node'), + ); + const WebpackMock = require('./utils/WebpackMock'); + webpackServerMap = WebpackMock.webpackServerMap; + ReactServerDOMServer = require('react-server-dom-webpack/server.node'); + jest.resetModules(); + ReactServerDOMClient = require('react-server-dom-webpack/client.node'); + + busboy = require('busboy'); + }); + + // Writes the body to busboy as a multipart stream. Blob entries become + // `filename`-bearing parts so busboy emits them as 'file' events (with + // streamed data) rather than 'field' events. + async function pipeBodyToBusboy(bb, body, boundary) { + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const [name, value] of body) { + if (typeof value === 'string') { + bb.write( + `--${boundary}\r\n` + + `Content-Disposition: form-data; name="${name}"\r\n` + + `\r\n` + + `${value}\r\n`, + ); + } else { + const filename = + typeof value.name === 'string' && value.name !== '' + ? value.name + : 'blob'; + const mimeType = + typeof value.type === 'string' && value.type !== '' + ? value.type + : 'application/octet-stream'; + const buffer = Buffer.from(await value.arrayBuffer()); + bb.write( + `--${boundary}\r\n` + + `Content-Disposition: form-data; name="${name}"; filename="${filename}"\r\n` + + `Content-Type: ${mimeType}\r\n` + + `\r\n`, + ); + bb.write(buffer); + bb.write('\r\n'); + } + } + bb.end(`--${boundary}--\r\n`); + } + + // FormData iterates entries in insertion order per spec, so a referenced + // FormData must round-trip with its entry order intact even when files + // and text fields are interleaved in the payload. + it('preserves entry order when referenced FormDatas interleave files and text', async () => { + const a = new FormData(); + a.append('text_a', 'value_a'); + a.append('file_a', new Blob(['content_a'], {type: 'text/plain'}), 'a.txt'); + const b = new FormData(); + b.append('text_b', 'value_b'); + b.append('file_b', new Blob(['content_b'], {type: 'text/plain'}), 'b.txt'); + + const body = await ReactServerDOMClient.encodeReply([a, b]); + const boundary = 'boundary'; + const bb = busboy({ + headers: { + 'content-type': `multipart/form-data; boundary=${boundary}`, + }, + }); + const reply = ReactServerDOMServer.decodeReplyFromBusboy( + bb, + webpackServerMap, + ); + await pipeBodyToBusboy(bb, body, boundary); + + const result = await reply; + expect(result).toHaveLength(2); + const [decodedA, decodedB] = result; + + const aEntries = Array.from(decodedA.entries()); + expect(aEntries.map(([k]) => k)).toEqual(['text_a', 'file_a']); + expect(aEntries[0][1]).toBe('value_a'); + expect(aEntries[1][1]).toBeInstanceOf(File); + expect(aEntries[1][1].name).toBe('a.txt'); + + const bEntries = Array.from(decodedB.entries()); + expect(bEntries.map(([k]) => k)).toEqual(['text_b', 'file_b']); + expect(bEntries[0][1]).toBe('value_b'); + expect(bEntries[1][1]).toBeInstanceOf(File); + expect(bEntries[1][1].name).toBe('b.txt'); + }); + + // Every entry of a referenced FormData must be present in the decoded + // FormData regardless of where files appear in its iteration order. + it('does not drop entries when referenced FormDatas iterate files before text', async () => { + const a = new FormData(); + a.append('file_a', new Blob(['content_a'], {type: 'text/plain'}), 'a.txt'); + a.append('text_a', 'value_a'); + const b = new FormData(); + b.append('file_b', new Blob(['content_b'], {type: 'text/plain'}), 'b.txt'); + b.append('text_b', 'value_b'); + + const body = await ReactServerDOMClient.encodeReply([a, b]); + const boundary = 'boundary'; + const bb = busboy({ + headers: { + 'content-type': `multipart/form-data; boundary=${boundary}`, + }, + }); + const reply = ReactServerDOMServer.decodeReplyFromBusboy( + bb, + webpackServerMap, + ); + await pipeBodyToBusboy(bb, body, boundary); + + const result = await reply; + expect(result).toHaveLength(2); + const [decodedA, decodedB] = result; + + const aKeys = Array.from(decodedA.keys()).sort(); + expect(aKeys).toEqual(['file_a', 'text_a']); + expect(decodedA.get('text_a')).toBe('value_a'); + expect(decodedA.get('file_a')).toBeInstanceOf(File); + + const bKeys = Array.from(decodedB.keys()).sort(); + expect(bKeys).toEqual(['file_b', 'text_b']); + expect(decodedB.get('text_b')).toBe('value_b'); + expect(decodedB.get('file_b')).toBeInstanceOf(File); + }); +}); diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js index 08a311fc1527..cc2d0009bff6 100644 --- a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js @@ -68,6 +68,7 @@ import { import {textEncoder} from 'react-server/src/ReactServerStreamConfigNode'; import type {TemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; +import type {FileHandle} from 'react-server/src/ReactFlightReplyServer'; export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTemporaryReferences'; @@ -551,6 +552,17 @@ function prerender( }); } +type PendingFile = { + name: string, + file: FileHandle, + complete: boolean, + // Lazily allocated when a text field arrives after this file's 'file' + // event but before its (deferred) 'end' event. Stored as flat + // [name1, value1, name2, value2, ...] pairs. + queuedFields: null | Array, + next: null | PendingFile, +}; + function decodeReplyFromBusboy( busboyStream: Busboy, webpackMap: ServerManifest, @@ -566,14 +578,55 @@ function decodeReplyFromBusboy( undefined, options ? options.arraySizeLimit : undefined, ); - let pendingFiles = 0; - const queuedFields: Array = []; + + // Linked list of pending files in arrival (payload) order. Text fields that + // arrive while a file is in flight are queued on the tail file's + // `queuedFields` so they can be resolved together when that file completes. + // Fields that arrive while the list is empty bypass it and resolve + // immediately. This makes the backing FormData's insertion order match the + // payload's entry order. + let head: null | PendingFile = null; + let tail: null | PendingFile = null; + let bodyFinished = false; + let closed = false; + + function flush() { + while (head !== null) { + const current = head; + if (!current.complete) { + // This file is still streaming. Hold later files and fields until it + // completes so the backing FormData reflects payload order. + return; + } + try { + resolveFileComplete(response, current.name, current.file); + const queuedFields = current.queuedFields; + if (queuedFields !== null) { + for (let i = 0; i < queuedFields.length; i += 2) { + resolveField(response, queuedFields[i], queuedFields[i + 1]); + } + } + } catch (error) { + busboyStream.destroy(error); + return; + } + head = current.next; + } + tail = null; + if (bodyFinished && !closed) { + closed = true; + close(response); + } + } + busboyStream.on('field', (name, value) => { - if (pendingFiles > 0) { - // Because the 'end' event fires two microtasks after the next 'field' - // we would resolve files and fields out of order. To handle this properly - // we queue any fields we receive until the previous file is done. - queuedFields.push(name, value); + if (tail !== null) { + // A file is in flight; queue the field on the tail (most recent) pending + // file so it resolves after that file, preserving payload order. + if (tail.queuedFields === null) { + tail.queuedFields = []; + } + tail.queuedFields.push(name, value); } else { try { resolveField(response, name, value); @@ -593,29 +646,46 @@ function decodeReplyFromBusboy( ); return; } - pendingFiles++; const file = resolveFileInfo(response, name, filename, mimeType); + const pendingFile: PendingFile = { + name, + file, + complete: false, + queuedFields: null, + next: null, + }; + if (tail === null) { + head = pendingFile; + } else { + tail.next = pendingFile; + } + tail = pendingFile; value.on('data', chunk => { - resolveFileChunk(response, file, chunk); - }); - value.on('end', () => { try { - resolveFileComplete(response, name, file); - pendingFiles--; - if (pendingFiles === 0) { - // Release any queued fields - for (let i = 0; i < queuedFields.length; i += 2) { - resolveField(response, queuedFields[i], queuedFields[i + 1]); - } - queuedFields.length = 0; - } + resolveFileChunk(response, file, chunk); } catch (error) { busboyStream.destroy(error); } }); + value.on('error', error => { + busboyStream.destroy(error); + }); + value.on('end', () => { + pendingFile.complete = true; + flush(); + }); }); busboyStream.on('finish', () => { - close(response); + bodyFinished = true; + flush(); + if (!closed) { + // Invariant: busboy delays 'finish' until every file's 'end' event has + // fired, so the flush above should always close the response. + reportGlobalError( + response, + new Error('Reply finished with incomplete file part.'), + ); + } }); busboyStream.on('error', err => { reportGlobalError( diff --git a/yarn.lock b/yarn.lock index d2f2e8d017c6..33c24235f7fc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6093,6 +6093,13 @@ bunyan@1.8.15: mv "~2" safe-json-stringify "~1" +busboy@^1.6.0: + version "1.6.0" + resolved "https://registry.yarnpkg.com/busboy/-/busboy-1.6.0.tgz#966ea36a9502e43cdb9146962523b92f531f6893" + integrity sha512-8SFQbg/0hQ9xy3UNTB0YEnsNBbWfhf7RtnzpL7TkBiTBRfrQ9Fxcnz7VJsleJpyp6rVLvXiuORqjlHi5q+PYuA== + dependencies: + streamsearch "^1.1.0" + bytes@3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/bytes/-/bytes-3.0.0.tgz#d32815404d689699f85a4ea4fa8755dd13a96048" @@ -16196,6 +16203,11 @@ stream-shift@^1.0.0: resolved "https://registry.yarnpkg.com/stream-shift/-/stream-shift-1.0.1.tgz#d7088281559ab2778424279b0877da3c392d5a3d" integrity sha512-AiisoFqQ0vbGcZgQPY1cdP2I76glaVA/RauYR4G4thNFgkTqr90yXTo4LYX60Jl+sIlPNHHdGSwo01AvbKUSVQ== +streamsearch@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/streamsearch/-/streamsearch-1.1.0.tgz#404dd1e2247ca94af554e841a8ef0eaa238da764" + integrity sha512-Mcc5wHehp9aXz1ax6bZUyY5afg9u2rv5cqQI3mRrYkGC8rW2hM02jWuwjtL++LS5qinSyhj2QfLyNsuc+VsExg== + strict-uri-encode@^1.0.0: version "1.1.0" resolved "https://registry.yarnpkg.com/strict-uri-encode/-/strict-uri-encode-1.1.0.tgz#279b225df1d582b1f54e65addd4352e18faa0713"