Skip to content

Commit

Permalink
Backport UI: Remove usage of htmlSafe (#20235) (#20262)
Browse files Browse the repository at this point in the history
* UI: Remove usage of htmlSafe (#20235)

* Fix test helper import
  • Loading branch information
hashishaw committed Apr 21, 2023
1 parent 18087ce commit 2e27394
Show file tree
Hide file tree
Showing 14 changed files with 72 additions and 19 deletions.
3 changes: 3 additions & 0 deletions changelog/20235.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: remove use of htmlSafe except when first sanitized
```
9 changes: 4 additions & 5 deletions ui/app/components/wizard-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { inject as service } from '@ember/service';
import Component from '@ember/component';
import { computed } from '@ember/object';
import { FEATURE_MACHINE_STEPS, INIT_STEPS } from 'vault/helpers/wizard-constants';
import { htmlSafe } from '@ember/template';

export default Component.extend({
wizard: service(),
Expand Down Expand Up @@ -84,25 +83,25 @@ export default Component.extend({
let bar = [];
if (this.currentTutorialProgress) {
bar.push({
style: htmlSafe(`width:${this.currentTutorialProgress.percentage}%;`),
style: `width:${this.currentTutorialProgress.percentage}%;`,
completed: false,
showIcon: true,
});
} else {
if (this.currentFeatureProgress) {
this.completedFeatures.forEach((feature) => {
bar.push({ style: htmlSafe('width:100%;'), completed: true, feature: feature, showIcon: true });
bar.push({ style: 'width:100%;', completed: true, feature: feature, showIcon: true });
});
this.wizard.featureList.forEach((feature) => {
if (feature === this.currentMachine) {
bar.push({
style: htmlSafe(`width:${this.currentFeatureProgress.percentage}%;`),
style: `width:${this.currentFeatureProgress.percentage}%;`,
completed: this.currentFeatureProgress.percentage == 100 ? true : false,
feature: feature,
showIcon: true,
});
} else {
bar.push({ style: htmlSafe('width:0%;'), completed: false, feature: feature, showIcon: true });
bar.push({ style: 'width:0%;', completed: false, feature: feature, showIcon: true });
}
});
}
Expand Down
5 changes: 2 additions & 3 deletions ui/app/components/wizard/features-selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { inject as service } from '@ember/service';
import Component from '@ember/component';
import { computed } from '@ember/object';
import { FEATURE_MACHINE_TIME } from 'vault/helpers/wizard-constants';
import { htmlSafe } from '@ember/template';

export default Component.extend({
wizard: service(),
Expand Down Expand Up @@ -50,10 +49,10 @@ export default Component.extend({
}),
selectProgress: computed('selectedFeatures', function () {
let bar = this.selectedFeatures.map((feature) => {
return { style: htmlSafe('width:0%;'), completed: false, showIcon: true, feature: feature };
return { style: 'width:0%;', completed: false, showIcon: true, feature: feature };
});
if (bar.length === 0) {
bar = [{ style: htmlSafe('width:0%;'), showIcon: false }];
bar = [{ style: 'width:0%;', showIcon: false }];
}
return bar;
}),
Expand Down
3 changes: 1 addition & 2 deletions ui/app/templates/components/diff-version-selector.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,5 @@
</Toolbar>

<div class="form-section visual-diff">
{{! template-lint-configure no-triple-curlies "warn" }}
<pre>{{{this.visualDiff}}}</pre>
<pre>{{sanitized-html this.visualDiff}}</pre>
</div>
2 changes: 1 addition & 1 deletion ui/app/templates/components/wizard-progress.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{{#each @progressBar as |bar|}}
<div class="feature-progress-container">
<span class="progress-bar">
<span class="feature-progress" style={{bar.style}} {{! template-lint-disable }}></span>
<span class="feature-progress" style={{sanitized-html bar.style}}></span>
</span>
{{#if bar.showIcon}}
<Icon @name="check-circle-fill" class="feature-check {{if bar.completed 'completed-check' 'incomplete-check'}}" />
Expand Down
3 changes: 1 addition & 2 deletions ui/lib/core/addon/components/confirm.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import Component from '@ember/component';
import { computed } from '@ember/object';
import { htmlSafe } from '@ember/template';
import layout from '../templates/components/confirm';
import { next } from '@ember/runloop';

Expand Down Expand Up @@ -29,7 +28,7 @@ export default Component.extend({
height: 0,
focusTrigger: null,
style: computed('height', function () {
return htmlSafe(`height: ${this.height}px`);
return `height: ${this.height}px`;
}),
wormholeReference: null,
wormholeId: computed('elementId', function () {
Expand Down
7 changes: 2 additions & 5 deletions ui/lib/core/addon/components/replication-dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import Component from '@ember/component';
import { computed } from '@ember/object';
import { clusterStates } from 'core/helpers/cluster-states';
import { capitalize } from '@ember/string';
import { htmlSafe } from '@ember/template';
import layout from '../templates/components/replication-dashboard';

/**
Expand Down Expand Up @@ -30,7 +29,7 @@ import layout from '../templates/components/replication-dashboard';
* @param {Boolean} [isSummaryDashboard=false] - Only true when the cluster is both a dr and performance primary. If true, replicationDetailsSummary is populated and used to pass through the cluster details.
* @param {Object} replicationDetailsSummary=null - An Ember data object computed off the Ember Model. It combines the Model.dr and Model.performance objects into one and contains details specific to the mode replication.
* @param {Object} replicationDetails=null - An Ember data object pulled from the Ember Model. It contains details specific to the whether the replication is dr or performance.
* @param {String} clusterMode=null - The cluster mode passed through to a table component.
* @param {String} clusterMode=null - The cluster mode passed through to a table component.
* @param {Object} reindexingDetails=null - An Ember data object used to show a reindexing progress bar.
*/

Expand Down Expand Up @@ -89,9 +88,7 @@ export default Component.extend({
}),
reindexMessage: computed('isSecondary', 'progressBar', function () {
if (!this.isSecondary) {
return htmlSafe(
'This can cause a delay depending on the size of the data store. You can <b>not</b> use Vault during this time.'
);
return 'This can cause a delay depending on the size of the data store. You can <b>not</b> use Vault during this time.';
}
return 'This can cause a delay depending on the size of the data store. You can use Vault during this time.';
}),
Expand Down
14 changes: 14 additions & 0 deletions ui/lib/core/addon/helpers/sanitized-html.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { helper } from '@ember/component/helper';
import { debug } from '@ember/debug';
import { htmlSafe } from '@ember/template';
import { sanitize } from 'dompurify';

export default helper(function sanitizedHtml([htmlString]) {
try {
return htmlSafe(sanitize(htmlString));
} catch (e) {
debug('Error sanitizing string', e);
// I couldn't get this to actually fail but as a fallback render the value as-is
return htmlString;
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
@title={{concat "Re-indexing in progress" this.reindexingStage}}
@type="info"
@progressBar={{this.progressBar}}
@message={{this.reindexMessage}}
@message={{sanitized-html this.reindexMessage}}
data-test-isReindexing
/>
</div>
Expand Down
1 change: 1 addition & 0 deletions ui/lib/core/app/helpers/sanitized-html.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from 'core/helpers/sanitized-html';
1 change: 1 addition & 0 deletions ui/lib/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"date-fns": "*",
"@icholy/duration": "*",
"base64-js": "*",
"dompurify": "*",
"ember-auto-import": "*",
"ember-basic-dropdown": "*",
"ember-cli-babel": "*",
Expand Down
1 change: 1 addition & 0 deletions ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
"date-fns-tz": "^1.2.2",
"deepmerge": "^4.0.0",
"doctoc": "^1.4.0",
"dompurify": "^3.0.2",
"ember-api-actions": "^0.2.8",
"ember-auto-import": "^1.12.0",
"ember-basic-dropdown": "4.0.3",
Expand Down
35 changes: 35 additions & 0 deletions ui/tests/integration/helpers/sanitized-html-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';

module('Integration | Helper | sanitized-html', function (hooks) {
setupRenderingTest(hooks);

test('it does not alter if string is safe', async function (assert) {
this.set('inputValue', 'height: 15.33px');

await render(hbs`{{sanitized-html this.inputValue}}`);
assert.dom(this.element).hasText('height: 15.33px');
});

test('it strips unsafe HTML before rendering safe HTML', async function (assert) {
this.set(
'inputValue',
'<main data-test-thing>This is something<script data-test-script>window.alert(`h4cK3d`)</script></main>'
);

await render(hbs`{{sanitized-html this.inputValue}}`);
assert.dom('[data-test-thing]').hasTagName('main');
assert.dom('[data-test-thing]').hasText('This is something', 'preserves non-problematic content');
assert.dom('[data-test-script]').doesNotExist('Script is stripped from render');
});

test('it does not invoke functions passed as value', async function (assert) {
this.set('inputValue', () => {
window.alert('h4cK3d');
});
await render(hbs`{{sanitized-html this.inputValue}}`);
assert.dom(this.element).hasText("() => { window.alert('h4cK3d'); }");
});
});
5 changes: 5 additions & 0 deletions ui/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7104,6 +7104,11 @@ domhandler@^2.3.0:
dependencies:
domelementtype "1"

dompurify@^3.0.2:
version "3.0.2"
resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-3.0.2.tgz#bc4c7c011c825e7704341a285461d8d407d9429a"
integrity sha512-B8c6JdiEpxAKnd8Dm++QQxJL4lfuc757scZtcapj6qjTjrQzyq5iAyznLKVvK+77eYNiFblHBlt7MM0fOeqoKw==

domutils@1.5.1:
version "1.5.1"
resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.5.1.tgz#dcd8488a26f563d61079e48c9f7b7e32373682cf"
Expand Down

0 comments on commit 2e27394

Please sign in to comment.