@samitouri / QOS-React-2 / commits / a1723e18fd

[Flight] Only skip past the end boundary if there is a newline character (#26945)

Follow up to #26932 For regular rows, we're increasing the index by one to skip past the last trailing newline character which acts a boundary. For length encoded rows we shouldn't skip an extra byte because it'll leave us missing one. This only accidentally worked because this was also the end of the current chunk which tests don't account for since we're just passing through the chunks. So I added some noise by splitting and joining the chunks so that this gets tested.

Sebastian Markbåge committed Jun 14, 2023 at 00:51 UTC a1723e18fd42e67d610adb0107c9015319613bd7
2 files changed +45 -7
packages/react-client/src/ReactFlightClient.js
+9 -5
@@ -950,29 +950,33 @@ export function processBinaryChunk(
950 }
951 case ROW_CHUNK_BY_LENGTH: {
952 // We're looking for the remaining byte length
953 - if (i + rowLength <= chunk.length) {
954 - lastIdx = i + rowLength;
953 + lastIdx = i + rowLength;
954 + if (lastIdx > chunk.length) {
955 + lastIdx = -1;
956 }
957 break;
958 }
959 }
960 + const offset = chunk.byteOffset + i;
961 if (lastIdx > -1) {
962 // We found the last chunk of the row
961 - const offset = chunk.byteOffset + i;
963 const length = lastIdx - i;
964 const lastChunk = new Uint8Array(chunk.buffer, offset, length);
965 processFullRow(response, rowID, rowTag, buffer, lastChunk);
966 // Reset state machine for a new row
967 + i = lastIdx;
968 + if (rowState === ROW_CHUNK_BY_NEWLINE) {
969 + // If we're trailing by a newline we need to skip it.
970 + i++;
971 + }
972 rowState = ROW_ID;
973 rowTag = 0;
974 rowID = 0;
975 rowLength = 0;
976 buffer.length = 0;
971 - i = lastIdx + 1;
977 } else {
978 // The rest of this row is in a future chunk. We stash the rest of the
979 // current chunk until we can process the full row.
975 - const offset = chunk.byteOffset + i;
980 const length = chunk.byteLength - i;
981 const remainingSlice = new Uint8Array(chunk.buffer, offset, length);
982 buffer.push(remainingSlice);
packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js
+36 -2
@@ -42,6 +42,37 @@ describe('ReactFlightDOMEdge', () => {
42 use = React.use;
43 });
44
45 + function passThrough(stream) {
46 + // Simulate more realistic network by splitting up and rejoining some chunks.
47 + // This lets us test that we don't accidentally rely on particular bounds of the chunks.
48 + return new ReadableStream({
49 + async start(controller) {
50 + const reader = stream.getReader();
51 + let prevChunk = new Uint8Array(0);
52 + function push() {
53 + reader.read().then(({done, value}) => {
54 + if (done) {
55 + controller.enqueue(prevChunk);
56 + controller.close();
57 + return;
58 + }
59 + const chunk = new Uint8Array(prevChunk.length + value.length);
60 + chunk.set(prevChunk, 0);
61 + chunk.set(value, prevChunk.length);
62 + if (chunk.length > 50) {
63 + controller.enqueue(chunk.subarray(0, chunk.length - 50));
64 + prevChunk = chunk.subarray(chunk.length - 50);
65 + } else {
66 + prevChunk = chunk;
67 + }
68 + push();
69 + });
70 + }
71 + push();
72 + },
73 + });
74 + }
75 +
76 async function readResult(stream) {
77 const reader = stream.getReader();
78 let result = '';
@@ -101,15 +132,17 @@ describe('ReactFlightDOMEdge', () => {
132
133 it('should encode long string in a compact format', async () => {
134 const testString = '"\n\t'.repeat(500) + '🙃';
135 + const testString2 = 'hello'.repeat(400);
136
137 const stream = ReactServerDOMServer.renderToReadableStream({
138 text: testString,
139 + text2: testString2,
140 });
108 - const [stream1, stream2] = stream.tee();
141 + const [stream1, stream2] = passThrough(stream).tee();
142
143 const serializedContent = await readResult(stream1);
144 // The content should be compact an unescaped
112 - expect(serializedContent.length).toBeLessThan(2000);
145 + expect(serializedContent.length).toBeLessThan(4000);
146 expect(serializedContent).not.toContain('\\n');
147 expect(serializedContent).not.toContain('\\t');
148 expect(serializedContent).not.toContain('\\"');
@@ -118,5 +151,6 @@ describe('ReactFlightDOMEdge', () => {
151 const result = await ReactServerDOMClient.createFromReadableStream(stream2);
152 // Should still match the result when parsed
153 expect(result.text).toBe(testString);
154 + expect(result.text2).toBe(testString2);
155 });
156 });