Fix broken file browsing with a submodule that has a relative link

Closes #775

3 files changed, 49 insertions, 6 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 0878c03207b..7e74c9b2535 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)