Skip to content

Commit

Permalink
Fix logic around attribute seralization (#26526)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gnoff committed Apr 3, 2023
1 parent b14f8da commit 4a1cc2d
Showing 1 changed file with 46 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3926,48 +3926,51 @@ function writeStyleResourceAttributeInJS(
return;

// Attribute renames
case 'className':
case 'className': {
attributeName = 'class';
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
attributeValue = '' + (value: any);
break;

}
// Booleans
case 'hidden':
case 'hidden': {
if (value === false) {
return;
}
attributeValue = '';
break;

}
// Santized URLs
case 'src':
case 'href': {
value = sanitizeURL(value);
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
value = sanitizeURL(value);
attributeValue = '' + (value: any);
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 (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
attributeValue = '' + (value: any);
}
}
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;
}
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
attributeValue = '' + (value: any);
writeChunk(destination, arrayInterstitial);
writeChunk(
destination,
Expand Down Expand Up @@ -4119,48 +4122,53 @@ function writeStyleResourceAttributeInAttr(
return;

// Attribute renames
case 'className':
case 'className': {
attributeName = 'class';
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
attributeValue = '' + (value: any);
break;
}

// Booleans
case 'hidden':
case 'hidden': {
if (value === false) {
return;
}
attributeValue = '';
break;
}

// Santized URLs
case 'src':
case 'href': {
value = sanitizeURL(value);
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
value = sanitizeURL(value);
attributeValue = '' + (value: any);
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 (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
attributeValue = '' + (value: any);
}
}
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;
}
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
attributeValue = '' + (value: any);
writeChunk(destination, arrayInterstitial);
writeChunk(
destination,
Expand Down

0 comments on commit 4a1cc2d

Please sign in to comment.