From b1a648608ba1bf1ae1948f4ea9ecc23ddf41efc2 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 5 Feb 2016 16:59:18 +0100 Subject: [PATCH 1/7] test(table): rewrite invalid tests --- .../app/plugins/panel/table/specs/renderer_specs.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/public/app/plugins/panel/table/specs/renderer_specs.ts b/public/app/plugins/panel/table/specs/renderer_specs.ts index 8f551b5cb1d..0e7d1e921f4 100644 --- a/public/app/plugins/panel/table/specs/renderer_specs.ts +++ b/public/app/plugins/panel/table/specs/renderer_specs.ts @@ -67,11 +67,16 @@ describe('when rendering table', () => { }); it('colored cell should have style', () => { - var html = renderer.renderCell(2, 85); - expect(html).to.be('85.0'); + var html = renderer.renderCell(2, 85); + expect(html).to.be('85.0'); }); - it('unformated undefined should be rendered as -', () => { + it('unformated undefined should be rendered as string', () => { + var html = renderer.renderCell(3, 'value'); + expect(html).to.be('value'); + }); + + it('undefined value should render as -', () => { var html = renderer.renderCell(3, undefined); expect(html).to.be(''); }); From d750908e364ad2c1c4484c82bb63984619dcc794 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 5 Feb 2016 17:58:51 +0100 Subject: [PATCH 2/7] feat(table): escape html by default closes #3673 --- public/app/plugins/panel/table/editor.html | 6 +++ public/app/plugins/panel/table/editor.ts | 1 + public/app/plugins/panel/table/renderer.ts | 49 ++++++++++++++----- .../panel/table/specs/renderer_specs.ts | 27 ++++++++++ 4 files changed, 70 insertions(+), 13 deletions(-) diff --git a/public/app/plugins/panel/table/editor.html b/public/app/plugins/panel/table/editor.html index c9e0bad5d9a..9206795c0d8 100644 --- a/public/app/plugins/panel/table/editor.html +++ b/public/app/plugins/panel/table/editor.html @@ -103,6 +103,11 @@ +
@@ -158,6 +163,7 @@
+ diff --git a/public/app/plugins/panel/table/editor.ts b/public/app/plugins/panel/table/editor.ts index 65c8b51cab8..0a6f695d34b 100644 --- a/public/app/plugins/panel/table/editor.ts +++ b/public/app/plugins/panel/table/editor.ts @@ -112,6 +112,7 @@ export class TablePanelEditorCtrl { pattern: '/.*/', dateFormat: 'YYYY-MM-DD HH:mm:ss', thresholds: [], + escapeHtml: true }; this.panel.styles.push(angular.copy(columnStyleDefaults)); diff --git a/public/app/plugins/panel/table/renderer.ts b/public/app/plugins/panel/table/renderer.ts index 755c7262070..42b9aeeb2d2 100644 --- a/public/app/plugins/panel/table/renderer.ts +++ b/public/app/plugins/panel/table/renderer.ts @@ -4,6 +4,8 @@ import _ from 'lodash'; import moment from 'moment'; import kbn from 'app/core/utils/kbn'; + + export class TableRenderer { formaters: any[]; colorState: any; @@ -24,22 +26,27 @@ export class TableRenderer { return _.first(style.colors); } - defaultCellFormater(v) { - if (v === null || v === void 0) { - return ''; - } + defaultCellFormater(escapeHtml = true) { + return function(v) { + if (v === null || v === void 0 || v === undefined) { + return ''; + } - if (_.isArray(v)) { - v = v.join(', '); - } + if (_.isArray(v)) { + v = v.join(', '); + } - return v; + if (_.isString(v) && escapeHtml) { + v = encodeHtml(v); + } + + return v; + }; } - createColumnFormater(style) { if (!style) { - return this.defaultCellFormater; + return this.defaultCellFormater(); } if (style.type === 'date') { @@ -62,7 +69,7 @@ export class TableRenderer { } if (_.isString(v)) { - return v; + return encodeHtml(v); } if (style.colorMode) { @@ -73,7 +80,11 @@ export class TableRenderer { }; } - return this.defaultCellFormater; + if (style.type === 'string') { + return this.defaultCellFormater(style.escapeHtml); + } + + return this.defaultCellFormater(); } formatColumnValue(colIndex, value) { @@ -91,7 +102,7 @@ export class TableRenderer { } } - this.formaters[colIndex] = this.defaultCellFormater; + this.formaters[colIndex] = this.defaultCellFormater(); return this.formaters[colIndex](value); } @@ -142,3 +153,15 @@ export class TableRenderer { return html; } } + +function encodeHtml(unsafe) { + return unsafe.replace(/[&<>"']/g, function(m) { + return ({ + '&': '&', + '<': '<', + '>': '>', + '"': '"', + '\'': ''' + })[m]; + }); +} diff --git a/public/app/plugins/panel/table/specs/renderer_specs.ts b/public/app/plugins/panel/table/specs/renderer_specs.ts index 0e7d1e921f4..70dca26a0e8 100644 --- a/public/app/plugins/panel/table/specs/renderer_specs.ts +++ b/public/app/plugins/panel/table/specs/renderer_specs.ts @@ -11,6 +11,8 @@ describe('when rendering table', () => { {text: 'Value'}, {text: 'Colored'}, {text: 'Undefined'}, + {text: 'String'}, + {text: 'UnescapedString' } ]; var panel = { @@ -35,6 +37,16 @@ describe('when rendering table', () => { colorMode: 'value', thresholds: [50, 80], colors: ['green', 'orange', 'red'] + }, + { + pattern: 'String', + type: 'string', + escapeHtml: true, + }, + { + pattern: 'UnescapedString', + type: 'string', + escapeHtml: false, } ] }; @@ -76,6 +88,21 @@ describe('when rendering table', () => { expect(html).to.be('value'); }); + it('string style with escape html should return escaped html', () => { + var html = renderer.renderCell(4, "&breaking
the
row"); + expect(html).to.be('&breaking <br /> the <br /> row'); + }); + + it('undefined formater should return escaped html', () => { + var html = renderer.renderCell(4, "&breaking
the
row"); + expect(html).to.be('&breaking <br /> the <br /> row'); + }); + + it('string style with escape html false should return html', () => { + var html = renderer.renderCell(5, "&breaking
the
row"); + expect(html).to.be('&breaking
the
row'); + }); + it('undefined value should render as -', () => { var html = renderer.renderCell(3, undefined); expect(html).to.be(''); From 5775c0a341d301644b9d7c97ef52d0a82201d592 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 7 Feb 2016 17:08:48 +0100 Subject: [PATCH 3/7] feat(table): remove option to disable html encoding --- public/app/plugins/panel/table/renderer.ts | 37 +++++++------------ .../panel/table/specs/renderer_specs.ts | 9 +---- 2 files changed, 14 insertions(+), 32 deletions(-) diff --git a/public/app/plugins/panel/table/renderer.ts b/public/app/plugins/panel/table/renderer.ts index 42b9aeeb2d2..c39f1d42bcf 100644 --- a/public/app/plugins/panel/table/renderer.ts +++ b/public/app/plugins/panel/table/renderer.ts @@ -4,8 +4,6 @@ import _ from 'lodash'; import moment from 'moment'; import kbn from 'app/core/utils/kbn'; - - export class TableRenderer { formaters: any[]; colorState: any; @@ -26,27 +24,21 @@ export class TableRenderer { return _.first(style.colors); } - defaultCellFormater(escapeHtml = true) { - return function(v) { - if (v === null || v === void 0 || v === undefined) { - return ''; - } + defaultCellFormater(value) { + if (value === null || value === void 0 || value === undefined) { + return ''; + } - if (_.isArray(v)) { - v = v.join(', '); - } + if (_.isArray(value)) { + value = value.join(', '); + } - if (_.isString(v) && escapeHtml) { - v = encodeHtml(v); - } - - return v; - }; + return value; } createColumnFormater(style) { if (!style) { - return this.defaultCellFormater(); + return this.defaultCellFormater; } if (style.type === 'date') { @@ -69,7 +61,7 @@ export class TableRenderer { } if (_.isString(v)) { - return encodeHtml(v); + return v; } if (style.colorMode) { @@ -80,11 +72,7 @@ export class TableRenderer { }; } - if (style.type === 'string') { - return this.defaultCellFormater(style.escapeHtml); - } - - return this.defaultCellFormater(); + return this.defaultCellFormater; } formatColumnValue(colIndex, value) { @@ -102,12 +90,13 @@ export class TableRenderer { } } - this.formaters[colIndex] = this.defaultCellFormater(); + this.formaters[colIndex] = this.defaultCellFormater; return this.formaters[colIndex](value); } renderCell(columnIndex, value, addWidthHack = false) { value = this.formatColumnValue(columnIndex, value); + value = encodeHtml(value); var style = ''; if (this.colorState.cell) { style = ' style="background-color:' + this.colorState.cell + ';color: white"'; diff --git a/public/app/plugins/panel/table/specs/renderer_specs.ts b/public/app/plugins/panel/table/specs/renderer_specs.ts index 70dca26a0e8..e5c3d343435 100644 --- a/public/app/plugins/panel/table/specs/renderer_specs.ts +++ b/public/app/plugins/panel/table/specs/renderer_specs.ts @@ -41,12 +41,10 @@ describe('when rendering table', () => { { pattern: 'String', type: 'string', - escapeHtml: true, }, { pattern: 'UnescapedString', - type: 'string', - escapeHtml: false, + type: 'string' } ] }; @@ -98,11 +96,6 @@ describe('when rendering table', () => { expect(html).to.be('&breaking <br /> the <br /> row'); }); - it('string style with escape html false should return html', () => { - var html = renderer.renderCell(5, "&breaking
the
row"); - expect(html).to.be('&breaking
the
row'); - }); - it('undefined value should render as -', () => { var html = renderer.renderCell(3, undefined); expect(html).to.be(''); From 7d89cf228c578325b1b9f2f4fbf77a20fe7e9513 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 7 Feb 2016 20:17:41 +0100 Subject: [PATCH 4/7] fix(table): remove html for htmlencoding option --- public/app/plugins/panel/table/editor.html | 6 ---- public/app/plugins/panel/table/editor.ts | 1 - public/app/plugins/panel/table/renderer.ts | 34 +++++++++---------- .../panel/table/specs/renderer_specs.ts | 9 ++--- 4 files changed, 19 insertions(+), 31 deletions(-) diff --git a/public/app/plugins/panel/table/editor.html b/public/app/plugins/panel/table/editor.html index 9206795c0d8..c9e0bad5d9a 100644 --- a/public/app/plugins/panel/table/editor.html +++ b/public/app/plugins/panel/table/editor.html @@ -103,11 +103,6 @@ -
    -
  • - -
    • -
@@ -163,7 +158,6 @@
- diff --git a/public/app/plugins/panel/table/editor.ts b/public/app/plugins/panel/table/editor.ts index 0a6f695d34b..65c8b51cab8 100644 --- a/public/app/plugins/panel/table/editor.ts +++ b/public/app/plugins/panel/table/editor.ts @@ -112,7 +112,6 @@ export class TablePanelEditorCtrl { pattern: '/.*/', dateFormat: 'YYYY-MM-DD HH:mm:ss', thresholds: [], - escapeHtml: true }; this.panel.styles.push(angular.copy(columnStyleDefaults)); diff --git a/public/app/plugins/panel/table/renderer.ts b/public/app/plugins/panel/table/renderer.ts index c39f1d42bcf..2a022d578c9 100644 --- a/public/app/plugins/panel/table/renderer.ts +++ b/public/app/plugins/panel/table/renderer.ts @@ -24,16 +24,16 @@ export class TableRenderer { return _.first(style.colors); } - defaultCellFormater(value) { - if (value === null || value === void 0 || value === undefined) { + defaultCellFormater(v) { + if (v === null || v === void 0 || v === undefined) { return ''; } - if (_.isArray(value)) { - value = value.join(', '); + if (_.isArray(v)) { + v = v.join(', '); } - return value; + return v; } createColumnFormater(style) { @@ -96,7 +96,7 @@ export class TableRenderer { renderCell(columnIndex, value, addWidthHack = false) { value = this.formatColumnValue(columnIndex, value); - value = encodeHtml(value); + value = this.encodeHtml(value); var style = ''; if (this.colorState.cell) { style = ' style="background-color:' + this.colorState.cell + ';color: white"'; @@ -141,16 +141,16 @@ export class TableRenderer { return html; } -} -function encodeHtml(unsafe) { - return unsafe.replace(/[&<>"']/g, function(m) { - return ({ - '&': '&', - '<': '<', - '>': '>', - '"': '"', - '\'': ''' - })[m]; - }); + encodeHtml(unsafe) { + return unsafe.replace(/[&<>"']/g, function(m) { + return ({ + '&': '&', + '<': '<', + '>': '>', + '"': '"', + '\'': ''' + })[m]; + }); + } } diff --git a/public/app/plugins/panel/table/specs/renderer_specs.ts b/public/app/plugins/panel/table/specs/renderer_specs.ts index e5c3d343435..75a6b03af98 100644 --- a/public/app/plugins/panel/table/specs/renderer_specs.ts +++ b/public/app/plugins/panel/table/specs/renderer_specs.ts @@ -11,8 +11,7 @@ describe('when rendering table', () => { {text: 'Value'}, {text: 'Colored'}, {text: 'Undefined'}, - {text: 'String'}, - {text: 'UnescapedString' } + {text: 'String'} ]; var panel = { @@ -41,10 +40,6 @@ describe('when rendering table', () => { { pattern: 'String', type: 'string', - }, - { - pattern: 'UnescapedString', - type: 'string' } ] }; @@ -92,7 +87,7 @@ describe('when rendering table', () => { }); it('undefined formater should return escaped html', () => { - var html = renderer.renderCell(4, "&breaking
the
row"); + var html = renderer.renderCell(3, "&breaking
the
row"); expect(html).to.be('&breaking <br /> the <br /> row'); }); From ddb1d0c684f46332ddf07ae3cda62656df5c094e Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 9 Feb 2016 10:57:32 +0100 Subject: [PATCH 5/7] fix(table): escape / chars as well --- public/app/plugins/panel/table/renderer.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/public/app/plugins/panel/table/renderer.ts b/public/app/plugins/panel/table/renderer.ts index 2a022d578c9..68cb594ab5e 100644 --- a/public/app/plugins/panel/table/renderer.ts +++ b/public/app/plugins/panel/table/renderer.ts @@ -149,7 +149,8 @@ export class TableRenderer { '<': '<', '>': '>', '"': '"', - '\'': ''' + '\'': ''', + '/': '/' })[m]; }); } From 6ba5471bd4f21ac5086f7b5e89c11c0b47a681e3 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 9 Feb 2016 11:00:48 +0100 Subject: [PATCH 6/7] docs(changelog): add html encoding for table panel --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49bd5762c3d..0e0721e4182 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ * **snapshot**: Annotations are now included in snapshots, closes [#3635](https://github.com/grafana/grafana/issues/3635) * **Admin**: Admin can now have global overview of Grafana setup, closes [#3812](https://github.com/grafana/grafana/issues/3812) * **graph**: Right side legend height is now fixed at row height, closes [#1277](https://github.com/grafana/grafana/issues/1277) +* **Table**: All content in table panel is now html escaped, closes [#3673](https://github.com/grafana/grafana/issues/3673) ### Bug fixes * **Playlist**: Fix for memory leak when running a playlist, closes [#3794](https://github.com/grafana/grafana/pull/3794) From e7ff018487bb556f619990f414f5826645f57042 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 9 Feb 2016 11:05:11 +0100 Subject: [PATCH 7/7] feat(table): uses lodash to escape html --- public/app/plugins/panel/table/renderer.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/public/app/plugins/panel/table/renderer.ts b/public/app/plugins/panel/table/renderer.ts index 68cb594ab5e..0a306b103a4 100644 --- a/public/app/plugins/panel/table/renderer.ts +++ b/public/app/plugins/panel/table/renderer.ts @@ -96,7 +96,7 @@ export class TableRenderer { renderCell(columnIndex, value, addWidthHack = false) { value = this.formatColumnValue(columnIndex, value); - value = this.encodeHtml(value); + value = _.escape(value); var style = ''; if (this.colorState.cell) { style = ' style="background-color:' + this.colorState.cell + ';color: white"'; @@ -141,17 +141,4 @@ export class TableRenderer { return html; } - - encodeHtml(unsafe) { - return unsafe.replace(/[&<>"']/g, function(m) { - return ({ - '&': '&', - '<': '<', - '>': '>', - '"': '"', - '\'': ''', - '/': '/' - })[m]; - }); - } }