Commit b0dbd651 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch 'rc/escape_dashboard_paths' into 'master'

Escape dashboard paths

Closes #206945

See merge request gitlab-org/gitlab!32714
parents 960be8a1 2a304b4c
......@@ -136,7 +136,7 @@ export default {
]),
selectDashboard(dashboard) {
const params = {
dashboard: dashboard.path,
dashboard: encodeURIComponent(dashboard.path),
};
redirectTo(mergeUrlParams(params, window.location.href));
},
......
......@@ -11,7 +11,8 @@ export default (props = {}) => {
const el = document.getElementById('prometheus-graphs');
if (el && el.dataset) {
const [currentDashboard] = getParameterValues('dashboard');
const [encodedDashboard] = getParameterValues('dashboard');
const currentDashboard = encodedDashboard ? decodeURIComponent(encodedDashboard) : null;
const { metricsDashboardBasePath, ...dataset } = el.dataset;
const { initState, dataProps } = stateAndPropsFromDataset({ currentDashboard, ...dataset });
......
......@@ -13,7 +13,7 @@ module MetricsDashboard
result = dashboard_finder.find(
project_for_dashboard,
current_user,
metrics_dashboard_params.to_h.symbolize_keys
decoded_params
)
if result
......@@ -114,4 +114,14 @@ module MetricsDashboard
json: result.slice(:all_dashboards, :message, :status)
}
end
def decoded_params
params = metrics_dashboard_params
if params[:dashboard_path]
params[:dashboard_path] = CGI.unescape(params[:dashboard_path])
end
params
end
end
---
title: Allow special characters in dashboard path
merge_request: 32714
author:
type: fixed
......@@ -76,6 +76,22 @@ RSpec.describe MetricsDashboard do
end
end
context 'when dashboard path includes encoded characters' do
let(:params) { { dashboard_path: 'dashboard%26copy.yml' } }
before do
allow(controller)
.to receive(:metrics_dashboard_params)
.and_return(params)
end
it 'decodes dashboard path' do
expect(::Gitlab::Metrics::Dashboard::Finder).to receive(:find).with(anything, anything, hash_including(dashboard_path: 'dashboard&copy.yml'))
json_response
end
end
context 'when parameters are provided and the list of all dashboards is required' do
before do
allow(controller).to receive(:include_all_dashboards?).and_return(true)
......
......@@ -426,6 +426,32 @@ describe('Dashboard', () => {
);
});
});
describe('when custom dashboard is selected', () => {
const windowLocation = window.location;
const findDashboardDropdown = () => wrapper.find(DashboardHeader).find(DashboardsDropdown);
beforeEach(() => {
delete window.location;
window.location = { ...windowLocation, assign: jest.fn() };
createMountedWrapper();
return wrapper.vm.$nextTick();
});
afterEach(() => {
window.location = windowLocation;
});
it('encodes dashboard param', () => {
findDashboardDropdown().vm.$emit('selectDashboard', {
path: 'dashboard&copy.yml',
});
expect(window.location.assign).toHaveBeenCalledWith(
'http://localhost/?dashboard=dashboard%2526copy.yml',
);
});
});
});
describe('when all requests have been commited by the store', () => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment