Skip to content

Commit

Permalink
[Breaking] Remove disableJavaScriptURLs
Browse files Browse the repository at this point in the history
  • Loading branch information
@rickhanlonii
rickhanlonii committed Mar 22, 2024
1 parent 6708115 commit 236181f
Show file tree
Hide file tree
Showing 9 changed files with 5 additions and 203 deletions.
27 changes: 5 additions & 22 deletions packages/react-dom-bindings/src/shared/sanitizeURL.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
* @flow
*/

import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags';

// A javascript: URL can contain leading C0 control or \u0020 SPACE,
// and any newline or tab are filtered out as if they're not part of the URL.
// https://url.spec.whatwg.org/#url-parsing
Expand All @@ -22,29 +20,14 @@ import {disableJavaScriptURLs} from 'shared/ReactFeatureFlags';
const isJavaScriptProtocol =
/^[\u0000-\u001F ]*j[\r\n\t]*a[\r\n\t]*v[\r\n\t]*a[\r\n\t]*s[\r\n\t]*c[\r\n\t]*r[\r\n\t]*i[\r\n\t]*p[\r\n\t]*t[\r\n\t]*\:/i;

let didWarn = false;

function sanitizeURL<T>(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(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(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(stringifiedURL),
);
}
if (isJavaScriptProtocol.test('' + (url: any))) {
// 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.')";
}
return url;
}
Expand Down
170 changes: 0 additions & 170 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,176 +23,6 @@ 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');
ReactDOMClient = require('react-dom/client');
ReactDOMServer = require('react-dom/server');
act = require('internal-test-utils').act;

// Make them available to the helpers.
return {
ReactDOMClient,
ReactDOMServer,
};
}

const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);

beforeEach(() => {
resetModules();
});

itRenders('a http link with the word javascript in it', async render => {
const e = await render(
<a href="http://javascript:0/thisisfine">Click me</a>,
);
expect(e.tagName).toBe('A');
expect(e.href).toBe('http://javascript:0/thisisfine');
});

itRenders('a javascript protocol href', async render => {
// Only the first one warns. The second warning is deduped.
const e = await render(
<div>
<a href="javascript:notfine">p0wned</a>
<a href="javascript:notfineagain">p0wned again</a>
</div>,
1,
);
expect(e.firstChild.href).toBe('javascript:notfine');
expect(e.lastChild.href).toBe('javascript:notfineagain');
});

itRenders('a javascript protocol with leading spaces', async render => {
const e = await render(
<a href={' \t \u0000\u001F\u0003javascript\n: notfine'}>p0wned</a>,
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 intermediate new lines and mixed casing',
async render => {
const e = await render(
<a href={'\t\r\n Jav\rasCr\r\niP\t\n\rt\n:notfine'}>p0wned</a>,
1,
);
expect(e.href).toBe('javascript:notfine');
},
);

itRenders('a javascript protocol area href', async render => {
const e = await render(
<map>
<area href="javascript:notfine" />
</map>,
1,
);
expect(e.firstChild.href).toBe('javascript:notfine');
});

itRenders('a javascript protocol form action', async render => {
const e = await render(<form action="javascript:notfine">p0wned</form>, 1);
expect(e.action).toBe('javascript:notfine');
});

itRenders('a javascript protocol input formAction', async render => {
const e = await render(
<input type="submit" formAction="javascript:notfine" />,
1,
);
expect(e.getAttribute('formAction')).toBe('javascript:notfine');
});

itRenders('a javascript protocol button formAction', async render => {
const e = await render(
<button formAction="javascript:notfine">p0wned</button>,
1,
);
expect(e.getAttribute('formAction')).toBe('javascript:notfine');
});

itRenders('a javascript protocol iframe src', async render => {
const e = await render(<iframe src="javascript:notfine" />, 1);
expect(e.src).toBe('javascript:notfine');
});

itRenders('a javascript protocol frame src', async render => {
const e = await render(
<html>
<head />
<frameset>
<frame src="javascript:notfine" />
</frameset>
</html>,
1,
);
expect(e.lastChild.firstChild.src).toBe('javascript:notfine');
});

itRenders('a javascript protocol in an SVG link', async render => {
const e = await render(
<svg>
<a href="javascript:notfine" />
</svg>,
1,
);
expect(e.firstChild.getAttribute('href')).toBe('javascript:notfine');
});

itRenders(
'a javascript protocol in an SVG link with a namespace',
async render => {
const e = await render(
<svg>
<a xlinkHref="javascript:notfine" />
</svg>,
1,
);
expect(
e.firstChild.getAttributeNS('http://www.w3.org/1999/xlink', 'href'),
).toBe('javascript:notfine');
},
);

it('rejects a javascript protocol href if it is added during an update', async () => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
await act(async () => {
root.render(<a href="thisisfine">click me</a>);
});
await expect(async () => {
await act(() => {
root.render(<a href="javascript:notfine">click me</a>);
});
}).toErrorDev(
'Warning: 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 "javascript:notfine".\n' +
' in a (at **)',
);
});
});

