Skip to content

Split escapeTextForBrowser into escapeTextContentForBrowser and quoteAttributeValueForBrowser #1599

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.

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
Feb 5, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/browser/ui/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var ReactMultiChild = require('ReactMultiChild');
var ReactPerf = require('ReactPerf');

var assign = require('Object.assign');
var escapeTextForBrowser = require('escapeTextForBrowser');
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
var invariant = require('invariant');
var isEventSupported = require('isEventSupported');
var keyOf = require('keyOf');
Expand Down Expand Up @@ -284,7 +284,7 @@ ReactDOMComponent.Mixin = {
CONTENT_TYPES[typeof props.children] ? props.children : null;
var childrenToUse = contentToUse != null ? null : props.children;
if (contentToUse != null) {
return prefix + escapeTextForBrowser(contentToUse);
return prefix + escapeTextContentForBrowser(contentToUse);
} else if (childrenToUse != null) {
var mountImages = this.mountChildren(
childrenToUse,
Expand Down
4 changes: 2 additions & 2 deletions src/browser/ui/ReactDOMTextComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var ReactComponentBrowserEnvironment =
var ReactDOMComponent = require('ReactDOMComponent');

var assign = require('Object.assign');
var escapeTextForBrowser = require('escapeTextForBrowser');
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
var invariant = require('invariant');

/**
Expand Down Expand Up @@ -67,7 +67,7 @@ assign(ReactDOMTextComponent.prototype, {
*/
mountComponent: function(rootID, transaction, context) {
this._rootNodeID = rootID;
var escapedText = escapeTextForBrowser(this._stringText);
var escapedText = escapeTextContentForBrowser(this._stringText);

if (transaction.renderToStaticMarkup) {
// Normally we'd wrap this in a `span` for the reasons stated above, but
Expand Down
17 changes: 17 additions & 0 deletions src/browser/ui/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,23 @@ describe('ReactDOMComponent', function() {
'style={{marginRight: spacing + \'em\'}} when using JSX.'
);
});

it("should properly escape text content and attributes values", function() {
expect(
React.renderToStaticMarkup(
React.DOM.div({
title: '\'"<>&',
style: {
textAlign: '\'"<>&'
}
}, '\'"<>&')
)
).toBe(
'<div title="&#x27;&quot;&lt;&gt;&amp;" style="text-align:&#x27;&quot;&lt;&gt;&amp;;">' +
'&#x27;&quot;&lt;&gt;&amp;' +
'</div>'
);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this!

});

describe('unmountComponent', function() {
Expand Down
13 changes: 7 additions & 6 deletions src/browser/ui/dom/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

var DOMProperty = require('DOMProperty');

var escapeTextForBrowser = require('escapeTextForBrowser');
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');
var memoizeStringOnly = require('memoizeStringOnly');
var warning = require('warning');

Expand All @@ -27,7 +28,7 @@ function shouldIgnoreValue(name, value) {
}

var processAttributeNameAndPrefix = memoizeStringOnly(function(name) {
return escapeTextForBrowser(name) + '="';
return escapeTextContentForBrowser(name) + '=';
});

if (__DEV__) {
Expand Down Expand Up @@ -82,7 +83,7 @@ var DOMPropertyOperations = {
*/
createMarkupForID: function(id) {
return processAttributeNameAndPrefix(DOMProperty.ID_ATTRIBUTE_NAME) +
escapeTextForBrowser(id) + '"';
quoteAttributeValueForBrowser(id);
},

/**
Expand All @@ -101,16 +102,16 @@ var DOMPropertyOperations = {
var attributeName = DOMProperty.getAttributeName[name];
if (DOMProperty.hasBooleanValue[name] ||
(DOMProperty.hasOverloadedBooleanValue[name] && value === true)) {
return escapeTextForBrowser(attributeName);
return escapeTextContentForBrowser(attributeName);
}
return processAttributeNameAndPrefix(attributeName) +
escapeTextForBrowser(value) + '"';
quoteAttributeValueForBrowser(value);
} else if (DOMProperty.isCustomAttribute(name)) {
if (value == null) {
return '';
}
return processAttributeNameAndPrefix(name) +
escapeTextForBrowser(value) + '"';
quoteAttributeValueForBrowser(value);
} else if (__DEV__) {
warnUnknownProperty(name);
}
Expand Down
4 changes: 2 additions & 2 deletions src/browser/ui/dom/setTextContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"use strict";

var ExecutionEnvironment = require('ExecutionEnvironment');
var escapeTextForBrowser = require('escapeTextForBrowser');
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
var setInnerHTML = require('setInnerHTML');

/**
Expand All @@ -39,7 +39,7 @@ var setTextContent = function(node, text) {
if (ExecutionEnvironment.canUseDOM) {
if (!('textContent' in document.documentElement)) {
setTextContent = function(node, text) {
setInnerHTML(node, escapeTextForBrowser(text));
setInnerHTML(node, escapeTextContentForBrowser(text));
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@

'use strict';

describe('escapeTextForBrowser', function() {
describe('escapeTextContentForBrowser', function() {

var escapeTextForBrowser = require('escapeTextForBrowser');
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');

it('should escape boolean to string', function() {
expect(escapeTextForBrowser(true)).toBe('true');
expect(escapeTextForBrowser(false)).toBe('false');
expect(escapeTextContentForBrowser(true)).toBe('true');
expect(escapeTextContentForBrowser(false)).toBe('false');
});

it('should escape object to string', function() {
var escaped = escapeTextForBrowser({
var escaped = escapeTextContentForBrowser({
toString: function() {
return 'ponys';
}
Expand All @@ -31,17 +31,17 @@ describe('escapeTextForBrowser', function() {
});

it('should escape number to string', function() {
expect(escapeTextForBrowser(42)).toBe('42');
expect(escapeTextContentForBrowser(42)).toBe('42');
});

it('should escape string', function() {
var escaped = escapeTextForBrowser('<script type=\'\' src=""></script>');
var escaped = escapeTextContentForBrowser('<script type=\'\' src=""></script>');
expect(escaped).not.toContain('<');
expect(escaped).not.toContain('>');
expect(escaped).not.toContain('\'');
expect(escaped).not.toContain('\"');

escaped = escapeTextForBrowser('&');
escaped = escapeTextContentForBrowser('&');
expect(escaped).toBe('&amp;');
});

Expand Down
48 changes: 48 additions & 0 deletions src/utils/__tests__/quoteAttributeValueForBrowser-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Copyright 2013-2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @emails react-core
*/

"use strict";

describe('quoteAttributeValueForBrowser', function() {

var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser');

it('should escape boolean to string', function() {
expect(quoteAttributeValueForBrowser(true)).toBe('"true"');
expect(quoteAttributeValueForBrowser(false)).toBe('"false"');
});

it('should escape object to string', function() {
var escaped = quoteAttributeValueForBrowser({
toString: function() {
return 'ponys';
}
});

expect(escaped).toBe('"ponys"');
});

it('should escape number to string', function() {
expect(quoteAttributeValueForBrowser(42)).toBe('"42"');
});

it('should escape string', function() {
var escaped = quoteAttributeValueForBrowser('<script type=\'\' src=""></script>');
expect(escaped).not.toContain('<');
expect(escaped).not.toContain('>');
expect(escaped).not.toContain('\'');
expect(escaped.substr(1, -1)).not.toContain('\"');

escaped = quoteAttributeValueForBrowser('&');
expect(escaped).toBe('"&amp;"');
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule escapeTextForBrowser
* @typechecks static-only
* @providesModule escapeTextContentForBrowser
*/

'use strict';
Expand All @@ -32,8 +31,8 @@ function escaper(match) {
* @param {*} text Text value to escape.
* @return {string} An escaped string.
*/
function escapeTextForBrowser(text) {
function escapeTextContentForBrowser(text) {
return ('' + text).replace(ESCAPE_REGEX, escaper);
}

module.exports = escapeTextForBrowser;
module.exports = escapeTextContentForBrowser;
26 changes: 26 additions & 0 deletions src/utils/quoteAttributeValueForBrowser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Copyright 2013-2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule quoteAttributeValueForBrowser
*/

"use strict";

var escapeTextContentForBrowser = require('escapeTextContentForBrowser');

/**
* Escapes attribute value to prevent scripting attacks.
*
* @param {*} value Value to escape.
* @return {string} An escaped string.
*/
function quoteAttributeValueForBrowser(value) {
return '"' + escapeTextContentForBrowser(value) + '"';
}

module.exports = quoteAttributeValueForBrowser;