Skip to content

Commit

Permalink
support encoding errors in html stream and escape user input
Browse files Browse the repository at this point in the history
This commit adds another way to get errors to the suspense instance by encoding them as dataset properties of a template element at the head of the boundary. Previously if there was an error before the boundary flushed there was no way to stream the error to the client because there would never be a client render instruction.

Additionally the error is sent in 3 parts

1) error hash - this is always sent (dev or prod) if one is provided
2) error message - Dev only
3) error component stack - Dev only, this now captures the stack at the point of error

Another item addressed in this commit is the escaping of potentially unsafe data. all error components are escaped as test for browers when written into the html and as javascript strings when written into a client render instruction.
  • Loading branch information
gnoff committed May 30, 2022
1 parent 1a10c8c commit e34e1df
Show file tree
Hide file tree
Showing 14 changed files with 705 additions and 134 deletions.
440 changes: 376 additions & 64 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Large diffs are not rendered by default.

Expand Up @@ -193,20 +193,30 @@ describe('ReactDOMFizzServer', () => {

// @gate experimental
it('should be able to complete by aborting even if the promise never resolves', async () => {
const errors = [];
const controller = new AbortController();
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback={<div>Loading</div>}>
<InfiniteSuspend />
</Suspense>
</div>,
{signal: controller.signal},
{
signal: controller.signal,
onError(x) {
errors.push(x.message);
},
},
);

controller.abort();

const result = await readResult(stream);
expect(result).toContain('Loading');

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
]);
});

// @gate experimental
Expand All @@ -223,12 +233,18 @@ describe('ReactDOMFizzServer', () => {
rendered = true;
return 'Done';
}
const errors = [];
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback={<div>Loading</div>}>
<Wait /> />
</Suspense>
</div>,
{
onError(x) {
errors.push(x.message);
},
},
);

stream.allReady.then(() => (isComplete = true));
Expand All @@ -239,6 +255,10 @@ describe('ReactDOMFizzServer', () => {
const reader = stream.getReader();
reader.cancel();

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
]);

hasLoaded = true;
resolve();

