Commit f3e44598 authored by Patrick Bajao's avatar Patrick Bajao

Do now unescape branch name when deleting branch

Previously, when we unescape the branch name passed by frontend
(e.g. `test%2ffeature`), it can match a branch matching the
unescaped name (e.g. `test/feature`).

In a case wherein both `test%2ffeature` and `test/feature` branch
exists, the previous behavior will lead to a bug wherein
`test/feature` will be deleted if `test%2ffeature` was the one
being deleted.

The fix is to remove the unescape code so we're just finding for
the branch to delete without unescaping the branch name.

Changelog: fixed
parent 91ebded4
......@@ -105,8 +105,7 @@ class Projects::BranchesController < Projects::ApplicationController
# rubocop: enable CodeReuse/ActiveRecord
def destroy
@branch_name = Addressable::URI.unescape(params[:id])
result = ::Branches::DeleteService.new(project, current_user).execute(@branch_name)
result = ::Branches::DeleteService.new(project, current_user).execute(params[:id])
respond_to do |format|
format.html do
......
......@@ -356,7 +356,7 @@ RSpec.describe Projects::BranchesController do
context "valid branch name with encoded slashes" do
let(:branch) { "improve%2Fawesome" }
it { expect(response).to have_gitlab_http_status(:ok) }
it { expect(response).to have_gitlab_http_status(:not_found) }
it { expect(response.body).to be_blank }
end
......@@ -396,10 +396,10 @@ RSpec.describe Projects::BranchesController do
let(:branch) { 'improve%2Fawesome' }
it 'returns JSON response with message' do
expect(json_response).to eql('message' => 'Branch was deleted')
expect(json_response).to eql('message' => 'No such branch')
end
it { expect(response).to have_gitlab_http_status(:ok) }
it { expect(response).to have_gitlab_http_status(:not_found) }
end
context 'invalid branch name, valid ref' do
......
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