summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2015-04-10 11:51:24 +0000
committerDouwe Maan <douwe@gitlab.com>2015-04-10 11:51:24 +0000
commit6331759735074e2b14d398dfa1804cfa1f241d49 (patch)
treea7155181e56614daa890fa245f76e00a4df753ac
parent149195eded675717e6c5c1e3a8b1dae2d4082ae0 (diff)
parent7d089701f1a9c15117e3e2d6f21eabaf08659ff6 (diff)
downloadgitlab-ce-6331759735074e2b14d398dfa1804cfa1f241d49.tar.gz
Merge branch 'fix-submodules-with-relative-links' into 'master'
Fix broken file browsing with a submodule that has a relative link ## What does this MR do? This MR fixes an error that occurs when browsing a submodule with a relative link. ### Are there points in the code the reviewer needs to double check? I re-wrote the function because I was confused by how the first one was supposed to work. Please review if it's clearer. ### Why was this MR needed? A 500 Error would occur when using the file browser on a repo with a submodule. Here's how to reproduce the bug: 1. Start a new project in GitLab. 2. Clone git://git.gniibe.org/gnuk/gnuk.git/ locally. 3. Push repo to new project. 4. Click on "Files" in the project. The .gitmodules file: ``` [submodule "chopstx"] path = chopstx url = ../../chopstx/chopstx.git ``` ### What are the relevant issue numbers / [Feature requests](http://feedback.gitlab.com/)? 1. #775 2. #1385 3. https://github.com/gitlabhq/gitlabhq/issues/8153 4. https://github.com/gitlabhq/gitlabhq/issues/8881 5. https://github.com/gitlabhq/gitlabhq/issues/7554 See merge request !508
-rw-r--r--CHANGELOG1
-rw-r--r--app/helpers/submodule_helper.rb19
-rw-r--r--spec/helpers/submodule_helper_spec.rb35
3 files changed, 49 insertions, 6 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 7901b8be732..38a4117eb80 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
Please view this file on the master branch, on stable branches it's out of date.
v 7.10.0 (unreleased)
+ - Fix broken file browsing with a submodule that contains a relative link (Stan Hu)
- Fix bug where Wiki pages that included a '/' were no longer accessible (Stan Hu)
- Fix bug where error messages from Dropzone would not be displayed on the issues page (Stan Hu)
- Add ability to configure Reply-To address in gitlab.yml (Stan Hu)
diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb
index 241462e5e4c..99231084cfe 100644
--- a/app/helpers/submodule_helper.rb
+++ b/app/helpers/submodule_helper.rb
@@ -53,15 +53,22 @@ module SubmoduleHelper
end
def relative_self_links(url, commit)
- if url.scan(/(\.\.\/)/).size == 2
- base = url[/([^\/]*\/[^\/]*)\.git/, 1]
- else
- base = [ @project.group.path, '/', url[/([^\/]*)\.git/, 1] ].join('')
+ # Map relative links to a namespace and project
+ # For example:
+ # ../bar.git -> same namespace, repo bar
+ # ../foo/bar.git -> namespace foo, repo bar
+ # ../../foo/bar/baz.git -> namespace bar, repo baz
+ components = url.split('/')
+ base = components.pop.gsub(/.git$/, '')
+ namespace = components.pop.gsub(/^\.\.$/, '')
+
+ if namespace.empty?
+ namespace = @project.group.path
end
[
- namespace_project_path(base.namespace, base),
- namespace_project_tree_path(base.namespace, base, commit)
+ namespace_project_path(namespace, base),
+ namespace_project_tree_path(namespace, base, commit)
]
end
end
diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb
index aef1108e333..e99c3f5bc11 100644
--- a/spec/helpers/submodule_helper_spec.rb
+++ b/spec/helpers/submodule_helper_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe SubmoduleHelper do
+ include RepoHelpers
+
describe 'submodule links' do
let(:submodule_item) { double(id: 'hash', path: 'rack') }
let(:config) { Gitlab.config.gitlab }
@@ -111,6 +113,39 @@ describe SubmoduleHelper do
expect(submodule_links(submodule_item)).to eq([ repo.submodule_url_for, nil ])
end
end
+
+ context 'submodules with relative links' do
+ let(:group) { create(:group) }
+ let(:project) { create(:project, group: group) }
+
+ before do
+ self.instance_variable_set(:@project, project)
+ end
+
+ it 'one level down' do
+ commit_id = sample_commit[:id]
+ result = relative_self_links('../test.git', commit_id)
+ expect(result).to eq(["/#{group.path}/test", "/#{group.path}/test/tree/#{commit_id}"])
+ end
+
+ it 'two levels down' do
+ commit_id = sample_commit[:id]
+ result = relative_self_links('../../test.git', commit_id)
+ expect(result).to eq(["/#{group.path}/test", "/#{group.path}/test/tree/#{commit_id}"])
+ end
+
+ it 'one level down with namespace and repo' do
+ commit_id = sample_commit[:id]
+ result = relative_self_links('../foobar/test.git', commit_id)
+ expect(result).to eq(["/foobar/test", "/foobar/test/tree/#{commit_id}"])
+ end
+
+ it 'two levels down with namespace and repo' do
+ commit_id = sample_commit[:id]
+ result = relative_self_links('../foobar/baz/test.git', commit_id)
+ expect(result).to eq(["/baz/test", "/baz/test/tree/#{commit_id}"])
+ end
+ end
end
def stub_url(url)