Skip to content

[Breaking] Remove disableJavaScriptURLs #28615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 27, 2024
Merged

[Breaking] Remove disableJavaScriptURLs #28615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 5 additions & 22 deletions packages/react-dom-bindings/src/shared/sanitizeURL.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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