Commit 8716823a authored by Denys Mishunov's avatar Denys Mishunov Committed by Martin Wortschack

Escape ref part of ide_edit_path

To make sure we link to WebIDE correctly, we escape the ref part
(the one containing the branch name). This allows branches to contain
"#" character without breaking WebIDE when getting there by clicking
the "WebIDE" buttons in product.
parent 6b95c3f9
import Vue from 'vue'; import Vue from 'vue';
import VueRouter from 'vue-router'; import IdeRouter from '~/ide/ide_router_extension';
import { joinPaths } from '~/lib/utils/url_utility'; import { joinPaths } from '~/lib/utils/url_utility';
import flash from '~/flash'; import flash from '~/flash';
import store from './stores'; import store from './stores';
import { __ } from '~/locale'; import { __ } from '~/locale';
Vue.use(VueRouter); Vue.use(IdeRouter);
/** /**
* Routes below /-/ide/: * Routes below /-/ide/:
...@@ -33,7 +33,7 @@ const EmptyRouterComponent = { ...@@ -33,7 +33,7 @@ const EmptyRouterComponent = {
}, },
}; };
const router = new VueRouter({ const router = new IdeRouter({
mode: 'history', mode: 'history',
base: joinPaths(gon.relative_url_root || '', '/-/ide/'), base: joinPaths(gon.relative_url_root || '', '/-/ide/'),
routes: [ routes: [
......
import VueRouter from 'vue-router';
import { escapeFileUrl } from '~/lib/utils/url_utility';
// To allow special characters (like "#," for example) in the branch names, we
// should encode all the locations before those get processed by History API.
// Otherwise, paths get messed up so that the router receives incorrect
// branchid. The only way to do it consistently and in a more or less
// future-proof manner is, unfortunately, to monkey-patch VueRouter or, as
// suggested here, achieve the same more reliably by subclassing VueRouter and
// update the methods, used in WebIDE.
//
// More context: https://gitlab.com/gitlab-org/gitlab/issues/35473
export default class IDERouter extends VueRouter {
push(location, onComplete, onAbort) {
super.push(escapeFileUrl(location), onComplete, onAbort);
}
resolve(to, current, append) {
return super.resolve(escapeFileUrl(to), current, append);
}
}
...@@ -194,12 +194,14 @@ export function redirectTo(url) { ...@@ -194,12 +194,14 @@ export function redirectTo(url) {
return window.location.assign(url); return window.location.assign(url);
} }
export const escapeFileUrl = fileUrl => encodeURIComponent(fileUrl).replace(/%2F/g, '/');
export function webIDEUrl(route = undefined) { export function webIDEUrl(route = undefined) {
let returnUrl = `${gon.relative_url_root || ''}/-/ide/`; let returnUrl = `${gon.relative_url_root || ''}/-/ide/`;
if (route) { if (route) {
returnUrl += `project${route.replace(new RegExp(`^${gon.relative_url_root || ''}`), '')}`; returnUrl += `project${route.replace(new RegExp(`^${gon.relative_url_root || ''}`), '')}`;
} }
return returnUrl; return escapeFileUrl(returnUrl);
} }
/** /**
...@@ -313,8 +315,6 @@ export const setUrlParams = (params, url = window.location.href, clearParams = f ...@@ -313,8 +315,6 @@ export const setUrlParams = (params, url = window.location.href, clearParams = f
return urlObj.toString(); return urlObj.toString();
}; };
export const escapeFileUrl = fileUrl => encodeURIComponent(fileUrl).replace(/%2F/g, '/');
export function urlIsDifferent(url, compare = String(window.location)) { export function urlIsDifferent(url, compare = String(window.location)) {
return url !== compare; return url !== compare;
} }
...@@ -27,7 +27,7 @@ module BlobHelper ...@@ -27,7 +27,7 @@ module BlobHelper
"#{current_user.namespace.full_path}/#{project.path}" "#{current_user.namespace.full_path}/#{project.path}"
end end
segments = [ide_path, 'project', project_path, 'edit', ref] segments = [ide_path, 'project', project_path, 'edit', encode_ide_path(ref)]
segments.concat(['-', encode_ide_path(path)]) if path.present? segments.concat(['-', encode_ide_path(path)]) if path.present?
File.join(segments) File.join(segments)
end end
......
---
title: 'WebIDE: Support # in branch names'
merge_request: 24717
author:
type: changed
import VueRouter from 'vue-router';
import IdeRouter from '~/ide/ide_router_extension';
jest.mock('vue-router');
describe('IDE overrides of VueRouter', () => {
const paths = branch => [
`${branch}`,
`/${branch}`,
`/${branch}/-/`,
`/edit/${branch}`,
`/edit/${branch}/-/`,
`/blob/${branch}`,
`/blob/${branch}/-/`,
`/blob/${branch}/-/src/merge_requests/2`,
`/blob/${branch}/-/src/blob/`,
`/tree/${branch}/-/src/blob/`,
`/tree/${branch}/-/src/tree/`,
];
let router;
beforeEach(() => {
VueRouter.mockClear();
router = new IdeRouter({
mode: 'history',
});
});
it.each`
path | expected
${'#-test'} | ${'%23-test'}
${'#test'} | ${'%23test'}
${'test#'} | ${'test%23'}
${'test-#'} | ${'test-%23'}
${'test-#-hash'} | ${'test-%23-hash'}
${'test/hash#123'} | ${'test/hash%23123'}
`('finds project path when route is $path', ({ path, expected }) => {
paths(path).forEach(route => {
const expectedPath = route.replace(path, expected);
router.push(route);
expect(VueRouter.prototype.push).toHaveBeenCalledWith(expectedPath, undefined, undefined);
router.resolve(route);
expect(VueRouter.prototype.resolve).toHaveBeenCalledWith(expectedPath, undefined, undefined);
});
});
});
...@@ -28,6 +28,12 @@ describe('URL utility', () => { ...@@ -28,6 +28,12 @@ describe('URL utility', () => {
gon.relative_url_root = ''; gon.relative_url_root = '';
}); });
it('escapes special characters', () => {
expect(urlUtils.webIDEUrl('/gitlab-org/gitlab-#-foss/merge_requests/1')).toBe(
'/-/ide/project/gitlab-org/gitlab-%23-foss/merge_requests/1',
);
});
describe('without relative_url_root', () => { describe('without relative_url_root', () => {
it('returns IDE path with route', () => { it('returns IDE path with route', () => {
expect(urlUtils.webIDEUrl('/gitlab-org/gitlab-foss/merge_requests/1')).toBe( expect(urlUtils.webIDEUrl('/gitlab-org/gitlab-foss/merge_requests/1')).toBe(
......
...@@ -244,8 +244,8 @@ describe BlobHelper do ...@@ -244,8 +244,8 @@ describe BlobHelper do
it 'escapes special characters' do it 'escapes special characters' do
Rails.application.routes.default_url_options[:script_name] = nil Rails.application.routes.default_url_options[:script_name] = nil
expect(helper.ide_edit_path(project, "testing/#hashes", "readme.md#test")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/testing/#hashes/-/readme.md%23test") expect(helper.ide_edit_path(project, "testing/#hashes", "readme.md#test")).to eq("/-/ide/project/#{project.full_path}/edit/testing/%23hashes/-/readme.md%23test")
expect(helper.ide_edit_path(project, "testing/#hashes", "src#/readme.md#test")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/testing/#hashes/-/src%23/readme.md%23test") expect(helper.ide_edit_path(project, "testing/#hashes", "src#/readme.md#test")).to eq("/-/ide/project/#{project.full_path}/edit/testing/%23hashes/-/src%23/readme.md%23test")
end end
it 'does not escape "/" character' do it 'does not escape "/" character' do
......
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