From b91823e21434ac665450e67ddc6f816710255938 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Thu, 14 May 2026 22:03:52 +0200 Subject: [PATCH] [FlightReply] Don't drop FormData entries in `decodeReplyFromBusboy` (#36468) 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. fixes vercel/next.js#93822 --- 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 | 13 ++ 8 files changed, 618 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 0ae925a1d4ec..b8c230a37132 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 a4bda173f158..0224bb382bfa 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 d267edd085af..d39081aecfef 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 5381072b3ad8..6c8d759a99ac 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 f118d764447f..eac2f9c48317 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 e710eafd00a1..c8809b040460 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 303fed599cdc..ddcbd689a91b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6092,6 +6092,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" @@ -8167,6 +8174,7 @@ eslint-plugin-no-unsanitized@4.0.2: "eslint-plugin-react-internal@link:./scripts/eslint-rules": version "0.0.0" + uid "" eslint-plugin-react@^6.7.1: version "6.10.3" @@ -16092,6 +16100,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"