Don't add a trailing slash in group redirects
Because we ignored the format, a request to `/groups/foo/labels.json` would redirect to `/groups/foo/-/labels/.json`. But really, it's worse than that, because unless the request contained a trailing slash, we shouldn't add one. Now, we only _keep_ a trailing slash, but don't _add_ one.
This commit is contained in:
parent
c5720382e6
commit
9f921b73f2
|
@ -46,10 +46,10 @@ module Gitlab
|
|||
# Only replace the last occurence of `path`.
|
||||
#
|
||||
# `request.fullpath` includes the querystring
|
||||
path = request.path.sub(%r{/#{path}/*(?!.*#{path})}, "/-/#{path}/")
|
||||
path << "?#{request.query_string}" if request.query_string.present?
|
||||
new_path = request.path.sub(%r{/#{path}(/*)(?!.*#{path})}, "/-/#{path}\\1")
|
||||
new_path << "?#{request.query_string}" if request.query_string.present?
|
||||
|
||||
path
|
||||
new_path
|
||||
end
|
||||
|
||||
paths.each do |path|
|
||||
|
|
|
@ -39,13 +39,19 @@ describe "Groups", "routing" do
|
|||
|
||||
describe 'legacy redirection' do
|
||||
describe 'labels' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels", "/groups/complex.group-namegit/-/labels/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels", "/groups/complex.group-namegit/-/labels" do
|
||||
let(:resource) { create(:group, parent: group, path: 'labels') }
|
||||
end
|
||||
|
||||
context 'when requesting JSON' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels.json", "/groups/complex.group-namegit/-/labels.json" do
|
||||
let(:resource) { create(:group, parent: group, path: 'labels') }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'group_members' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/group_members", "/groups/complex.group-namegit/-/group_members/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/group_members", "/groups/complex.group-namegit/-/group_members" do
|
||||
let(:resource) { create(:group, parent: group, path: 'group_members') }
|
||||
end
|
||||
end
|
||||
|
@ -60,7 +66,7 @@ describe "Groups", "routing" do
|
|||
end
|
||||
|
||||
describe 'milestones' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones", "/groups/complex.group-namegit/-/milestones/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones", "/groups/complex.group-namegit/-/milestones" do
|
||||
let(:resource) { create(:group, parent: group, path: 'milestones') }
|
||||
end
|
||||
|
||||
|
@ -76,18 +82,18 @@ describe "Groups", "routing" do
|
|||
end
|
||||
|
||||
context 'with a query string' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?hello=world", "/groups/complex.group-namegit/-/milestones/?hello=world" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?hello=world", "/groups/complex.group-namegit/-/milestones?hello=world" do
|
||||
let(:resource) { create(:group, parent: group, path: 'milestones') }
|
||||
end
|
||||
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?milestones=/milestones", "/groups/complex.group-namegit/-/milestones/?milestones=/milestones" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?milestones=/milestones", "/groups/complex.group-namegit/-/milestones?milestones=/milestones" do
|
||||
let(:resource) { create(:group, parent: group, path: 'milestones') }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'edit' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/edit", "/groups/complex.group-namegit/-/edit/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/edit", "/groups/complex.group-namegit/-/edit" do
|
||||
let(:resource) do
|
||||
pending('still rejected because of the wildcard reserved word')
|
||||
create(:group, parent: group, path: 'edit')
|
||||
|
@ -96,29 +102,29 @@ describe "Groups", "routing" do
|
|||
end
|
||||
|
||||
describe 'issues' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/issues", "/groups/complex.group-namegit/-/issues/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/issues", "/groups/complex.group-namegit/-/issues" do
|
||||
let(:resource) { create(:group, parent: group, path: 'issues') }
|
||||
end
|
||||
end
|
||||
|
||||
describe 'merge_requests' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/merge_requests", "/groups/complex.group-namegit/-/merge_requests/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/merge_requests", "/groups/complex.group-namegit/-/merge_requests" do
|
||||
let(:resource) { create(:group, parent: group, path: 'merge_requests') }
|
||||
end
|
||||
end
|
||||
|
||||
describe 'projects' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/projects", "/groups/complex.group-namegit/-/projects/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/projects", "/groups/complex.group-namegit/-/projects" do
|
||||
let(:resource) { create(:group, parent: group, path: 'projects') }
|
||||
end
|
||||
end
|
||||
|
||||
describe 'activity' do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/activity", "/groups/complex.group-namegit/-/activity/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/activity", "/groups/complex.group-namegit/-/activity" do
|
||||
let(:resource) { create(:group, parent: group, path: 'activity') }
|
||||
end
|
||||
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/activity/activity", "/groups/activity/-/activity/" do
|
||||
it_behaves_like 'redirecting a legacy path', "/groups/activity/activity", "/groups/activity/-/activity" do
|
||||
let!(:parent) { create(:group, path: 'activity') }
|
||||
let(:resource) { create(:group, parent: parent, path: 'activity') }
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue