Commit 2b0f7299 authored by Denys Mishunov's avatar Denys Mishunov

Fixed regression when URL was encoded in a loop

When a user would get to and from a file with white spaces, they would
generate the loop of encodings that would, in return, generate new
files.js. See https://gitlab.com/gitlab-org/gitlab/issues/207837 for
reference
parent a29c2a71
import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils'; import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
import { escapeFileUrl } from '~/lib/utils/url_utility';
import { decorateData, sortTree } from '../stores/utils'; import { decorateData, sortTree } from '../stores/utils';
export const splitParent = path => { export const splitParent = path => {
...@@ -48,7 +47,7 @@ export const decorateFiles = ({ ...@@ -48,7 +47,7 @@ export const decorateFiles = ({
id: path, id: path,
name, name,
path, path,
url: `/${projectId}/tree/${branchId}/-/${escapeFileUrl(path)}/`, url: `/${projectId}/tree/${branchId}/-/${path}/`,
type: 'tree', type: 'tree',
parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`, parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`,
tempFile, tempFile,
...@@ -85,7 +84,7 @@ export const decorateFiles = ({ ...@@ -85,7 +84,7 @@ export const decorateFiles = ({
id: path, id: path,
name, name,
path, path,
url: `/${projectId}/blob/${branchId}/-/${escapeFileUrl(path)}`, url: `/${projectId}/blob/${branchId}/-/${path}`,
type: 'blob', type: 'blob',
parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`, parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`,
tempFile, tempFile,
......
import { commitActionTypes, FILE_VIEW_MODE_EDITOR } from '../constants'; import { commitActionTypes, FILE_VIEW_MODE_EDITOR } from '../constants';
import { escapeFileUrl } from '~/lib/utils/url_utility';
export const dataStructure = () => ({ export const dataStructure = () => ({
id: '', id: '',
...@@ -220,9 +219,7 @@ export const mergeTrees = (fromTree, toTree) => { ...@@ -220,9 +219,7 @@ export const mergeTrees = (fromTree, toTree) => {
export const replaceFileUrl = (url, oldPath, newPath) => { export const replaceFileUrl = (url, oldPath, newPath) => {
// Add `/-/` so that we don't accidentally replace project path // Add `/-/` so that we don't accidentally replace project path
const result = url.replace(`/-/${escapeFileUrl(oldPath)}`, `/-/${escapeFileUrl(newPath)}`); return url.replace(`/-/${oldPath}`, `/-/${newPath}`);
return result;
}; };
export const swapInStateArray = (state, arr, key, entryPath) => export const swapInStateArray = (state, arr, key, entryPath) =>
......
<script> <script>
import FileHeader from '~/vue_shared/components/file_row_header.vue'; import FileHeader from '~/vue_shared/components/file_row_header.vue';
import FileIcon from '~/vue_shared/components/file_icon.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue';
import { escapeFileUrl } from '~/lib/utils/url_utility';
export default { export default {
name: 'FileRow', name: 'FileRow',
...@@ -94,7 +95,7 @@ export default { ...@@ -94,7 +95,7 @@ export default {
hasUrlAtCurrentRoute() { hasUrlAtCurrentRoute() {
if (!this.$router || !this.$router.currentRoute) return true; if (!this.$router || !this.$router.currentRoute) return true;
return this.$router.currentRoute.path === `/project${this.file.url}`; return this.$router.currentRoute.path === `/project${escapeFileUrl(this.file.url)}`;
}, },
}, },
}; };
......
---
title: Fixed regression when URL was encoded in a loop
merge_request: 25849
author:
type: fixed
import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils'; import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
import { decorateFiles, splitParent } from '~/ide/lib/files'; import { decorateFiles, splitParent } from '~/ide/lib/files';
import { decorateData } from '~/ide/stores/utils'; import { decorateData } from '~/ide/stores/utils';
import { escapeFileUrl } from '~/lib/utils/url_utility';
const TEST_BRANCH_ID = 'lorem-ipsum'; const TEST_BRANCH_ID = 'lorem-ipsum';
const TEST_PROJECT_ID = 10; const TEST_PROJECT_ID = 10;
...@@ -22,7 +21,7 @@ const createEntries = paths => { ...@@ -22,7 +21,7 @@ const createEntries = paths => {
id: path, id: path,
name, name,
path, path,
url: createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}/-/${escapeFileUrl(path)}`), url: createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}/-/${path}`),
type, type,
previewMode, previewMode,
binary: (previewMode && previewMode.binary) || false, binary: (previewMode && previewMode.binary) || false,
......
...@@ -494,7 +494,7 @@ describe('Multi-file store mutations', () => { ...@@ -494,7 +494,7 @@ describe('Multi-file store mutations', () => {
it('properly handles files with spaces in name', () => { it('properly handles files with spaces in name', () => {
const path = 'my fancy path'; const path = 'my fancy path';
const newPath = 'new path'; const newPath = 'new path';
const oldEntry = { ...file(path, path, 'blob'), url: `project/-/${encodeURI(path)}` }; const oldEntry = { ...file(path, path, 'blob'), url: `project/-/${path}` };
localState.entries[path] = oldEntry; localState.entries[path] = oldEntry;
...@@ -510,12 +510,12 @@ describe('Multi-file store mutations', () => { ...@@ -510,12 +510,12 @@ describe('Multi-file store mutations', () => {
id: newPath, id: newPath,
path: newPath, path: newPath,
name: newPath, name: newPath,
url: `project/-/new%20path`, url: `project/-/new path`,
key: expect.stringMatching(newPath), key: expect.stringMatching(newPath),
prevId: path, prevId: path,
prevName: path, prevName: path,
prevPath: path, prevPath: path,
prevUrl: `project/-/my%20fancy%20path`, prevUrl: `project/-/my fancy path`,
prevKey: oldEntry.key, prevKey: oldEntry.key,
prevParentPath: oldEntry.parentPath, prevParentPath: oldEntry.parentPath,
}); });
......
import { file } from 'jest/ide/helpers'; import { file } from 'jest/ide/helpers';
import FileRow from '~/vue_shared/components/file_row.vue'; import FileRow from '~/vue_shared/components/file_row.vue';
import { mount } from '@vue/test-utils'; import FileHeader from '~/vue_shared/components/file_row_header.vue';
import { shallowMount } from '@vue/test-utils';
import { nextTick } from 'vue';
import { escapeFileUrl } from '~/lib/utils/url_utility';
describe('File row component', () => { describe('File row component', () => {
let wrapper; let wrapper;
function createComponent(propsData) { function createComponent(propsData, $router = undefined) {
wrapper = mount(FileRow, { wrapper = shallowMount(FileRow, {
propsData, propsData,
mocks: {
$router,
},
}); });
} }
...@@ -61,7 +67,7 @@ describe('File row component', () => { ...@@ -61,7 +67,7 @@ describe('File row component', () => {
}), }),
}); });
return wrapper.vm.$nextTick().then(() => { return nextTick().then(() => {
expect(wrapper.vm.scrollIntoView).toHaveBeenCalled(); expect(wrapper.vm.scrollIntoView).toHaveBeenCalled();
}); });
}); });
...@@ -85,6 +91,27 @@ describe('File row component', () => { ...@@ -85,6 +91,27 @@ describe('File row component', () => {
level: 0, level: 0,
}); });
expect(wrapper.element.classList).toContain('js-file-row-header'); expect(wrapper.contains(FileHeader)).toBe(true);
});
it('matches the current route against encoded file URL', () => {
const fileName = 'with space';
const rowFile = Object.assign({}, file(fileName), {
url: `/${fileName}`,
});
const routerPath = `/project/${escapeFileUrl(fileName)}`;
createComponent(
{
file: rowFile,
level: 0,
},
{
currentRoute: {
path: routerPath,
},
},
);
expect(wrapper.vm.hasUrlAtCurrentRoute()).toBe(true);
}); });
}); });
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