Commit 63317597 authored by Douwe Maan's avatar Douwe Maan

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
parents 149195ed 7d089701
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 7.10.0 (unreleased) 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 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) - 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) - Add ability to configure Reply-To address in gitlab.yml (Stan Hu)
......
...@@ -53,15 +53,22 @@ module SubmoduleHelper ...@@ -53,15 +53,22 @@ module SubmoduleHelper
end end
def relative_self_links(url, commit) def relative_self_links(url, commit)
if url.scan(/(\.\.\/)/).size == 2 # Map relative links to a namespace and project
base = url[/([^\/]*\/[^\/]*)\.git/, 1] # For example:
else # ../bar.git -> same namespace, repo bar
base = [ @project.group.path, '/', url[/([^\/]*)\.git/, 1] ].join('') # ../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 end
[ [
namespace_project_path(base.namespace, base), namespace_project_path(namespace, base),
namespace_project_tree_path(base.namespace, base, commit) namespace_project_tree_path(namespace, base, commit)
] ]
end end
end end
require 'spec_helper' require 'spec_helper'
describe SubmoduleHelper do describe SubmoduleHelper do
include RepoHelpers
describe 'submodule links' do describe 'submodule links' do
let(:submodule_item) { double(id: 'hash', path: 'rack') } let(:submodule_item) { double(id: 'hash', path: 'rack') }
let(:config) { Gitlab.config.gitlab } let(:config) { Gitlab.config.gitlab }
...@@ -111,6 +113,39 @@ describe SubmoduleHelper do ...@@ -111,6 +113,39 @@ describe SubmoduleHelper do
expect(submodule_links(submodule_item)).to eq([ repo.submodule_url_for, nil ]) expect(submodule_links(submodule_item)).to eq([ repo.submodule_url_for, nil ])
end end
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 end
def stub_url(url) def stub_url(url)
......
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