Skip to content

Commit

Permalink
Generate safe javascript url instead of throwing
Browse files Browse the repository at this point in the history
  • Loading branch information
sebmarkbage committed Mar 29, 2023
1 parent 666589e commit 2658946
Show file tree
Hide file tree
Showing 3 changed files with 196 additions and 108 deletions.
25 changes: 15 additions & 10 deletions packages/react-dom-bindings/src/client/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
} from '../shared/DOMProperty';
import sanitizeURL from '../shared/sanitizeURL';
import {
disableJavaScriptURLs,
enableTrustedTypesIntegration,
enableCustomElementPropertySupport,
enableFilterEmptyStringAttributesDOM,
Expand All @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down
7 changes: 4 additions & 3 deletions packages/react-dom-bindings/src/shared/sanitizeURL.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ function sanitizeURL<T>(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)) {
Expand Down
Loading

0 comments on commit 2658946

Please sign in to comment.