From 177924625780cdf3a86814a5e1654f38b561a06e Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 4 Jan 2022 16:08:06 +0000 Subject: [PATCH] ui: Fix URL params decoding (#11931) * ui: Move wildcard param decoding to routlet service --- .changelog/11931.txt | 3 ++ ui/packages/consul-ui/app/routing/route.js | 30 +++++-------------- ui/packages/consul-ui/app/services/routlet.js | 29 +++++++++++++++--- .../consul-ui/app/templates/dc/kv/edit.hbs | 2 +- .../tests/acceptance/dc/kvs/edit.feature | 19 ++++++++++-- .../consul-ui/tests/pages/dc/kv/edit.js | 3 ++ 6 files changed, 56 insertions(+), 30 deletions(-) create mode 100644 .changelog/11931.txt diff --git a/.changelog/11931.txt b/.changelog/11931.txt new file mode 100644 index 000000000..b76c2849f --- /dev/null +++ b/.changelog/11931.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixes a bug with URL decoding within KV area +``` diff --git a/ui/packages/consul-ui/app/routing/route.js b/ui/packages/consul-ui/app/routing/route.js index 2eaabc836..2028c96df 100644 --- a/ui/packages/consul-ui/app/routing/route.js +++ b/ui/packages/consul-ui/app/routing/route.js @@ -2,17 +2,14 @@ import Route from '@ember/routing/route'; import { get, setProperties, action } from '@ember/object'; import { inject as service } from '@ember/service'; -// paramsFor import { routes } from 'consul-ui/router'; -import wildcard from 'consul-ui/utils/routing/wildcard'; - -const isWildcard = wildcard(routes); export default class BaseRoute extends Route { @service('container') container; @service('env') env; @service('repository/permission') permissions; @service('router') router; + @service('routlet') routlet; _setRouteName() { super._setRouteName(...arguments); @@ -114,27 +111,14 @@ export default class BaseRoute extends Route { } /** - * Adds urldecoding to any wildcard route `params` passed into ember `model` - * hooks, plus of course anywhere else where `paramsFor` is used. This means - * the entire ember app is now changed so that all paramsFor calls returns - * urldecoded params instead of raw ones. - * For example we use this largely for URLs for the KV store: - * /kv/*key > /ui/kv/%25-kv-name/%25-here > key = '%-kv-name/%-here' + * Normalizes any params passed into ember `model` hooks, plus of course + * anywhere else where `paramsFor` is used. This means the entire ember app + * is now changed so that all paramsFor calls returns normalized params + * instead of raw ones. For example we use this largely for URLs for the KV + * store: /kv/*key > /ui/kv/%25-kv-name/%25-here > key = '%-kv-name/%-here' */ paramsFor(name) { - const params = super.paramsFor(...arguments); - if (isWildcard(this.routeName)) { - return Object.keys(params).reduce(function(prev, item) { - if (typeof params[item] !== 'undefined') { - prev[item] = decodeURIComponent(params[item]); - } else { - prev[item] = params[item]; - } - return prev; - }, {}); - } else { - return params; - } + return this.routlet.normalizeParamsFor(this.routeName, super.paramsFor(...arguments)); } @action diff --git a/ui/packages/consul-ui/app/services/routlet.js b/ui/packages/consul-ui/app/services/routlet.js index a2a286f74..b743e51b6 100644 --- a/ui/packages/consul-ui/app/services/routlet.js +++ b/ui/packages/consul-ui/app/services/routlet.js @@ -1,6 +1,11 @@ import Service, { inject as service } from '@ember/service'; import { schedule } from '@ember/runloop'; +import wildcard from 'consul-ui/utils/routing/wildcard'; +import { routes } from 'consul-ui/router'; + +const isWildcard = wildcard(routes); + class Outlets { constructor() { this.map = new Map(); @@ -87,6 +92,24 @@ export default class RoutletService extends Service { return {}; } + /** + * Adds urldecoding to any wildcard route `params` + */ + normalizeParamsFor(name, params = {}) { + if (isWildcard(name)) { + return Object.keys(params).reduce(function(prev, item) { + if (typeof params[item] !== 'undefined') { + prev[item] = decodeURIComponent(params[item]); + } else { + prev[item] = params[item]; + } + return prev; + }, {}); + } else { + return params; + } + } + paramsFor(name) { let outletParams = {}; const outlet = outlets.get(name); @@ -102,16 +125,14 @@ export default class RoutletService extends Service { // of the specified params with the values specified let current = route; let parent; - let routeParams = { - ...current.params, - }; + let routeParams = this.normalizeParamsFor(name, current.params); // TODO: Not entirely sure whether we are ok exposing queryParams here // seeing as accessing them from here means you can get them but not set // them as yet // let queryParams = {}; while ((parent = current.parent)) { routeParams = { - ...parent.params, + ...this.normalizeParamsFor(parent.name, parent.params), ...routeParams, }; // queryParams = { diff --git a/ui/packages/consul-ui/app/templates/dc/kv/edit.hbs b/ui/packages/consul-ui/app/templates/dc/kv/edit.hbs index b84750d09..e441d26fd 100644 --- a/ui/packages/consul-ui/app/templates/dc/kv/edit.hbs +++ b/ui/packages/consul-ui/app/templates/dc/kv/edit.hbs @@ -83,7 +83,7 @@ as |parts|}} -

+

{{#if (and item.Key (not-eq item.Key parentKey))}} {{left-trim item.Key parentKey}} diff --git a/ui/packages/consul-ui/tests/acceptance/dc/kvs/edit.feature b/ui/packages/consul-ui/tests/acceptance/dc/kvs/edit.feature index 832c8209e..fd26b2169 100644 --- a/ui/packages/consul-ui/tests/acceptance/dc/kvs/edit.feature +++ b/ui/packages/consul-ui/tests/acceptance/dc/kvs/edit.feature @@ -1,6 +1,19 @@ @setupApplicationTest Feature: dc / kvs / edit: KV Viewing - Scenario: + Scenario: Viewing a KV with a URL unsafe character + Given 1 datacenter model with the value "datacenter" + And 1 kv model from yaml + --- + Key: "@key" + --- + When I visit the kv page for yaml + --- + dc: datacenter + kv: "@key" + --- + Then the url should be /datacenter/kv/%40key/edit + And I see Key on the kv like "@key" + Scenario: Viewing a Session attached to a KV Given 1 datacenter model with the value "datacenter" And 1 kv model from yaml --- @@ -14,7 +27,9 @@ Feature: dc / kvs / edit: KV Viewing --- Then the url should be /datacenter/kv/key/edit And I see ID on the session like "session-id" - Given 1 kv model from yaml + Scenario: Viewing a Session attached to a KV + Given 1 datacenter model with the value "datacenter" + And 1 kv model from yaml --- Key: another-key Session: ~ diff --git a/ui/packages/consul-ui/tests/pages/dc/kv/edit.js b/ui/packages/consul-ui/tests/pages/dc/kv/edit.js index bb577470d..e3fe94cf9 100644 --- a/ui/packages/consul-ui/tests/pages/dc/kv/edit.js +++ b/ui/packages/consul-ui/tests/pages/dc/kv/edit.js @@ -11,6 +11,9 @@ export default function(visitable, attribute, present, submitable, deletable, ca ...submitable({}, 'main'), ...cancelable(), ...deletable(), + kv: { + Key: attribute('data-test-kv-key', '[data-test-kv-key]') + }, session: { warning: present('[data-test-session-warning]'), ID: attribute('data-test-session', '[data-test-session]'),