Expand Down
30 changes: 28 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js
Expand Up @@ -211,7 +211,7 @@ describe('ReactDOMFizzServer', () => {

{
onError(x) {
reportedErrors.push(x);
reportedErrors.push(x.message);
},
onShellError(x) {
reportedShellErrors.push(x);
Expand All @@ -224,7 +224,10 @@ describe('ReactDOMFizzServer', () => {

expect(output.error).toBe(theError);
expect(output.result).toBe('');
expect(reportedErrors).toEqual([theError]);
expect(reportedErrors).toEqual([
theError.message,
'This Suspense boundary was aborted by the server',
]);
expect(reportedShellErrors).toEqual([theError]);
});

Expand Down Expand Up @@ -289,6 +292,7 @@ describe('ReactDOMFizzServer', () => {
// @gate experimental
it('should be able to complete by aborting even if the promise never resolves', async () => {
let isCompleteCalls = 0;
const errors = [];
const {writable, output, completed} = getTestWritable();
const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -298,6 +302,9 @@ describe('ReactDOMFizzServer', () => {
</div>,

{
onError(x) {
errors.push(x.message);
},
onAllReady() {
isCompleteCalls++;
},
Expand All @@ -314,6 +321,9 @@ describe('ReactDOMFizzServer', () => {

await completed;

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
]);
expect(output.error).toBe(undefined);
expect(output.result).toContain('Loading');
expect(isCompleteCalls).toBe(1);
Expand All @@ -322,6 +332,7 @@ describe('ReactDOMFizzServer', () => {
// @gate experimental
it('should be able to complete by abort when the fallback is also suspended', async () => {
let isCompleteCalls = 0;
const errors = [];
const {writable, output, completed} = getTestWritable();
const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -333,6 +344,9 @@ describe('ReactDOMFizzServer', () => {
</div>,

{
onError(x) {
errors.push(x.message);
},
onAllReady() {
isCompleteCalls++;
},
Expand All @@ -349,6 +363,11 @@ describe('ReactDOMFizzServer', () => {

await completed;

expect(errors).toEqual([
// There are two boundaries that abort
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server',
]);
expect(output.error).toBe(undefined);
expect(output.result).toContain('Loading');
expect(isCompleteCalls).toBe(1);
Expand Down Expand Up @@ -552,6 +571,7 @@ describe('ReactDOMFizzServer', () => {
rendered = true;
return 'Done';
}
const errors = [];
const {writable, completed} = getTestWritable();
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -560,6 +580,9 @@ describe('ReactDOMFizzServer', () => {
</Suspense>
</div>,
{
onError(x) {
errors.push(x.message);
},
onAllReady() {
isComplete = true;
},
Expand All @@ -579,6 +602,9 @@ describe('ReactDOMFizzServer', () => {

await completed;

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
]);
expect(rendered).toBe(false);
expect(isComplete).toBe(true);
});
Expand Down
25 changes: 22 additions & 3 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Expand Up @@ -729,10 +729,29 @@ export function isSuspenseInstancePending(instance: SuspenseInstance) {
export function isSuspenseInstanceFallback(instance: SuspenseInstance) {
return instance.data === SUSPENSE_FALLBACK_START_DATA;
}
export function getSuspenseInstanceFallbackError(
export function getSuspenseInstanceFallbackErrorDetails(
instance: SuspenseInstance,
): string {
return (instance: any).data2;
) {
const nextSibling = instance.nextSibling;
let errorMessage /*, errorComponentStack, errorHash*/;
if (
nextSibling &&
nextSibling.nodeType === ELEMENT_NODE &&
nextSibling.nodeName.toLowerCase() === 'template'
) {
const msg = ((nextSibling: any): HTMLTemplateElement).dataset.msg;
if (msg !== null) errorMessage = msg;

// @TODO read and return hash and componentStack once we know how we are goign to
// expose this extra errorInfo to onRecoverableError

// const hash = ((nextSibling: any): HTMLTemplateElement).dataset.hash;
// if (hash !== null) errorHash = hash;

// const stack = ((nextSibling: any): HTMLTemplateElement).dataset.stack;
// if (stack !== null) errorComponentStack = stack;
}
return {errorMessage /*, errorComponentStack, errorHash*/};
}

export function registerSuspenseInstanceRetry(
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/server/ReactDOMFizzServerNode.js
Expand Up @@ -43,7 +43,7 @@ type Options = {|
onShellReady?: () => void,
onShellError?: (error: mixed) => void,
onAllReady?: () => void,
onError?: (error: mixed) => string,
onError?: (error: mixed) => ?string,
|};

type PipeableStream = {|
Expand Down
119 changes: 107 additions & 12 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Expand Up @@ -1503,6 +1503,19 @@ const startClientRenderedSuspenseBoundary = stringToPrecomputedChunk(
);
const endSuspenseBoundary = stringToPrecomputedChunk('<!--/$-->');

const clientRenderedSuspenseBoundaryError1 = stringToPrecomputedChunk(
'<template data-hash="',
);
const clientRenderedSuspenseBoundaryError1A = stringToPrecomputedChunk(
'" data-msg="',
);
const clientRenderedSuspenseBoundaryError1B = stringToPrecomputedChunk(
'" data-stack="',
);
const clientRenderedSuspenseBoundaryError2 = stringToPrecomputedChunk(
'"></template>',
);

export function pushStartCompletedSuspenseBoundary(
target: Array<Chunk | PrecomputedChunk>,
) {
Expand Down Expand Up @@ -1540,8 +1553,43 @@ export function writeStartPendingSuspenseBoundary(
export function writeStartClientRenderedSuspenseBoundary(
destination: Destination,
responseState: ResponseState,
errorHash: ?string,
errorMesssage: ?string,
errorComponentStack: ?string,
): boolean {
return writeChunkAndReturn(destination, startClientRenderedSuspenseBoundary);
let result;
result = writeChunkAndReturn(
destination,
startClientRenderedSuspenseBoundary,
);
if (errorHash) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1);
writeChunk(destination, stringToChunk(escapeTextForBrowser(errorHash)));
// In prod errorMessage will usually be nullish but there is one case where
// it is used (currently when the server aborts the task) so we leave it ungated.
if (errorMesssage) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1A);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorMesssage)),
);
}
if (__DEV__) {
// Component stacks are currently only captured in dev
if (errorComponentStack) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1B);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorComponentStack)),
);
}
}
result = writeChunkAndReturn(
destination,
clientRenderedSuspenseBoundaryError2,
);
}
return result;
}
export function writeEndCompletedSuspenseBoundary(
destination: Destination,
Expand Down Expand Up @@ -1701,7 +1749,7 @@ export function writeEndSegment(
// const SUSPENSE_PENDING_START_DATA = '$?';
// const SUSPENSE_FALLBACK_START_DATA = '$!';
//
// function clientRenderBoundary(suspenseBoundaryID, errorMsg) {
// function clientRenderBoundary(suspenseBoundaryID, errorHash, errorMsg, errorComponentStack) {
// // Find the fallback's first element.
// const suspenseIdNode = document.getElementById(suspenseBoundaryID);
// if (!suspenseIdNode) {
Expand All @@ -1713,7 +1761,11 @@ export function writeEndSegment(
// const suspenseNode = suspenseIdNode.previousSibling;
// // Tag it to be client rendered.
// suspenseNode.data = SUSPENSE_FALLBACK_START_DATA;
// suspenseNode.data2 = errorMsg;
// // assign error metadata to first sibling
// let dataset = suspenseIdNode.dataset;
// if (errorHash) dataset.hash = errorHash;
// if (errorMsg) dataset.msg = errorMsg;
// if (errorComponentStack) dataset.stack = errorComponentStack;
// // Tell React to retry it if the parent already hydrated.
// if (suspenseNode._reactRetry) {
// suspenseNode._reactRetry();
Expand Down Expand Up @@ -1801,7 +1853,7 @@ const completeSegmentFunction =
const completeBoundaryFunction =
'function $RC(a,b){a=document.getElementById(a);b=document.getElementById(b);b.parentNode.removeChild(b);if(a){a=a.previousSibling;var f=a.parentNode,c=a.nextSibling,e=0;do{if(c&&8===c.nodeType){var d=c.data;if("/$"===d)if(0===e)break;else e--;else"$"!==d&&"$?"!==d&&"$!"!==d||e++}d=c.nextSibling;f.removeChild(c);c=d}while(c);for(;b.firstChild;)f.insertBefore(b.firstChild,c);a.data="$";a._reactRetry&&a._reactRetry()}}';
const clientRenderFunction =
'function $RX(a,b){if(a=document.getElementById(a))a=a.previousSibling,a.data="$!",a.data2=b,a._reactRetry&&a._reactRetry()}';
'function $RX(b,c,d,e){var a=document.getElementById(b);a&&(b=a.previousSibling,b.data="$!",a=a.dataset,c&&(a.hash=c),d&&(a.msg=d),e&&(a.stack=e),b._reactRetry&&b._reactRetry())}';

const completeSegmentScript1Full = stringToPrecomputedChunk(
completeSegmentFunction + ';$RS("',
Expand Down Expand Up @@ -1871,17 +1923,20 @@ export function writeCompletedBoundaryInstruction(
}

const clientRenderScript1Full = stringToPrecomputedChunk(
clientRenderFunction + ";$RX('",
clientRenderFunction + ';$RX("',
);
const clientRenderScript1Partial = stringToPrecomputedChunk("$RX('");
const clientRenderScript2 = stringToPrecomputedChunk("')</script>");
const clientRenderErrorScript1 = stringToPrecomputedChunk("','");
const clientRenderScript1Partial = stringToPrecomputedChunk('$RX("');
const clientRenderScript1A = stringToPrecomputedChunk('"');
const clientRenderScript2 = stringToPrecomputedChunk(')</script>');
const clientRenderErrorScriptArgInterstitial = stringToPrecomputedChunk(',');

export function writeClientRenderBoundaryInstruction(
destination: Destination,
responseState: ResponseState,
boundaryID: SuspenseBoundaryID,
error: ?string,
errorHash: ?string,
errorMessage?: string,
errorComponentStack?: string,
): boolean {
writeChunk(destination, responseState.startInlineScript);
if (!responseState.sentClientRenderFunction) {
Expand All @@ -1900,9 +1955,49 @@ export function writeClientRenderBoundaryInstruction(
}

writeChunk(destination, boundaryID);
if (error) {
writeChunk(destination, clientRenderErrorScript1);
writeChunk(destination, stringToChunk(error));
writeChunk(destination, clientRenderScript1A);
if (errorHash || errorMessage || errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorHash || '')),
);
}
if (errorMessage || errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorMessage || '')),
);
}
if (errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorComponentStack)),
);
}
return writeChunkAndReturn(destination, clientRenderScript2);
}

const regexForJSStringsInScripts = /[<\u2028\u2029]/g;
function escapeJSStringsForInstructionScripts(input: string): string {
const escaped = JSON.stringify(input);
return escaped.replace(regexForJSStringsInScripts, match => {
switch (match) {
// santizing breaking out of strings and script tags
case '<':
return '\\u003c';
case '\u2028':
return '\\u2028';
case '\u2029':
return '\\u2029';
default: {
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
'escapeJSStringsForInstructionScripts encountered a match it does not know how to replace. this means the match regex and the replacement characters are no longer in sync. This is a bug in React',
);
}
}
});
}
Expand Up @@ -127,6 +127,10 @@ export function writeStartCompletedSuspenseBoundary(
export function writeStartClientRenderedSuspenseBoundary(
destination: Destination,
responseState: ResponseState,
// flushing these error arguments are not currently supported in this legacy streaming format.
errorHash: ?string,
errorMessage?: string,
errorComponentStack?: string,
): boolean {
if (responseState.generateStaticMarkup) {
// A client rendered boundary is done and doesn't need a representation in the HTML
Expand Down

0 comments on commit e34e1df

Please sign in to comment.