[bugfix] Use CSSOM for style manipulations (#601)

* [bugfix] Use CSSOM for style manipulations Currently we assign the style property directly when manipulating CSS. This is an XSS vector, and prevents security conscious users from using a CSP that prevents this type of manipulation. We can avoid it by assigning style properties directly on the `element.style` object, which filters the text safely (as per CSSOM). fixes #566 * fix prettier * fix api wrappers
Addepar · Aug 2, 2018 · aeceb6c · aeceb6c
1 parent 9a36ddb
commit aeceb6c
Show file tree

Hide file tree

Showing 12 changed files with 172 additions and 131 deletions.
diff --git a/addon/-private/column-tree.js b/addon/-private/column-tree.js
@@ -68,16 +68,29 @@ class TableColumnMeta extends EmberObject {
   // meta object. This is set to the default width.
   _width = DEFAULT_COLUMN_WIDTH;
 
-  @readOnly('_node.isLeaf') isLeaf;
-  @readOnly('_node.isFixed') isFixed;
+  @readOnly('_node.isLeaf')
+  isLeaf;
 
-  @readOnly('_node.isSortable') isSortable;
-  @readOnly('_node.isResizable') isResizable;
-  @readOnly('_node.isReorderable') isReorderable;
+  @readOnly('_node.isFixed')
+  isFixed;
 
-  @readOnly('_node.width') width;
-  @readOnly('_node.offsetLeft') offsetLeft;
-  @readOnly('_node.offsetRight') offsetRight;
+  @readOnly('_node.isSortable')
+  isSortable;
+
+  @readOnly('_node.isResizable')
+  isResizable;
+
+  @readOnly('_node.isReorderable')
+  isReorderable;
+
+  @readOnly('_node.width')
+  width;
+
+  @readOnly('_node.offsetLeft')
+  offsetLeft;
+
+  @readOnly('_node.offsetRight')
+  offsetRight;
 
   @computed('isLeaf', '_node.{depth,tree.root.maxChildDepth}')
   get rowSpan() {

diff --git a/addon/components/-private/base-table-cell.js b/addon/components/-private/base-table-cell.js
@@ -0,0 +1,47 @@
+import Component from '@ember/component';
+import { equal } from '@ember/object/computed';
+import { observer } from '@ember/object';
+import { scheduleOnce } from '@ember/runloop';
+
+export default Component.extend({
+  // Provided by subclasses
+  columnMeta: null,
+
+  classNameBindings: ['isFirstColumn', 'isFixedLeft', 'isFixedRight'],
+
+  isFirstColumn: equal('columnMeta.index', 0),
+  isFixedLeft: equal('columnMeta.isFixed', 'left'),
+  isFixedRight: equal('columnMeta.isFixed', 'right'),
+
+  // eslint-disable-next-line
+  scheduleUpdateStyles: observer(
+    'columnMeta.{width,offsetLeft,offsetRight}',
+    'isFixedLeft',
+    'isFixedRight',
+
+    function() {
+      scheduleOnce('actions', this, 'updateStyles');
+    }
+  ),
+
+  updateStyles() {
+    if (typeof FastBoot === 'undefined' && this.element) {
+      let width = `${this.get('columnMeta.width')}px`;
+
+      this.element.style.width = width;
+      this.element.style.minWidth = width;
+      this.element.style.maxWidth = width;
+
+      if (this.get('isFixedLeft')) {
+        this.element.style.left = `${Math.round(this.get('columnMeta.offsetLeft'))}px`;
+      } else if (this.get('isFixedRight')) {
+        this.element.style.right = `${Math.round(this.get('columnMeta.offsetRight'))}px`;
+      }
+    }
+  },
+
+  didInsertElement() {
+    this._super(...arguments);
+    this.updateStyles();
+  },
+});
diff --git a/addon/components/-private/row-wrapper.js b/addon/components/-private/row-wrapper.js
@@ -29,21 +29,32 @@ class CellWrapper extends EmberObject {
   }
 }
 
+const layout = hbs`{{yield api}}`;
+
 @tagName('')
 export default class RowWrapper extends Component {
-  layout = hbs`
-    {{yield api}}
-  `;
+  layout = layout;
+
+  @argument
+  rowValue;
+
+  @argument
+  columns;
+
+  @argument
+  columnMetaCache;
+
+  @argument
+  rowMetaCache;
 
-  @argument rowValue;
-  @argument columns;
+  @argument
+  canSelect;
 
-  @argument columnMetaCache;
-  @argument rowMetaCache;
+  @argument
+  rowSelectionMode;
 
-  @argument canSelect;
-  @argument rowSelectionMode;
-  @argument checkboxSelectionMode;
+  @argument
+  checkboxSelectionMode;
 
   _cells = emberA([]);
 

diff --git a/addon/components/-private/simple-checkbox.js b/addon/components/-private/simple-checkbox.js
@@ -8,7 +8,8 @@ import { Action } from '@ember-decorators/argument/types';
 export default class SimpleCheckbox extends Component {
   value = null;
 
-  @attribute type = 'checkbox';
+  @attribute
+  type = 'checkbox';
 
   @argument({ defaultIfUndefined: true })
   @type('boolean')

diff --git a/addon/components/ember-table/component.js b/addon/components/ember-table/component.js
@@ -39,9 +39,11 @@ import layout from './template';
 export default class EmberTable extends Component {
   layout = layout;
 
-  @service userAgent;
+  @service
+  userAgent;
 
-  @attribute('data-test-ember-table') dataTestEmberTable = true;
+  @attribute('data-test-ember-table')
+  dataTestEmberTable = true;
 
   didInsertElement() {
     super.didInsertElement(...arguments);

diff --git a/addon/components/ember-tbody/component.js b/addon/components/ember-tbody/component.js
@@ -3,7 +3,7 @@ import { isArray } from '@ember/array';
 
 import { tagName } from '@ember-decorators/component';
 import { computed } from '@ember-decorators/object';
-import { bool, readOnly } from '@ember-decorators/object/computed';
+import { bool, readOnly, or } from '@ember-decorators/object/computed';
 
 import { argument } from '@ember-decorators/argument';
 import { required } from '@ember-decorators/argument/validation';
@@ -49,13 +49,14 @@ export default class EmberTBody extends Component {
   @type('object')
   api;
 
-  @computed('api.api')
-  get unwrappedApi() {
-    return this.get('api.api') || this.get('api');
-  }
+  @or('api.api', 'api')
+  unwrappedApi;
+
+  @readOnly('unwrappedApi.columnTree.leaves')
+  columns;
 
-  @readOnly('unwrappedApi.columnTree.leaves') columns;
-  @readOnly('unwrappedApi.columnTree.columnMetaCache') columnMetaCache;
+  @readOnly('unwrappedApi.columnTree.columnMetaCache')
+  columnMetaCache;
 
   /**
     Sets which row selection behavior to follow. Possible values are 'none'
@@ -211,7 +212,8 @@ export default class EmberTBody extends Component {
     Whether or not the table can select, is true if an `onSelect` action was
     passed to the table.
   */
-  @bool('onSelect') canSelect;
+  @bool('onSelect')
+  canSelect;
 
   constructor() {
     super(...arguments);

diff --git a/addon/components/ember-td/component.js b/addon/components/ember-td/component.js
@@ -1,9 +1,8 @@
-import Component from '@ember/component';
-import { htmlSafe } from '@ember/string';
+import BaseTableCell from '../-private/base-table-cell';
 
 import { action, computed } from '@ember-decorators/object';
-import { alias, readOnly, equal } from '@ember-decorators/object/computed';
-import { tagName, attribute, className } from '@ember-decorators/component';
+import { alias, readOnly } from '@ember-decorators/object/computed';
+import { tagName } from '@ember-decorators/component';
 import { argument } from '@ember-decorators/argument';
 import { type, optional } from '@ember-decorators/argument/type';
 import { Action } from '@ember-decorators/argument/types';
@@ -39,7 +38,7 @@ import { SELECT_MODE } from '../../-private/collapse-tree';
   @yield {object} rowMeta - The meta object associated with the row
 */
 @tagName('td')
-export default class EmberTd extends Component {
+export default class EmberTd extends BaseTableCell {
   layout = layout;
 
   /**
@@ -63,36 +62,37 @@ export default class EmberTd extends Component {
   @type(optional(Action))
   onDoubleClick;
 
-  @computed('api.api')
+  @computed('api') // only watch `api` due to a bug in Ember
   get unwrappedApi() {
     return this.get('api.api') || this.get('api');
   }
 
-  @alias('unwrappedApi.cellValue') cellValue;
-  @readOnly('unwrappedApi.cellMeta') cellMeta;
+  @alias('unwrappedApi.cellValue')
+  cellValue;
 
-  @readOnly('unwrappedApi.columnValue') columnValue;
-  @readOnly('unwrappedApi.columnMeta') columnMeta;
+  @readOnly('unwrappedApi.cellMeta')
+  cellMeta;
 
-  @readOnly('unwrappedApi.rowValue') rowValue;
-  @readOnly('unwrappedApi.rowMeta') rowMeta;
+  @readOnly('unwrappedApi.columnValue')
+  columnValue;
 
-  @readOnly('unwrappedApi.rowSelectionMode') rowSelectionMode;
-  @readOnly('unwrappedApi.checkboxSelectionMode') checkboxSelectionMode;
+  @readOnly('unwrappedApi.columnMeta')
+  columnMeta;
 
-  @className
-  @equal('columnMeta.index', 0)
-  isFirstColumn;
+  @readOnly('unwrappedApi.rowValue')
+  rowValue;
 
-  @className
-  @equal('columnMeta.isFixed', 'left')
-  isFixedLeft;
+  @readOnly('unwrappedApi.rowMeta')
+  rowMeta;
 
-  @className
-  @equal('columnMeta.isFixed', 'right')
-  isFixedRight;
+  @readOnly('unwrappedApi.rowSelectionMode')
+  rowSelectionMode;
 
-  @readOnly('rowMeta.canCollapse') canCollapse;
+  @readOnly('unwrappedApi.checkboxSelectionMode')
+  checkboxSelectionMode;
+
+  @readOnly('rowMeta.canCollapse')
+  canCollapse;
 
   @computed('rowMeta.depth')
   get depthClass() {
@@ -120,28 +120,6 @@ export default class EmberTd extends Component {
     );
   }
 
-  @attribute
-  @computed('columnMeta.{width,offsetLeft,offsetRight}', 'isFixed')
-  get style() {
-    let width = this.get('columnMeta.width');
-
-    let style = `width: ${width}px; min-width: ${width}px; max-width: ${width}px;`;
-
-    if (this.get('isFixedLeft')) {
-      style += `left: ${Math.round(this.get('columnMeta.offsetLeft'))}px;`;
-    } else if (this.get('isFixedRight')) {
-      style += `right: ${Math.round(this.get('columnMeta.offsetRight'))}px;`;
-    }
-
-    if (typeof FastBoot === 'undefined' && this.element) {
-      // Keep any styling added by the Sticky polyfill
-      style += `position: ${this.element.style.position};`;
-      style += `top: ${this.element.style.top};`;
-    }
-
-    return htmlSafe(style);
-  }
-
   @action
   onSelectionToggled(event) {
     let rowMeta = this.get('rowMeta');

diff --git a/addon/components/ember-th/component.js b/addon/components/ember-th/component.js
@@ -1,10 +1,9 @@
 /* global Hammer */
-import Component from '@ember/component';
-import { htmlSafe } from '@ember/string';
+import BaseTableCell from '../-private/base-table-cell';
 import { next } from '@ember/runloop';
 
-import { action, computed } from '@ember-decorators/object';
-import { readOnly, equal } from '@ember-decorators/object/computed';
+import { action } from '@ember-decorators/object';
+import { readOnly } from '@ember-decorators/object/computed';
 import { attribute, className, tagName } from '@ember-decorators/component';
 import { argument } from '@ember-decorators/argument';
 import { required } from '@ember-decorators/argument/validation';
@@ -40,7 +39,7 @@ const COLUMN_REORDERING = 2;
   @yield {object} columnMeta - The meta object associated with this column
 */
 @tagName('th')
-export default class EmberTh extends Component {
+export default class EmberTh extends BaseTableCell {
   layout = layout;
 
   /**
@@ -51,13 +50,17 @@ export default class EmberTh extends Component {
   @type('object')
   api;
 
-  @readOnly('api.columnValue') columnValue;
-  @readOnly('api.columnMeta') columnMeta;
+  @readOnly('api.columnValue')
+  columnValue;
+
+  @readOnly('api.columnMeta')
+  columnMeta;
 
   /**
     Any sorts applied to the table.
   */
-  @readOnly('api.sorts') sorts;
+  @readOnly('api.sorts')
+  sorts;
 
   /**
     Whether or not the column is sortable. Is true IFF the column is a leaf node
@@ -81,43 +84,17 @@ export default class EmberTh extends Component {
   @readOnly('columnMeta.isReorderable')
   isReorderable;
 
-  @readOnly('columnMeta.sortIndex') sortIndex;
-  @readOnly('columnMeta.isSorted') isSorted;
-  @readOnly('columnMeta.isMultiSorted') isMultiSorted;
-  @readOnly('columnMeta.isSortedAsc') isSortedAsc;
-
-  @equal('columnMeta.index', 0)
-  isFirstColumn;
-
-  @className
-  @equal('columnMeta.isFixed', 'left')
-  isFixedLeft;
-
-  @className
-  @equal('columnMeta.isFixed', 'right')
-  isFixedRight;
+  @readOnly('columnMeta.sortIndex')
+  sortIndex;
 
-  @attribute
-  @computed('columnMeta.{width,offsetLeft,offsetRight}', 'isFixed')
-  get style() {
-    let width = this.get('columnMeta.width');
+  @readOnly('columnMeta.isSorted')
+  isSorted;
 
-    let style = `width: ${width}px; min-width: ${width}px; max-width: ${width}px;`;
+  @readOnly('columnMeta.isMultiSorted')
+  isMultiSorted;
 
-    if (this.get('isFixedLeft')) {
-      style += `left: ${this.get('columnMeta.offsetLeft')}px;`;
-    } else if (this.get('isFixedRight')) {
-      style += `right: ${this.get('columnMeta.offsetRight')}px;`;
-    }
-
-    if (typeof FastBoot === 'undefined' && this.element) {
-      // Keep any styling added by the Sticky polyfill
-      style += `position: ${this.element.style.position};`;
-      style += `top: ${this.element.style.top};`;
-    }
-
-    return htmlSafe(style);
-  }
+  @readOnly('columnMeta.isSortedAsc')
+  isSortedAsc;
 
   @attribute('colspan')
   @readOnly('columnMeta.columnSpan')