Only set innerHTML if formatter returns object with html property. (#…

…1422)

Avoid potential XSS attack by default.

* Only set innerHTML if formatter returns object with html property.
* Add and update tests and documentation for column formatter

Co-authored-by: Mangala Sadhu Sangeet Singh Khalsa <[email protected]>

Grid.js

@@ -233,8 +233,13 @@ define([
 				// Support formatter, with or without formatterScope
 				var formatter = this.formatter,
 					formatterScope = this.grid.formatterScope;
-				td.innerHTML = typeof formatter === 'string' && formatterScope ?
+				var formattedValue = typeof formatter === 'string' && formatterScope ?
 					formatterScope[formatter](value, object) : this.formatter(value, object);
+				if (formattedValue != null && formattedValue.hasOwnProperty('html')) {
+					td.innerHTML = formattedValue.html;
+				} else if (formattedValue != null) {
+					td.appendChild(document.createTextNode(formattedValue));
+				}
 			}
 			else if (value != null) {
 				td.appendChild(document.createTextNode(value));

doc/components/core-components/Grid.md

@@ -200,7 +200,7 @@ Property | Description
 `sortable` | Indicates whether or not the grid should allow sorting by values in this field, by clicking on the column's header cell. Defaults to `true`. Note that it is always possible to programmatically sort a Grid by a given field by calling `set("sort", property, descending)` regardless of`sortable` status or even visible presence in the Grid altogether.
 `get(item)` | An optional function that, given a data item, will return the value to render in the cell.
 `set(item)` | An optional function that, given a modified data item, will return the value to set for the respective field on that item upon a call to `save()`. If no value is returned, the value as set in the passed item will be used.  (Modifying the passed item directly is thus also an option.)
-`formatter(value, object)` | An optional function that will return a string of HTML for rendering.  The function is passed the value that would normally be rendered, and the object from the collection.  If `formatterScope` is used, this can be a string instead of a function, in which case a function will be looked up on the `formatterScope` object using the given string. (Note: if a custom `renderCell` is specified, `formatter` will be ignored unless the custom `renderCell` accounts for it.)
+`formatter(value, object)` | An optional function that will return a string for rendering.  The function is passed the value that would normally be rendered, and the object from the collection. To render an HTML string, the function should return an object with an `html` property that is an HTML string, e.g. `{ html: '<b>formatted value</b>' }`.  If `formatterScope` is used, this can be a string instead of a function, in which case a function will be looked up on the `formatterScope` object using the given string. (Note: if a custom `renderCell` is specified, `formatter` will be ignored unless the custom `renderCell` accounts for it.)
 `renderCell(object, value, node)` | An optional function that will be called to render the value into the target cell. `object` refers to the record from the grid's collection for the row, and `value` refers to the specific value for the current cell (which may have been modified by the column definition's `get` function). `node` refers to the table cell that will be placed in the grid; if `renderCell` returns a node, that returned node will be placed inside the table cell. (Note: if a custom `renderCell` is specified, `formatter` will be ignored unless the custom `renderCell` accounts for it.)
 `renderHeaderCell(node)` | An optional function that will be called to render the column's header cell. Like `renderCell`, this may either operate on the node directly, or return a new node to be placed within it.
 

test/GridFromHtml.html

@@ -43,8 +43,9 @@
 			}
 
 			function testFormatter(item){
-				return "<h3>Step " + item.order + ": " + item.name + "</h3><p>" +
-					item.description + "</p>";
+				return {
+					html: '<h3>Step ' + item.order + ': ' + item.name + '</h3>' + '<p>' + item.description + '</p>'
+				}
 			}
 
 			function testGet(item){

test/Grid_rendering.html

@@ -43,8 +43,9 @@
 			}
 
 			function testFormatter(item){
-				return "<h3>Step " + item.order + ": " + item.name + "</h3><p>" +
-					item.description + "</p>";
+				return {
+					html: '<h3>Step ' + item.order + ': ' + item.name + '</h3><p>' + item.description + '</p>'
+				};
 			}
 
 			function testGet(item){
@@ -67,10 +68,12 @@
 
 			var testFormatterScope = {
 				foo: function(item){
-					return "<h3>" + this.word + " " + item.order + ": " + item.name + "</h3><p>" +
-						item.description + "</p>";
+					return {
+						html: '<h3>' + this.word + item.order + ': ' + item.name + '</h3>' +
+							'<p>' + item.description + '</p>'
+					};
 				},
-				word: "Step" // for testing proper execution context
+				word: 'Step ' // for testing proper execution context
 			};
 
 			var columnsLegacyFormatter = [

test/Tree.html

@@ -111,7 +111,9 @@
 								label: "Name",
 								field: "name",
 								formatter: function(value){
-									return "<strong>" + value + "</strong>";
+									return {
+										html: '<strong>' + value + '</strong>'
+									};
 								},
 								sortable: false
 							},

test/intern/core/columns.js

@@ -141,6 +141,42 @@ define([
 				assert.isFalse(warnCalled);
 			});
 		});
+
+		test.test('column formatter', function () {
+			var nameCells;
+			var orderCells;
+			var gridFormatter = {
+				formatName: function (value) {
+					return {
+						html: '<' + this.tagName + '>' + value + '</' + this.tagName + '>'
+					};
+				},
+				tagName: 'b'
+			};
+
+			grid = new Grid({
+				columns: {
+					order: {
+						label: 'Order',
+						formatter: function (value) {
+							return '<b>' + value + '</b>';
+						}
+					},
+					name: {
+						label: 'Name',
+						formatter: 'formatName'
+					}
+				},
+				formatterScope: gridFormatter
+			});
+			grid.startup();
+			grid.renderArray(orderedData.items);
+
+			orderCells = query('td.dgrid-cell.field-order', grid.domNode);
+			assert.strictEqual(orderCells[0].innerHTML, '&lt;b&gt;1&lt;/b&gt;');
+			nameCells = query('td.dgrid-cell.field-name', grid.domNode);
+			assert.strictEqual(nameCells[0].innerHTML, '<b>preheat</b>');
+		});
 	});
 
 	test.suite('columnSets', function () {