diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index 7b5178dda8303..c27066b0cc31d 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); - } 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; } diff --git a/packages/react-dom-bindings/src/shared/sanitizeURL.js b/packages/react-dom-bindings/src/shared/sanitizeURL.js index 8cde90e7dfc0c..b3de4657e9158 100644 --- a/packages/react-dom-bindings/src/shared/sanitizeURL.js +++ b/packages/react-dom-bindings/src/shared/sanitizeURL.js @@ -30,9 +30,10 @@ function sanitizeURL(url: T): T | string { const stringifiedURL = '' + (url: any); if (disableJavaScriptURLs) { if (isJavaScriptProtocol.test(stringifiedURL)) { - throw new Error( - 'React has blocked a javascript: URL as a security precaution.', - ); + // 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(stringifiedURL)) { diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js index 6aadcb75749fb..c7da08897ae37 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js @@ -19,7 +19,38 @@ let ReactDOM; let ReactDOMServer; let ReactTestUtils; -function runTests(itRenders, itRejectsRendering, expectToReject) { +const EXPECTED_SAFE_URL = + "javascript:throw new Error('React has blocked a javascript: URL as a security precaution.')"; + +describe('ReactDOMServerIntegration - Untrusted URLs', () => { + // The `itRenders` helpers don't work with the gate pragma, so we have to do + // this instead. + if (gate(flags => flags.disableJavaScriptURLs)) { + it("empty test so Jest doesn't complain", () => {}); + return; + } + + function initModules() { + jest.resetModules(); + React = require('react'); + ReactDOM = require('react-dom'); + ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); + + // Make them available to the helpers. + return { + ReactDOM, + ReactDOMServer, + ReactTestUtils, + }; + } + + const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules); + + beforeEach(() => { + resetModules(); + }); + itRenders('a http link with the word javascript in it', async render => { const e = await render( Click me, @@ -28,7 +59,7 @@ function runTests(itRenders, itRejectsRendering, expectToReject) { expect(e.href).toBe('http://javascript:0/thisisfine'); }); - itRejectsRendering('a javascript protocol href', async render => { + itRenders('a javascript protocol href', async render => { // Only the first one warns. The second warning is deduped. const e = await render(
@@ -41,20 +72,17 @@ function runTests(itRenders, itRejectsRendering, expectToReject) { 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'); - }, - ); + itRenders('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( + itRenders( 'a javascript protocol with intermediate new lines and mixed casing', async render => { const e = await render( @@ -65,7 +93,7 @@ function runTests(itRenders, itRejectsRendering, expectToReject) { }, ); - itRejectsRendering('a javascript protocol area href', async render => { + itRenders('a javascript protocol area href', async render => { const e = await render( @@ -75,20 +103,17 @@ function runTests(itRenders, itRejectsRendering, expectToReject) { expect(e.firstChild.href).toBe('javascript:notfine'); }); - itRejectsRendering('a javascript protocol form action', async render => { + itRenders('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'); - }, - ); + itRenders('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 => { + itRenders('a javascript protocol input formAction', async render => { const e = await render( , 1, @@ -96,12 +121,12 @@ function runTests(itRenders, itRejectsRendering, expectToReject) { expect(e.getAttribute('formAction')).toBe('javascript:notfine'); }); - itRejectsRendering('a javascript protocol iframe src', async render => { + itRenders('a javascript protocol iframe src', async render => { const e = await render(