describe('ReactDOMServerIntegration - Untrusted URLs - disableJavaScriptURLs', () => {
// 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();

Expand Down
4 changes: 0 additions & 4 deletions packages/shared/ReactFeatureFlags.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,6 @@ const __NEXT_MAJOR__ = __EXPERIMENTAL__;
// Removes legacy style context
export const disableLegacyContext = __NEXT_MAJOR__;

// Not ready to break experimental yet.
// Disable javascript: URL strings in href for XSS protection.
export const disableJavaScriptURLs = __NEXT_MAJOR__;

// Not ready to break experimental yet.
// Modern <StrictMode /> behaviour aligns more with what components
// components will encounter in production, especially when used With <Offscreen />.
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const enableBinaryFlight = true;
export const enableTaint = true;
export const enablePostpone = false;
export const debugRenderPhaseSideEffectsForStrictMode = __DEV__;
export const disableJavaScriptURLs = true;
export const disableCommentsAsDOMContainers = true;
export const disableInputAttributeSyncing = false;
export const disableIEWorkarounds = true;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export const enableFormActions = true; // Doesn't affect Native
export const enableBinaryFlight = true;
export const enableTaint = true;
export const enablePostpone = false;
export const disableJavaScriptURLs = true;
export const disableCommentsAsDOMContainers = true;
export const disableInputAttributeSyncing = false;
export const disableIEWorkarounds = true;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export const enableFormActions = true; // Doesn't affect Test Renderer
export const enableBinaryFlight = true;
export const enableTaint = true;
export const enablePostpone = false;
export const disableJavaScriptURLs = true;
export const disableCommentsAsDOMContainers = true;
export const disableInputAttributeSyncing = false;
export const disableIEWorkarounds = true;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export const enableFormActions = true; // Doesn't affect Test Renderer
export const enableBinaryFlight = true;
export const enableTaint = true;
export const enablePostpone = false;
export const disableJavaScriptURLs = true;
export const disableCommentsAsDOMContainers = true;
export const disableInputAttributeSyncing = false;
export const disableIEWorkarounds = true;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export const enableFormActions = true; // Doesn't affect Test Renderer
export const enableBinaryFlight = true;
export const enableTaint = true;
export const enablePostpone = false;
export const disableJavaScriptURLs = true;
export const disableCommentsAsDOMContainers = true;
export const disableInputAttributeSyncing = false;
export const disableIEWorkarounds = true;
Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.www.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ export const enableTaint = false;

export const enablePostpone = false;

export const disableJavaScriptURLs = true;

// TODO: www currently relies on this feature. It's disabled in open source.
// Need to remove it.
export const disableCommentsAsDOMContainers = false;
Expand Down

0 comments on commit 236181f

Please sign in to comment.