Commit 12522d22 authored by Nick Thomas's avatar Nick Thomas

Merge branch 'feature/webide_escaping' into 'master'

Improve file handling of special characters in WebIDE

Closes #54376

See merge request gitlab-org/gitlab-ce!25727
parents 7700e029 ebbcc17f
import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils'; import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
import { decorateData, sortTree } from '../stores/utils'; import { decorateData, sortTree } from '../stores/utils';
export const escapeFileUrl = fileUrl => encodeURIComponent(fileUrl).replace(/%2F/g, '/');
export const splitParent = path => { export const splitParent = path => {
const idx = path.lastIndexOf('/'); const idx = path.lastIndexOf('/');
...@@ -45,7 +47,7 @@ export const decorateFiles = ({ ...@@ -45,7 +47,7 @@ export const decorateFiles = ({
id: path, id: path,
name, name,
path, path,
url: `/${projectId}/tree/${branchId}/-/${path}/`, url: `/${projectId}/tree/${branchId}/-/${escapeFileUrl(path)}/`,
type: 'tree', type: 'tree',
parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`, parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`,
tempFile, tempFile,
...@@ -81,7 +83,7 @@ export const decorateFiles = ({ ...@@ -81,7 +83,7 @@ export const decorateFiles = ({
id: path, id: path,
name, name,
path, path,
url: `/${projectId}/blob/${branchId}/-/${path}`, url: `/${projectId}/blob/${branchId}/-/${escapeFileUrl(path)}`,
type: 'blob', type: 'blob',
parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`, parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`,
tempFile, tempFile,
......
...@@ -19,10 +19,14 @@ module BlobHelper ...@@ -19,10 +19,14 @@ module BlobHelper
def ide_edit_path(project = @project, ref = @ref, path = @path, options = {}) def ide_edit_path(project = @project, ref = @ref, path = @path, options = {})
segments = [ide_path, 'project', project.full_path, 'edit', ref] segments = [ide_path, 'project', project.full_path, 'edit', ref]
segments.concat(['-', path]) if path.present? segments.concat(['-', encode_ide_path(path)]) if path.present?
File.join(segments) File.join(segments)
end end
def encode_ide_path(path)
url_encode(path).gsub('%2F', '/')
end
def edit_blob_button(project = @project, ref = @ref, path = @path, options = {}) def edit_blob_button(project = @project, ref = @ref, path = @path, options = {})
return unless blob = readable_blob(options, path, project, ref) return unless blob = readable_blob(options, path, project, ref)
......
---
title: Fixed bug with hashes in urls in WebIDE
merge_request: 54376
author: Kieran Andrews
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, escapeFileUrl } from '~/ide/lib/files';
import { decorateData } from '~/ide/stores/utils'; import { decorateData } from '~/ide/stores/utils';
const TEST_BRANCH_ID = 'lorem-ipsum'; const TEST_BRANCH_ID = 'lorem-ipsum';
...@@ -20,7 +20,7 @@ const createEntries = paths => { ...@@ -20,7 +20,7 @@ const createEntries = paths => {
id: path, id: path,
name, name,
path, path,
url: createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}/-/${path}`), url: createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}/-/${escapeFileUrl(path)}`),
type, type,
previewMode: viewerInformationForPath(path), previewMode: viewerInformationForPath(path),
parentPath: parent, parentPath: parent,
...@@ -28,7 +28,7 @@ const createEntries = paths => { ...@@ -28,7 +28,7 @@ const createEntries = paths => {
? parentEntry.url ? parentEntry.url
: createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}`), : createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}`),
}), }),
tree: children.map(childName => jasmine.objectContaining({ name: childName })), tree: children.map(childName => expect.objectContaining({ name: childName })),
}; };
return acc; return acc;
...@@ -36,10 +36,10 @@ const createEntries = paths => { ...@@ -36,10 +36,10 @@ const createEntries = paths => {
const entries = paths.reduce(createEntry, {}); const entries = paths.reduce(createEntry, {});
// Wrap entries in jasmine.objectContaining. // Wrap entries in expect.objectContaining.
// We couldn't do this earlier because we still need to select properties from parent entries. // We couldn't do this earlier because we still need to select properties from parent entries.
return Object.keys(entries).reduce((acc, key) => { return Object.keys(entries).reduce((acc, key) => {
acc[key] = jasmine.objectContaining(entries[key]); acc[key] = expect.objectContaining(entries[key]);
return acc; return acc;
}, {}); }, {});
...@@ -47,13 +47,14 @@ const createEntries = paths => { ...@@ -47,13 +47,14 @@ const createEntries = paths => {
describe('IDE lib decorate files', () => { describe('IDE lib decorate files', () => {
it('creates entries and treeList', () => { it('creates entries and treeList', () => {
const data = ['app/assets/apples/foo.js', 'app/bugs.js', 'README.md']; const data = ['app/assets/apples/foo.js', 'app/bugs.js', 'app/#weird#file?.txt', 'README.md'];
const expectedEntries = createEntries([ const expectedEntries = createEntries([
{ path: 'app', type: 'tree', children: ['assets', 'bugs.js'] }, { path: 'app', type: 'tree', children: ['assets', '#weird#file?.txt', 'bugs.js'] },
{ path: 'app/assets', type: 'tree', children: ['apples'] }, { path: 'app/assets', type: 'tree', children: ['apples'] },
{ path: 'app/assets/apples', type: 'tree', children: ['foo.js'] }, { path: 'app/assets/apples', type: 'tree', children: ['foo.js'] },
{ path: 'app/assets/apples/foo.js', type: 'blob', children: [] }, { path: 'app/assets/apples/foo.js', type: 'blob', children: [] },
{ path: 'app/bugs.js', type: 'blob', children: [] }, { path: 'app/bugs.js', type: 'blob', children: [] },
{ path: 'app/#weird#file?.txt', type: 'blob', children: [] },
{ path: 'README.md', type: 'blob', children: [] }, { path: 'README.md', type: 'blob', children: [] },
]); ]);
...@@ -64,7 +65,7 @@ describe('IDE lib decorate files', () => { ...@@ -64,7 +65,7 @@ describe('IDE lib decorate files', () => {
}); });
// Here we test the keys and then each key/value individually because `expect(entries).toEqual(expectedEntries)` // Here we test the keys and then each key/value individually because `expect(entries).toEqual(expectedEntries)`
// was taking a very long time for some reason. Probably due to large objects and nested `jasmine.objectContaining`. // was taking a very long time for some reason. Probably due to large objects and nested `expect.objectContaining`.
const entryKeys = Object.keys(entries); const entryKeys = Object.keys(entries);
expect(entryKeys).toEqual(Object.keys(expectedEntries)); expect(entryKeys).toEqual(Object.keys(expectedEntries));
......
...@@ -230,5 +230,18 @@ describe BlobHelper do ...@@ -230,5 +230,18 @@ describe BlobHelper do
expect(helper.ide_edit_path(project, "master", "")).to eq("/gitlab/-/ide/project/#{project.namespace.path}/#{project.path}/edit/master") expect(helper.ide_edit_path(project, "master", "")).to eq("/gitlab/-/ide/project/#{project.namespace.path}/#{project.path}/edit/master")
end end
it 'escapes special characters' do
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", "src#/readme.md#test")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/testing/#hashes/-/src%23/readme.md%23test")
end
it 'does not escape "/" character' do
Rails.application.routes.default_url_options[:script_name] = nil
expect(helper.ide_edit_path(project, "testing/slashes", "readme.md/")).to eq("/-/ide/project/#{project.namespace.path}/#{project.path}/edit/testing/slashes/-/readme.md/")
end
end end
end end
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