From 8fb86b0ce0d055c4ee1b1cd8274a921483b3c9c5 Mon Sep 17 00:00:00 2001 From: gnoff Date: Mon, 3 Apr 2023 16:39:57 +0000 Subject: [PATCH] Fix logic around attribute seralization (#26526) There was a bug in the attribute seralization for stylesheet resources injected by the Fizz runtime. For boolean properties the attribute value was set to an empty string but later immediately set to a string coerced value. This PR fixes that bug and refactors the code paths to be clearer DiffTrain build for [4a1cc2ddd035f5c269e82ab6f7686e2e60d3b3ea](https://github.com/facebook/react/commit/4a1cc2ddd035f5c269e82ab6f7686e2e60d3b3ea) --- compiled/facebook-www/REVISION | 2 +- .../ReactDOMServer-dev.classic.js | 92 ++++++++++++------- .../facebook-www/ReactDOMServer-dev.modern.js | 92 ++++++++++++------- .../ReactDOMServer-prod.classic.js | 62 +++++++------ .../ReactDOMServer-prod.modern.js | 62 +++++++------ .../ReactDOMServerStreaming-dev.modern.js | 90 +++++++++++------- .../ReactDOMServerStreaming-prod.modern.js | 60 ++++++------ 7 files changed, 275 insertions(+), 185 deletions(-) diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION index 90896084774bb..1f0e06fdd4d8a 100644 --- a/compiled/facebook-www/REVISION +++ b/compiled/facebook-www/REVISION @@ -1 +1 @@ -7329ea81c154d40800e30104be40f050e8c2af3e +4a1cc2ddd035f5c269e82ab6f7686e2e60d3b3ea diff --git a/compiled/facebook-www/ReactDOMServer-dev.classic.js b/compiled/facebook-www/ReactDOMServer-dev.classic.js index 6faebb23d4564..b20a788c19f69 100644 --- a/compiled/facebook-www/ReactDOMServer-dev.classic.js +++ b/compiled/facebook-www/ReactDOMServer-dev.classic.js @@ -19,7 +19,7 @@ if (__DEV__) { var React = require("react"); var ReactDOM = require("react-dom"); -var ReactVersion = "18.3.0-www-classic-81d2f28c"; +var ReactVersion = "18.3.0-www-classic-83a1ba88"; // This refers to a WWW module. var warningWWW = require("warning"); @@ -6311,52 +6311,63 @@ function writeStyleResourceAttributeInJS(destination, name, value) { return; // Attribute renames - case "className": + case "className": { attributeName = "class"; + + { + checkAttributeStringCoercion(value, attributeName); + } + + attributeValue = "" + value; break; + } // Booleans - case "hidden": + case "hidden": { if (value === false) { return; } attributeValue = ""; break; + } // Santized URLs case "src": case "href": { + value = sanitizeURL(value); + { checkAttributeStringCoercion(value, attributeName); } - value = sanitizeURL(value); + attributeValue = "" + value; break; } default: { + if ( + // unrecognized event handlers are not SSR'd and we (apparently) + // use on* as hueristic for these handler props + name.length > 2 && + (name[0] === "o" || name[0] === "O") && + (name[1] === "n" || name[1] === "N") + ) { + return; + } + if (!isAttributeNameSafe(name)) { return; } - } - } - if ( - // shouldIgnoreAttribute - // We have already filtered out null/undefined and reserved words. - name.length > 2 && - (name[0] === "o" || name[0] === "O") && - (name[1] === "n" || name[1] === "N") - ) { - return; - } + { + checkAttributeStringCoercion(value, attributeName); + } - { - checkAttributeStringCoercion(value, attributeName); + attributeValue = "" + value; + } } - attributeValue = "" + value; writeChunk(destination, arrayInterstitial); writeChunk( destination, @@ -6502,52 +6513,63 @@ function writeStyleResourceAttributeInAttr(destination, name, value) { return; // Attribute renames - case "className": + case "className": { attributeName = "class"; + + { + checkAttributeStringCoercion(value, attributeName); + } + + attributeValue = "" + value; break; + } // Booleans - case "hidden": + case "hidden": { if (value === false) { return; } attributeValue = ""; break; + } // Santized URLs case "src": case "href": { + value = sanitizeURL(value); + { checkAttributeStringCoercion(value, attributeName); } - value = sanitizeURL(value); + attributeValue = "" + value; break; } default: { + if ( + // unrecognized event handlers are not SSR'd and we (apparently) + // use on* as hueristic for these handler props + name.length > 2 && + (name[0] === "o" || name[0] === "O") && + (name[1] === "n" || name[1] === "N") + ) { + return; + } + if (!isAttributeNameSafe(name)) { return; } - } - } - if ( - // shouldIgnoreAttribute - // We have already filtered out null/undefined and reserved words. - name.length > 2 && - (name[0] === "o" || name[0] === "O") && - (name[1] === "n" || name[1] === "N") - ) { - return; - } + { + checkAttributeStringCoercion(value, attributeName); + } - { - checkAttributeStringCoercion(value, attributeName); + attributeValue = "" + value; + } } - attributeValue = "" + value; writeChunk(destination, arrayInterstitial); writeChunk( destination, diff --git a/compiled/facebook-www/ReactDOMServer-dev.modern.js b/compiled/facebook-www/ReactDOMServer-dev.modern.js index c17cb6b64d60d..81461edb9cf09 100644 --- a/compiled/facebook-www/ReactDOMServer-dev.modern.js +++ b/compiled/facebook-www/ReactDOMServer-dev.modern.js @@ -19,7 +19,7 @@ if (__DEV__) { var React = require("react"); var ReactDOM = require("react-dom"); -var ReactVersion = "18.3.0-www-modern-4e6bdd84"; +var ReactVersion = "18.3.0-www-modern-06decfab"; // This refers to a WWW module. var warningWWW = require("warning"); @@ -6311,52 +6311,63 @@ function writeStyleResourceAttributeInJS(destination, name, value) { return; // Attribute renames - case "className": + case "className": { attributeName = "class"; + + { + checkAttributeStringCoercion(value, attributeName); + } + + attributeValue = "" + value; break; + } // Booleans - case "hidden": + case "hidden": { if (value === false) { return; } attributeValue = ""; break; + } // Santized URLs case "src": case "href": { + value = sanitizeURL(value); + { checkAttributeStringCoercion(value, attributeName); } - value = sanitizeURL(value); + attributeValue = "" + value; break; } default: { + if ( + // unrecognized event handlers are not SSR'd and we (apparently) + // use on* as hueristic for these handler props + name.length > 2 && + (name[0] === "o" || name[0] === "O") && + (name[1] === "n" || name[1] === "N") + ) { + return; + } + if (!isAttributeNameSafe(name)) { return; } - } - } - if ( - // shouldIgnoreAttribute - // We have already filtered out null/undefined and reserved words. - name.length > 2 && - (name[0] === "o" || name[0] === "O") && - (name[1] === "n" || name[1] === "N") - ) { - return; - } + { + checkAttributeStringCoercion(value, attributeName); + } - { - checkAttributeStringCoercion(value, attributeName); + attributeValue = "" + value; + } } - attributeValue = "" + value; writeChunk(destination, arrayInterstitial); writeChunk( destination, @@ -6502,52 +6513,63 @@ function writeStyleResourceAttributeInAttr(destination, name, value) { return; // Attribute renames - case "className": + case "className": { attributeName = "class"; + + { + checkAttributeStringCoercion(value, attributeName); + } + + attributeValue = "" + value; break; + } // Booleans - case "hidden": + case "hidden": { if (value === false) { return; } attributeValue = ""; break; + } // Santized URLs case "src": case "href": { + value = sanitizeURL(value); + { checkAttributeStringCoercion(value, attributeName); } - value = sanitizeURL(value); + attributeValue = "" + value; break; } default: { + if ( + // unrecognized event handlers are not SSR'd and we (apparently) + // use on* as hueristic for these handler props + name.length > 2 && + (name[0] === "o" || name[0] === "O") && + (name[1] === "n" || name[1] === "N") + ) { + return; + } + if (!isAttributeNameSafe(name)) { return; } - } - } - if ( - // shouldIgnoreAttribute - // We have already filtered out null/undefined and reserved words. - name.length > 2 && - (name[0] === "o" || name[0] === "O") && - (name[1] === "n" || name[1] === "N") - ) { - return; - } + { + checkAttributeStringCoercion(value, attributeName); + } - { - checkAttributeStringCoercion(value, attributeName); + attributeValue = "" + value; + } } - attributeValue = "" + value; writeChunk(destination, arrayInterstitial); writeChunk( destination, diff --git a/compiled/facebook-www/ReactDOMServer-prod.classic.js b/compiled/facebook-www/ReactDOMServer-prod.classic.js index a508a0f270de6..df917d17c3816 100644 --- a/compiled/facebook-www/ReactDOMServer-prod.classic.js +++ b/compiled/facebook-www/ReactDOMServer-prod.classic.js @@ -1647,29 +1647,33 @@ function writeStyleResourceAttributeInJS(destination, name, value) { return; case "className": attributeName = "class"; + name = "" + value; break; case "hidden": if (!1 === value) return; + name = ""; break; case "src": case "href": value = sanitizeURL(value); + name = "" + value; break; default: - if (!isAttributeNameSafe(name)) return; + if ( + (2 < name.length && + ("o" === name[0] || "O" === name[0]) && + ("n" === name[1] || "N" === name[1])) || + !isAttributeNameSafe(name) + ) + return; + name = "" + value; } - if ( - !(2 < name.length) || - ("o" !== name[0] && "O" !== name[0]) || - ("n" !== name[1] && "N" !== name[1]) - ) - (name = "" + value), - destination.push(","), - (attributeName = escapeJSObjectForInstructionScripts(attributeName)), - destination.push(attributeName), - destination.push(","), - (attributeName = escapeJSObjectForInstructionScripts(name)), - destination.push(attributeName); + destination.push(","); + attributeName = escapeJSObjectForInstructionScripts(attributeName); + destination.push(attributeName); + destination.push(","); + attributeName = escapeJSObjectForInstructionScripts(name); + destination.push(attributeName); } function writeStyleResourceDependenciesInAttr(destination, boundaryResources) { destination.push("["); @@ -1739,29 +1743,33 @@ function writeStyleResourceAttributeInAttr(destination, name, value) { return; case "className": attributeName = "class"; + name = "" + value; break; case "hidden": if (!1 === value) return; + name = ""; break; case "src": case "href": value = sanitizeURL(value); + name = "" + value; break; default: - if (!isAttributeNameSafe(name)) return; + if ( + (2 < name.length && + ("o" === name[0] || "O" === name[0]) && + ("n" === name[1] || "N" === name[1])) || + !isAttributeNameSafe(name) + ) + return; + name = "" + value; } - if ( - !(2 < name.length) || - ("o" !== name[0] && "O" !== name[0]) || - ("n" !== name[1] && "N" !== name[1]) - ) - (name = "" + value), - destination.push(","), - (attributeName = escapeTextForBrowser(JSON.stringify(attributeName))), - destination.push(attributeName), - destination.push(","), - (attributeName = escapeTextForBrowser(JSON.stringify(name))), - destination.push(attributeName); + destination.push(","); + attributeName = escapeTextForBrowser(JSON.stringify(attributeName)); + destination.push(attributeName); + destination.push(","); + attributeName = escapeTextForBrowser(JSON.stringify(name)); + destination.push(attributeName); } function prefetchDNS(href) { if (currentResources) { @@ -3788,4 +3796,4 @@ exports.renderToString = function (children, options) { 'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server' ); }; -exports.version = "18.3.0-www-classic-2919f4b0"; +exports.version = "18.3.0-www-classic-57e47ad8"; diff --git a/compiled/facebook-www/ReactDOMServer-prod.modern.js b/compiled/facebook-www/ReactDOMServer-prod.modern.js index cd1fcfec241a8..1d636ed7d4b18 100644 --- a/compiled/facebook-www/ReactDOMServer-prod.modern.js +++ b/compiled/facebook-www/ReactDOMServer-prod.modern.js @@ -1645,29 +1645,33 @@ function writeStyleResourceAttributeInJS(destination, name, value) { return; case "className": attributeName = "class"; + name = "" + value; break; case "hidden": if (!1 === value) return; + name = ""; break; case "src": case "href": value = sanitizeURL(value); + name = "" + value; break; default: - if (!isAttributeNameSafe(name)) return; + if ( + (2 < name.length && + ("o" === name[0] || "O" === name[0]) && + ("n" === name[1] || "N" === name[1])) || + !isAttributeNameSafe(name) + ) + return; + name = "" + value; } - if ( - !(2 < name.length) || - ("o" !== name[0] && "O" !== name[0]) || - ("n" !== name[1] && "N" !== name[1]) - ) - (name = "" + value), - destination.push(","), - (attributeName = escapeJSObjectForInstructionScripts(attributeName)), - destination.push(attributeName), - destination.push(","), - (attributeName = escapeJSObjectForInstructionScripts(name)), - destination.push(attributeName); + destination.push(","); + attributeName = escapeJSObjectForInstructionScripts(attributeName); + destination.push(attributeName); + destination.push(","); + attributeName = escapeJSObjectForInstructionScripts(name); + destination.push(attributeName); } function writeStyleResourceDependenciesInAttr(destination, boundaryResources) { destination.push("["); @@ -1737,29 +1741,33 @@ function writeStyleResourceAttributeInAttr(destination, name, value) { return; case "className": attributeName = "class"; + name = "" + value; break; case "hidden": if (!1 === value) return; + name = ""; break; case "src": case "href": value = sanitizeURL(value); + name = "" + value; break; default: - if (!isAttributeNameSafe(name)) return; + if ( + (2 < name.length && + ("o" === name[0] || "O" === name[0]) && + ("n" === name[1] || "N" === name[1])) || + !isAttributeNameSafe(name) + ) + return; + name = "" + value; } - if ( - !(2 < name.length) || - ("o" !== name[0] && "O" !== name[0]) || - ("n" !== name[1] && "N" !== name[1]) - ) - (name = "" + value), - destination.push(","), - (attributeName = escapeTextForBrowser(JSON.stringify(attributeName))), - destination.push(attributeName), - destination.push(","), - (attributeName = escapeTextForBrowser(JSON.stringify(name))), - destination.push(attributeName); + destination.push(","); + attributeName = escapeTextForBrowser(JSON.stringify(attributeName)); + destination.push(attributeName); + destination.push(","); + attributeName = escapeTextForBrowser(JSON.stringify(name)); + destination.push(attributeName); } function prefetchDNS(href) { if (currentResources) { @@ -3685,4 +3693,4 @@ exports.renderToString = function (children, options) { 'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server' ); }; -exports.version = "18.3.0-www-modern-70645e15"; +exports.version = "18.3.0-www-modern-4d7a15fb"; diff --git a/compiled/facebook-www/ReactDOMServerStreaming-dev.modern.js b/compiled/facebook-www/ReactDOMServerStreaming-dev.modern.js index 551189f46f37c..e1859ce13792d 100644 --- a/compiled/facebook-www/ReactDOMServerStreaming-dev.modern.js +++ b/compiled/facebook-www/ReactDOMServerStreaming-dev.modern.js @@ -6318,52 +6318,63 @@ function writeStyleResourceAttributeInJS(destination, name, value) { return; // Attribute renames - case "className": + case "className": { attributeName = "class"; + + { + checkAttributeStringCoercion(value, attributeName); + } + + attributeValue = "" + value; break; + } // Booleans - case "hidden": + case "hidden": { if (value === false) { return; } attributeValue = ""; break; + } // Santized URLs case "src": case "href": { + value = sanitizeURL(value); + { checkAttributeStringCoercion(value, attributeName); } - value = sanitizeURL(value); + attributeValue = "" + value; break; } default: { + if ( + // unrecognized event handlers are not SSR'd and we (apparently) + // use on* as hueristic for these handler props + name.length > 2 && + (name[0] === "o" || name[0] === "O") && + (name[1] === "n" || name[1] === "N") + ) { + return; + } + if (!isAttributeNameSafe(name)) { return; } - } - } - if ( - // shouldIgnoreAttribute - // We have already filtered out null/undefined and reserved words. - name.length > 2 && - (name[0] === "o" || name[0] === "O") && - (name[1] === "n" || name[1] === "N") - ) { - return; - } + { + checkAttributeStringCoercion(value, attributeName); + } - { - checkAttributeStringCoercion(value, attributeName); + attributeValue = "" + value; + } } - attributeValue = "" + value; writeChunk(destination, arrayInterstitial); writeChunk( destination, @@ -6509,52 +6520,63 @@ function writeStyleResourceAttributeInAttr(destination, name, value) { return; // Attribute renames - case "className": + case "className": { attributeName = "class"; + + { + checkAttributeStringCoercion(value, attributeName); + } + + attributeValue = "" + value; break; + } // Booleans - case "hidden": + case "hidden": { if (value === false) { return; } attributeValue = ""; break; + } // Santized URLs case "src": case "href": { + value = sanitizeURL(value); + { checkAttributeStringCoercion(value, attributeName); } - value = sanitizeURL(value); + attributeValue = "" + value; break; } default: { + if ( + // unrecognized event handlers are not SSR'd and we (apparently) + // use on* as hueristic for these handler props + name.length > 2 && + (name[0] === "o" || name[0] === "O") && + (name[1] === "n" || name[1] === "N") + ) { + return; + } + if (!isAttributeNameSafe(name)) { return; } - } - } - if ( - // shouldIgnoreAttribute - // We have already filtered out null/undefined and reserved words. - name.length > 2 && - (name[0] === "o" || name[0] === "O") && - (name[1] === "n" || name[1] === "N") - ) { - return; - } + { + checkAttributeStringCoercion(value, attributeName); + } - { - checkAttributeStringCoercion(value, attributeName); + attributeValue = "" + value; + } } - attributeValue = "" + value; writeChunk(destination, arrayInterstitial); writeChunk( destination, diff --git a/compiled/facebook-www/ReactDOMServerStreaming-prod.modern.js b/compiled/facebook-www/ReactDOMServerStreaming-prod.modern.js index 8ad0e922d89f6..3f817ccc55328 100644 --- a/compiled/facebook-www/ReactDOMServerStreaming-prod.modern.js +++ b/compiled/facebook-www/ReactDOMServerStreaming-prod.modern.js @@ -1689,29 +1689,33 @@ function writeStyleResourceAttributeInJS(destination, name, value) { return; case "className": attributeName = "class"; + name = "" + value; break; case "hidden": if (!1 === value) return; + name = ""; break; case "src": case "href": value = sanitizeURL(value); + name = "" + value; break; default: - if (!isAttributeNameSafe(name)) return; + if ( + (2 < name.length && + ("o" === name[0] || "O" === name[0]) && + ("n" === name[1] || "N" === name[1])) || + !isAttributeNameSafe(name) + ) + return; + name = "" + value; } - if ( - !(2 < name.length) || - ("o" !== name[0] && "O" !== name[0]) || - ("n" !== name[1] && "N" !== name[1]) - ) - (name = "" + value), - (destination.buffer += ","), - (attributeName = escapeJSObjectForInstructionScripts(attributeName)), - (destination.buffer += attributeName), - (destination.buffer += ","), - (attributeName = escapeJSObjectForInstructionScripts(name)), - (destination.buffer += attributeName); + destination.buffer += ","; + attributeName = escapeJSObjectForInstructionScripts(attributeName); + destination.buffer += attributeName; + destination.buffer += ","; + attributeName = escapeJSObjectForInstructionScripts(name); + destination.buffer += attributeName; } function writeStyleResourceDependenciesInAttr(destination, boundaryResources) { destination.buffer += "["; @@ -1787,29 +1791,33 @@ function writeStyleResourceAttributeInAttr(destination, name, value) { return; case "className": attributeName = "class"; + name = "" + value; break; case "hidden": if (!1 === value) return; + name = ""; break; case "src": case "href": value = sanitizeURL(value); + name = "" + value; break; default: - if (!isAttributeNameSafe(name)) return; + if ( + (2 < name.length && + ("o" === name[0] || "O" === name[0]) && + ("n" === name[1] || "N" === name[1])) || + !isAttributeNameSafe(name) + ) + return; + name = "" + value; } - if ( - !(2 < name.length) || - ("o" !== name[0] && "O" !== name[0]) || - ("n" !== name[1] && "N" !== name[1]) - ) - (name = "" + value), - (destination.buffer += ","), - (attributeName = escapeTextForBrowser(JSON.stringify(attributeName))), - (destination.buffer += attributeName), - (destination.buffer += ","), - (attributeName = escapeTextForBrowser(JSON.stringify(name))), - (destination.buffer += attributeName); + destination.buffer += ","; + attributeName = escapeTextForBrowser(JSON.stringify(attributeName)); + destination.buffer += attributeName; + destination.buffer += ","; + attributeName = escapeTextForBrowser(JSON.stringify(name)); + destination.buffer += attributeName; } function prefetchDNS(href) { if (currentResources) {