diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index bfb3eddfc5..c27066b0cc 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -17,7 +17,6 @@ import { } from '../shared/DOMProperty'; import sanitizeURL from '../shared/sanitizeURL'; import { - disableJavaScriptURLs, enableTrustedTypesIntegration, enableCustomElementPropertySupport, enableFilterEmptyStringAttributesDOM, @@ -43,15 +42,6 @@ export function getValueForProperty( const {propertyName} = propertyInfo; return (node: any)[propertyName]; } - if (!disableJavaScriptURLs && propertyInfo.sanitizeURL) { - // If we haven't fully disabled javascript: URLs, and if - // the hydration is successful of a javascript: URL, we - // still want to warn on the client. - if (__DEV__) { - checkAttributeStringCoercion(expected, name); - } - sanitizeURL('' + (expected: any)); - } const attributeName = propertyInfo.attributeName; @@ -134,6 +124,11 @@ export function getValueForProperty( } // shouldRemoveAttribute + switch (typeof expected) { + case 'function': + case 'symbol': // eslint-disable-line + return value; + } switch (propertyInfo.type) { case BOOLEAN: { if (expected) { @@ -175,6 +170,16 @@ export function getValueForProperty( if (__DEV__) { checkAttributeStringCoercion(expected, name); } + if (propertyInfo.sanitizeURL) { + // We have already verified this above. + // eslint-disable-next-line react-internal/safe-string-coercion + if (value === '' + (sanitizeURL(expected): any)) { + return expected; + } + return value; + } + // We have already verified this above. + // eslint-disable-next-line react-internal/safe-string-coercion if (value === '' + (expected: any)) { return expected; } @@ -395,19 +400,25 @@ export function setValueForProperty(node: Element, name: string, value: mixed) { } break; default: { + if (__DEV__) { + checkAttributeStringCoercion(value, attributeName); + } let attributeValue; // `setAttribute` with objects becomes only `[object]` in IE8/9, // ('' + value) makes it output the correct toString()-value. if (enableTrustedTypesIntegration) { - attributeValue = (value: any); - } else { - if (__DEV__) { - checkAttributeStringCoercion(value, attributeName); + if (propertyInfo.sanitizeURL) { + attributeValue = (sanitizeURL(value): any); + } else { + attributeValue = (value: any); } + } else { + // We have already verified this above. + // eslint-disable-next-line react-internal/safe-string-coercion attributeValue = '' + (value: any); - } - if (propertyInfo.sanitizeURL) { - sanitizeURL(attributeValue.toString()); + if (propertyInfo.sanitizeURL) { + attributeValue = sanitizeURL(attributeValue); + } } const attributeNamespace = propertyInfo.attributeNamespace; if (attributeNamespace) { diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 09c879e8aa..543fa064ea 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -736,12 +736,13 @@ function pushAttribute( } break; default: + if (__DEV__) { + checkAttributeStringCoercion(value, attributeName); + } if (propertyInfo.sanitizeURL) { - if (__DEV__) { - checkAttributeStringCoercion(value, attributeName); - } - value = '' + (value: any); - sanitizeURL(value); + // We've already checked above. + // eslint-disable-next-line react-internal/safe-string-coercion + value = sanitizeURL('' + (value: any)); } target.push( attributeSeparator, @@ -3844,15 +3845,12 @@ function writeStyleResourceDependencyHrefOnlyInJS( function writeStyleResourceDependencyInJS( destination: Destination, - href: string, - precedence: string, + href: mixed, + precedence: mixed, props: Object, ) { - if (__DEV__) { - checkAttributeStringCoercion(href, 'href'); - } - const coercedHref = '' + (href: any); - sanitizeURL(coercedHref); + // eslint-disable-next-line react-internal/safe-string-coercion + const coercedHref = sanitizeURL('' + (href: any)); writeChunk( destination, stringToChunk(escapeJSObjectForInstructionScripts(coercedHref)), @@ -3939,8 +3937,7 @@ function writeStyleResourceAttributeInJS( if (__DEV__) { checkAttributeStringCoercion(value, attributeName); } - attributeValue = '' + (value: any); - sanitizeURL(attributeValue); + value = sanitizeURL(value); break; } default: { @@ -4041,15 +4038,12 @@ function writeStyleResourceDependencyHrefOnlyInAttr( function writeStyleResourceDependencyInAttr( destination: Destination, - href: string, - precedence: string, + href: mixed, + precedence: mixed, props: Object, ) { - if (__DEV__) { - checkAttributeStringCoercion(href, 'href'); - } - const coercedHref = '' + (href: any); - sanitizeURL(coercedHref); + // eslint-disable-next-line react-internal/safe-string-coercion + const coercedHref = sanitizeURL('' + (href: any)); writeChunk( destination, stringToChunk(escapeTextForBrowser(JSON.stringify(coercedHref))), @@ -4136,8 +4130,7 @@ function writeStyleResourceAttributeInAttr( if (__DEV__) { checkAttributeStringCoercion(value, attributeName); } - attributeValue = '' + (value: any); - sanitizeURL(attributeValue); + value = sanitizeURL(value); break; } default: { diff --git a/packages/react-dom-bindings/src/shared/sanitizeURL.js b/packages/react-dom-bindings/src/shared/sanitizeURL.js index f112ec2f1b..b3de4657e9 100644 --- a/packages/react-dom-bindings/src/shared/sanitizeURL.js +++ b/packages/react-dom-bindings/src/shared/sanitizeURL.js @@ -24,24 +24,29 @@ const isJavaScriptProtocol = let didWarn = false; -function sanitizeURL(url: string) { +function sanitizeURL(url: T): T | string { + // We should never have symbols here because they get filtered out elsewhere. + // eslint-disable-next-line react-internal/safe-string-coercion + const stringifiedURL = '' + (url: any); if (disableJavaScriptURLs) { - if (isJavaScriptProtocol.test(url)) { - throw new Error( - 'React has blocked a javascript: URL as a security precaution.', - ); + if (isJavaScriptProtocol.test(stringifiedURL)) { + // Return a different javascript: url that doesn't cause any side-effects and just + // throws if ever visited. + // eslint-disable-next-line no-script-url + return "javascript:throw new Error('React has blocked a javascript: URL as a security precaution.')"; } } else if (__DEV__) { - if (!didWarn && isJavaScriptProtocol.test(url)) { + if (!didWarn && isJavaScriptProtocol.test(stringifiedURL)) { didWarn = true; console.error( 'A future version of React will block javascript: URLs as a security precaution. ' + 'Use event handlers instead if you can. If you need to generate unsafe HTML try ' + 'using dangerouslySetInnerHTML instead. React was passed %s.', - JSON.stringify(url), + JSON.stringify(stringifiedURL), ); } } + return url; } export default sanitizeURL; diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js index 6aadcb7574..c7da08897a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js @@ -19,134 +19,8 @@ let ReactDOM; let ReactDOMServer; let ReactTestUtils; -function runTests(itRenders, itRejectsRendering, expectToReject) { - itRenders('a http link with the word javascript in it', async render => { - const e = await render( - Click me, - ); - expect(e.tagName).toBe('A'); - expect(e.href).toBe('http://javascript:0/thisisfine'); - }); - - itRejectsRendering('a javascript protocol href', async render => { - // Only the first one warns. The second warning is deduped. - const e = await render( -
- p0wned - p0wned again -
, - 1, - ); - expect(e.firstChild.href).toBe('javascript:notfine'); - expect(e.lastChild.href).toBe('javascript:notfineagain'); - }); - - itRejectsRendering( - 'a javascript protocol with leading spaces', - async render => { - const e = await render( - p0wned, - 1, - ); - // We use an approximate comparison here because JSDOM might not parse - // \u0000 in HTML properly. - expect(e.href).toContain('notfine'); - }, - ); - - itRejectsRendering( - 'a javascript protocol with intermediate new lines and mixed casing', - async render => { - const e = await render( - p0wned, - 1, - ); - expect(e.href).toBe('javascript:notfine'); - }, - ); - - itRejectsRendering('a javascript protocol area href', async render => { - const e = await render( - - - , - 1, - ); - expect(e.firstChild.href).toBe('javascript:notfine'); - }); - - itRejectsRendering('a javascript protocol form action', async render => { - const e = await render(
p0wned
, 1); - expect(e.action).toBe('javascript:notfine'); - }); - - itRejectsRendering( - 'a javascript protocol button formAction', - async render => { - const e = await render(, 1); - expect(e.getAttribute('formAction')).toBe('javascript:notfine'); - }, - ); - - itRejectsRendering('a javascript protocol input formAction', async render => { - const e = await render( - , - 1, - ); - expect(e.getAttribute('formAction')).toBe('javascript:notfine'); - }); - - itRejectsRendering('a javascript protocol iframe src', async render => { - const e = await render(