Skip to content

Commit

Permalink
Drop .textContent IE8 polyfill and rewrite escaping tests against pub…
Browse files Browse the repository at this point in the history
…lic API (facebook#11331)

* Rename escapeText util. Test quoteAttributeValueForBrowser through ReactDOMServer API

* Fix lint errors

* Prettier reformatting

* Change syntax to prevent prettier escape doble quote

* Name and description gardening. Add tests for escapeTextForBrowser. Add missing tests

* Improve script tag as text content test

* Update escapeTextForBrowser-test.js

* Update quoteAttributeValueForBrowser-test.js

* Simplify tests

* Move utilities to server folder
  • Loading branch information
jeremenichelli authored and raphamorim committed Nov 27, 2017
1 parent f92cb85 commit e391819
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 107 deletions.
7 changes: 0 additions & 7 deletions packages/react-dom/src/__tests__/ReactServerRendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,6 @@ describe('ReactDOMServer', () => {
expect(response).toMatch(new RegExp('<img data-reactroot=""' + '/>'));
});

it('should generate simple markup for attribute with `>` symbol', () => {
var response = ReactDOMServer.renderToString(<img data-attr=">" />);
expect(response).toMatch(
new RegExp('<img data-attr="&gt;" data-reactroot=""' + '/>'),
);
});

it('should generate comment markup for component returns null', () => {
class NullComponent extends React.Component {
render() {
Expand Down

This file was deleted.

66 changes: 66 additions & 0 deletions packages/react-dom/src/__tests__/escapeTextForBrowser-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/

'use strict';

var React;
var ReactDOMServer;

describe('escapeTextForBrowser', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOMServer = require('react-dom/server');
});

it('ampersand is escaped when passed as text content', () => {
var response = ReactDOMServer.renderToString(<span>{'&'}</span>);
expect(response).toMatch('<span data-reactroot="">&amp;</span>');
});

it('double quote is escaped when passed as text content', () => {
var response = ReactDOMServer.renderToString(<span>{'"'}</span>);
expect(response).toMatch('<span data-reactroot="">&quot;</span>');
});

it('single quote is escaped when passed as text content', () => {
var response = ReactDOMServer.renderToString(<span>{"'"}</span>);
expect(response).toMatch('<span data-reactroot="">&#x27;</span>');
});

it('greater than entity is escaped when passed as text content', () => {
var response = ReactDOMServer.renderToString(<span>{'>'}</span>);
expect(response).toMatch('<span data-reactroot="">&gt;</span>');
});

it('lower than entity is escaped when passed as text content', () => {
var response = ReactDOMServer.renderToString(<span>{'<'}</span>);
expect(response).toMatch('<span data-reactroot="">&lt;</span>');
});

it('number is correctly passed as text content', () => {
var response = ReactDOMServer.renderToString(<span>{42}</span>);
expect(response).toMatch('<span data-reactroot="">42</span>');
});

it('number is escaped to string when passed as text content', () => {
var response = ReactDOMServer.renderToString(<img data-attr={42} />);
expect(response).toMatch('<img data-attr="42" data-reactroot=""/>');
});

it('escape text content representing a script tag', () => {
var response = ReactDOMServer.renderToString(
<span>{'<script type=\'\' src=""></script>'}</span>,
);
expect(response).toMatch(
'<span data-reactroot="">&lt;script type=&#x27;&#x27; ' +
'src=&quot;&quot;&gt;&lt;/script&gt;</span>',
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,40 +9,67 @@

'use strict';

var React;
var ReactDOMServer;

describe('quoteAttributeValueForBrowser', () => {
// TODO: can we express this test with only public API?
var quoteAttributeValueForBrowser = require('../shared/quoteAttributeValueForBrowser')
.default;
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOMServer = require('react-dom/server');
});

it('should escape boolean to string', () => {
expect(quoteAttributeValueForBrowser(true)).toBe('"true"');
expect(quoteAttributeValueForBrowser(false)).toBe('"false"');
it('ampersand is escaped inside attributes', () => {
var response = ReactDOMServer.renderToString(<img data-attr="&" />);
expect(response).toMatch('<img data-attr="&amp;" data-reactroot=""/>');
});

it('should escape object to string', () => {
var escaped = quoteAttributeValueForBrowser({
toString: function() {
return 'ponys';
},
});
it('double quote is escaped inside attributes', () => {
var response = ReactDOMServer.renderToString(<img data-attr={'"'} />);
expect(response).toMatch('<img data-attr="&quot;" data-reactroot=""/>');
});

it('single quote is escaped inside attributes', () => {
var response = ReactDOMServer.renderToString(<img data-attr="'" />);
expect(response).toMatch('<img data-attr="&#x27;" data-reactroot=""/>');
});

expect(escaped).toBe('"ponys"');
it('greater than entity is escaped inside attributes', () => {
var response = ReactDOMServer.renderToString(<img data-attr=">" />);
expect(response).toMatch('<img data-attr="&gt;" data-reactroot=""/>');
});

it('should escape number to string', () => {
expect(quoteAttributeValueForBrowser(42)).toBe('"42"');
it('lower than entity is escaped inside attributes', () => {
var response = ReactDOMServer.renderToString(<img data-attr="<" />);
expect(response).toMatch('<img data-attr="&lt;" data-reactroot=""/>');
});

it('should escape string', () => {
var escaped = quoteAttributeValueForBrowser(
'<script type=\'\' src=""></script>',
it('number is escaped to string inside attributes', () => {
var response = ReactDOMServer.renderToString(<img data-attr={42} />);
expect(response).toMatch('<img data-attr="42" data-reactroot=""/>');
});

it('object is passed to a string inside attributes', () => {
var sampleObject = {
toString: function() {
return 'ponys';
},
};

var response = ReactDOMServer.renderToString(
<img data-attr={sampleObject} />,
);
expect(escaped).not.toContain('<');
expect(escaped).not.toContain('>');
expect(escaped).not.toContain("'");
expect(escaped.substr(1, -1)).not.toContain('"');
expect(response).toMatch('<img data-attr="ponys" data-reactroot=""/>');
});

escaped = quoteAttributeValueForBrowser('&');
expect(escaped).toBe('"&amp;"');
it('script tag is escaped inside attributes', () => {
var response = ReactDOMServer.renderToString(
<img data-attr={'<script type=\'\' src=""></script>'} />,
);
expect(response).toMatch(
'<img data-attr="&lt;script type=&#x27;&#x27; ' +
'src=&quot;&quot;&gt;&lt;/script&gt;" ' +
'data-reactroot=""/>',
);
});
});
16 changes: 0 additions & 16 deletions packages/react-dom/src/client/setTextContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/

import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment';

import setInnerHTML from './setInnerHTML';
import escapeTextContentForBrowser from '../shared/escapeTextContentForBrowser';
import {TEXT_NODE} from '../shared/HTMLNodeType';

/**
Expand Down Expand Up @@ -37,16 +33,4 @@ let setTextContent = function(node, text) {
node.textContent = text;
};

if (ExecutionEnvironment.canUseDOM) {
if (!('textContent' in document.documentElement)) {
setTextContent = function(node, text) {
if (node.nodeType === TEXT_NODE) {
node.nodeValue = text;
return;
}
setInnerHTML(node, escapeTextContentForBrowser(text));
};
}
}

export default setTextContent;
2 changes: 1 addition & 1 deletion packages/react-dom/src/server/DOMMarkupOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
shouldAttributeAcceptBooleanValue,
shouldSetAttribute,
} from '../shared/DOMProperty';
import quoteAttributeValueForBrowser from '../shared/quoteAttributeValueForBrowser';
import quoteAttributeValueForBrowser from './quoteAttributeValueForBrowser';
import warning from 'fbjs/lib/warning';

// isAttributeNameSafe() is currently duplicated in DOMPropertyOperations.
Expand Down
10 changes: 5 additions & 5 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
createMarkupForProperty,
createMarkupForRoot,
} from './DOMMarkupOperations';
import escapeTextForBrowser from './escapeTextForBrowser';
import {
Namespaces,
getIntrinsicNamespace,
Expand All @@ -34,7 +35,6 @@ import {
import ReactControlledValuePropTypes from '../shared/ReactControlledValuePropTypes';
import assertValidProps from '../shared/assertValidProps';
import dangerousStyleValue from '../shared/dangerousStyleValue';
import escapeTextContentForBrowser from '../shared/escapeTextContentForBrowser';
import isCustomComponent from '../shared/isCustomComponent';
import omittedCloseTags from '../shared/omittedCloseTags';
import warnValidStyle from '../shared/warnValidStyle';
Expand Down Expand Up @@ -204,7 +204,7 @@ function getNonChildrenInnerMarkup(props) {
} else {
var content = props.children;
if (typeof content === 'string' || typeof content === 'number') {
return escapeTextContentForBrowser(content);
return escapeTextForBrowser(content);
}
}
return null;
Expand Down Expand Up @@ -572,13 +572,13 @@ class ReactDOMServerRenderer {
return '';
}
if (this.makeStaticMarkup) {
return escapeTextContentForBrowser(text);
return escapeTextForBrowser(text);
}
if (this.previousWasTextNode) {
return '<!-- -->' + escapeTextContentForBrowser(text);
return '<!-- -->' + escapeTextForBrowser(text);
}
this.previousWasTextNode = true;
return escapeTextContentForBrowser(text);
return escapeTextForBrowser(text);
} else {
var nextChild;
({child: nextChild, context} = resolve(child, context));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@
var matchHtmlRegExp = /["'&<>]/;

/**
* Escape special characters in the given string of html.
* Escapes special characters and HTML entities in a given html string.
*
* @param {string} string The string to escape for inserting into HTML
* @param {string} string HTML string to escape for later insertion
* @return {string}
* @public
*/
Expand Down Expand Up @@ -98,7 +98,7 @@ function escapeHtml(string) {
* @param {*} text Text value to escape.
* @return {string} An escaped string.
*/
function escapeTextContentForBrowser(text) {
function escapeTextForBrowser(text) {
if (typeof text === 'boolean' || typeof text === 'number') {
// this shortcircuit helps perf for types that we know will never have
// special characters, especially given that this function is used often
Expand All @@ -108,4 +108,4 @@ function escapeTextContentForBrowser(text) {
return escapeHtml(text);
}

export default escapeTextContentForBrowser;
export default escapeTextForBrowser;
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import escapeTextContentForBrowser from './escapeTextContentForBrowser';
import escapeTextForBrowser from './escapeTextForBrowser';

/**
* Escapes attribute value to prevent scripting attacks.
Expand All @@ -14,7 +14,7 @@ import escapeTextContentForBrowser from './escapeTextContentForBrowser';
* @return {string} An escaped string.
*/
function quoteAttributeValueForBrowser(value) {
return '"' + escapeTextContentForBrowser(value) + '"';
return '"' + escapeTextForBrowser(value) + '"';
}

export default quoteAttributeValueForBrowser;

0 comments on commit e391819

Please sign in to comment.