Skip to content

Commit 58e709f

Browse files
committed
[Trusted Types] Don't stringify DOM attributes (#19588 redo)
Summary: This is a resubmit of #19588. Fixes #19587.
1 parent c5b9375 commit 58e709f

File tree

4 files changed

+77
-10
lines changed

4 files changed

+77
-10
lines changed

packages/react-dom-bindings/src/client/DOMPropertyOperations.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
import isAttributeNameSafe from '../shared/isAttributeNameSafe';
1111
import {
12-
enableTrustedTypesIntegration,
1312
enableCustomElementPropertySupport,
1413
} from 'shared/ReactFeatureFlags';
1514
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
@@ -135,7 +134,7 @@ export function setValueForAttribute(
135134
}
136135
node.setAttribute(
137136
name,
138-
enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
137+
(value: any)
139138
);
140139
}
141140
}
@@ -163,7 +162,7 @@ export function setValueForKnownAttribute(
163162
}
164163
node.setAttribute(
165164
name,
166-
enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
165+
(value: any)
167166
);
168167
}
169168

@@ -192,7 +191,7 @@ export function setValueForNamespacedAttribute(
192191
node.setAttributeNS(
193192
namespace,
194193
name,
195-
enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
194+
(value: any)
196195
);
197196
}
198197

packages/react-dom-bindings/src/shared/sanitizeURL.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ const isJavaScriptProtocol =
2525
let didWarn = false;
2626

2727
function sanitizeURL<T>(url: T): T | string {
28+
if (
29+
typeof trustedTypes === 'undefined' ||
30+
!trustedTypes.isScriptURL(url)
31+
) {
32+
// Coerce to a string, unless we know it's an immutable TrustedScriptURL object.
33+
url = '' + url;
34+
}
35+
2836
// We should never have symbols here because they get filtered out elsewhere.
2937
// eslint-disable-next-line react-internal/safe-string-coercion
3038
const stringifiedURL = '' + (url: any);

packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,34 @@ describe('when Trusted Types are available in global object', () => {
1717
let ttObject1;
1818
let ttObject2;
1919

20+
const expectToReject = fn => {
21+
let msg;
22+
try {
23+
fn();
24+
} catch (x) {
25+
msg = x.message;
26+
}
27+
expect(msg).toContain(
28+
'React has blocked a javascript: URL as a security precaution.',
29+
);
30+
};
31+
2032
beforeEach(() => {
2133
jest.resetModules();
2234
container = document.createElement('div');
23-
const fakeTTObjects = new Set();
35+
fakeTTObjects = new Set();
2436
window.trustedTypes = {
2537
isHTML: function (value) {
2638
if (this !== window.trustedTypes) {
2739
throw new Error(this);
2840
}
2941
return fakeTTObjects.has(value);
3042
},
31-
isScript: () => false,
32-
isScriptURL: () => false,
43+
isScript: value => fakeTTObjects.has(value),
44+
isScriptURL: value => fakeTTObjects.has(value),
3345
};
3446
ReactFeatureFlags = require('shared/ReactFeatureFlags');
35-
ReactFeatureFlags.enableTrustedTypesIntegration = true;
47+
ReactFeatureFlags.disableJavaScriptURLs = true;
3648
React = require('react');
3749
ReactDOM = require('react-dom');
3850
ttObject1 = {
@@ -152,6 +164,56 @@ describe('when Trusted Types are available in global object', () => {
152164
}
153165
});
154166

167+
it('should not stringify attributes that go through sanitizeURL', () => {
168+
const setAttribute = Element.prototype.setAttribute;
169+
try {
170+
const setAttributeCalls = [];
171+
Element.prototype.setAttribute = function(name, value) {
172+
setAttributeCalls.push([this, name.toLowerCase(), value]);
173+
return setAttribute.apply(this, arguments);
174+
};
175+
const trustedScriptUrlHttps = {
176+
toString: () => 'https://ok.example',
177+
};
178+
fakeTTObjects.add(trustedScriptUrlHttps);
179+
// It's not a matching type (under Trusted Types a.href a string will do),
180+
// but a.href undergoes URL and we're only testing if the value was
181+
// passed unmodified to setAttribute.
182+
ReactDOM.render(<a href={trustedScriptUrlHttps} />, container);
183+
expect(setAttributeCalls.length).toBe(1);
184+
expect(setAttributeCalls[0][0]).toBe(container.firstChild);
185+
expect(setAttributeCalls[0][1]).toBe('href');
186+
// Ensure it didn't get stringified when passed to a DOM sink:
187+
expect(setAttributeCalls[0][2]).toBe(trustedScriptUrlHttps);
188+
} finally {
189+
Element.prototype.setAttribute = setAttribute;
190+
}
191+
});
192+
193+
it('should sanitize attributes even if they are Trusted Types', () => {
194+
const setAttribute = Element.prototype.setAttribute;
195+
try {
196+
const setAttributeCalls = [];
197+
Element.prototype.setAttribute = function(name, value) {
198+
setAttributeCalls.push([this, name.toLowerCase(), value]);
199+
return setAttribute.apply(this, arguments);
200+
};
201+
const trustedScriptUrlJavascript = {
202+
// eslint-disable-next-line no-script-url
203+
toString: () => 'javascript:notfine',
204+
};
205+
fakeTTObjects.add(trustedScriptUrlJavascript);
206+
// Assert that the URL sanitization will correctly unwrap and verify the
207+
// value.
208+
expectToReject(() => {
209+
ReactDOM.render(<a href={trustedScriptUrlJavascript} />, container);
210+
});
211+
expect(setAttributeCalls.length).toBe(0);
212+
} finally {
213+
Element.prototype.setAttribute = setAttribute;
214+
}
215+
});
216+
155217
it('should not stringify trusted values for setAttributeNS', () => {
156218
const setAttributeNS = Element.prototype.setAttributeNS;
157219
try {

packages/shared/ReactFeatureFlags.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,6 @@ export const disableCommentsAsDOMContainers = true;
176176
// Disable javascript: URL strings in href for XSS protection.
177177
export const disableJavaScriptURLs = false;
178178

179-
export const enableTrustedTypesIntegration = false;
180-
181179
// Prevent the value and checked attributes from syncing with their related
182180
// DOM properties
183181
export const disableInputAttributeSyncing = false;

0 commit comments

Comments
 (0)