Resolve "500 Internal Server Error: Cherrypick commit with empty branch name"

7 files changed, 27 insertions, 14 deletions

diff --git a/changelogs/unreleased/50677-fix-cherry-pick-branch-empty-name.yml b/changelogs/unreleased/50677-fix-cherry-pick-branch-empty-name.yml
new file mode 100644
index 00000000000..88a2ab802c8
--- /dev/null
+++ b/changelogs/unreleased/50677-fix-cherry-pick-branch-empty-name.yml
@@ -0,0 +1,5 @@
+---
+title: Fixes 500 for cherry pick API with empty branch name
+merge_request: 21501
+author: Jacopo Beschi @jacopo-beschi
+type: fixed
diff --git a/lib/api/branches.rb b/lib/api/branches.rb
index 6c1d445e53d..ac33c2924ca 100644
--- a/lib/api/branches.rb
+++ b/lib/api/branches.rb
@@ -69,7 +69,7 @@ module API
         success Entities::Branch
       end
       params do
-        requires :branch, type: String, desc: 'The name of the branch'
+        requires :branch, type: String, desc: 'The name of the branch', allow_blank: false
         optional :developers_can_push, type: Boolean, desc: 'Flag if developers can push to that branch'
         optional :developers_can_merge, type: Boolean, desc: 'Flag if developers can merge to that branch'
       end
@@ -106,7 +106,7 @@ module API
         success Entities::Branch
       end
       params do
-        requires :branch, type: String, desc: 'The name of the branch'
+        requires :branch, type: String, desc: 'The name of the branch', allow_blank: false
       end
       put ':id/repository/branches/:branch/unprotect', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
         authorize_admin_project
@@ -122,8 +122,8 @@ module API
         success Entities::Branch
       end
       params do
-        requires :branch, type: String, desc: 'The name of the branch'
-        requires :ref, type: String, desc: 'Create branch from commit sha or existing branch'
+        requires :branch, type: String, desc: 'The name of the branch', allow_blank: false
+        requires :ref, type: String, desc: 'Create branch from commit sha or existing branch', allow_blank: false
       end
       post ':id/repository/branches' do
         authorize_push_project
@@ -143,7 +143,7 @@ module API
 
       desc 'Delete a branch'
       params do
-        requires :branch, type: String, desc: 'The name of the branch'
+        requires :branch, type: String, desc: 'The name of the branch', allow_blank: false
       end
       delete ':id/repository/branches/:branch', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
         authorize_push_project
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index 3e8de3c8dae..f86ac60b54b 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -71,7 +71,7 @@ module API
         detail 'This feature was introduced in GitLab 8.13'
       end
       params do
-        requires :branch, type: String, desc: 'Name of the branch to commit into. To create a new branch, also provide `start_branch`.'
+        requires :branch, type: String, desc: 'Name of the branch to commit into. To create a new branch, also provide `start_branch`.', allow_blank: false
         requires :commit_message, type: String, desc: 'Commit message'
         requires :actions, type: Array[Hash], desc: 'Actions to perform in commit'
         optional :start_branch, type: String, desc: 'Name of the branch to start the new commit from'
@@ -151,7 +151,7 @@ module API
       end
       params do
         requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag to be cherry picked'
-        requires :branch, type: String, desc: 'The name of the branch'
+        requires :branch, type: String, desc: 'The name of the branch', allow_blank: false
       end
       post ':id/repository/commits/:sha/cherry_pick', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do
         authorize_push_to_branch!(params[:branch])
diff --git a/lib/api/files.rb b/lib/api/files.rb
index ff4f75c12df..ac02488d30c 100644
--- a/lib/api/files.rb
+++ b/lib/api/files.rb
@@ -58,7 +58,7 @@ module API
 
       params :simple_file_params do
         requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
-        requires :branch, type: String, desc: 'Name of the branch to commit into. To create a new branch, also provide `start_branch`.'
+        requires :branch, type: String, desc: 'Name of the branch to commit into. To create a new branch, also provide `start_branch`.', allow_blank: false
         requires :commit_message, type: String, allow_blank: false, desc: 'Commit message'
         optional :start_branch, type: String, desc: 'Name of the branch to start the new commit from'
         optional :author_email, type: String, desc: 'The email of the author'
@@ -80,7 +80,7 @@ module API
       desc 'Get raw file metadata from repository'
       params do
         requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
-        requires :ref, type: String, desc: 'The name of branch, tag or commit'
+        requires :ref, type: String, desc: 'The name of branch, tag or commit', allow_blank: false
       end
       head ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS do
         assign_file_vars!
@@ -91,7 +91,7 @@ module API
       desc 'Get raw file contents from the repository'
       params do
         requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
-        requires :ref, type: String, desc: 'The name of branch, tag commit'
+        requires :ref, type: String, desc: 'The name of branch, tag commit', allow_blank: false
       end
       get ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS do
         assign_file_vars!
@@ -104,7 +104,7 @@ module API
       desc 'Get file metadata from repository'
       params do
         requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
-        requires :ref, type: String, desc: 'The name of branch, tag or commit'
+        requires :ref, type: String, desc: 'The name of branch, tag or commit', allow_blank: false
       end
       head ":id/repository/files/:file_path", requirements: FILE_ENDPOINT_REQUIREMENTS do
         assign_file_vars!
@@ -115,7 +115,7 @@ module API
       desc 'Get a file from the repository'
       params do
         requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
-        requires :ref, type: String, desc: 'The name of branch, tag or commit'
+        requires :ref, type: String, desc: 'The name of branch, tag or commit', allow_blank: false
       end
       get ":id/repository/files/:file_path", requirements: FILE_ENDPOINT_REQUIREMENTS do
         assign_file_vars!
diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb
index 37f32411296..ae4a7654ec1 100644
--- a/lib/api/pipeline_schedules.rb
+++ b/lib/api/pipeline_schedules.rb
@@ -39,7 +39,7 @@ module API
       end
       params do
         requires :description, type: String, desc: 'The description of pipeline schedule'
-        requires :ref, type: String, desc: 'The branch/tag name will be triggered'
+        requires :ref, type: String, desc: 'The branch/tag name will be triggered', allow_blank: false
         requires :cron, type: String, desc: 'The cron'
         optional :cron_timezone, type: String, default: 'UTC', desc: 'The timezone'
         optional :active, type: Boolean, default: true, desc: 'The activation of pipeline schedule'
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index b29e660c6e0..be95ef9e928 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -10,7 +10,7 @@ module API
         success Entities::Pipeline
       end
       params do
-        requires :ref, type: String, desc: 'The commit sha or name of a branch or tag'
+        requires :ref, type: String, desc: 'The commit sha or name of a branch or tag', allow_blank: false
         requires :token, type: String, desc: 'The unique token of trigger'
         optional :variables, type: Hash, desc: 'The list of variables to be injected into build'
       end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 246947e58a8..d5b31610dad 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -1040,6 +1040,14 @@ describe API::Commits do
         end
       end
 
+      context 'when branch is empty' do
+        ['', ' '].each do |branch|
+          it_behaves_like '400 response' do
+            let(:request) { post api(route, current_user), branch: branch }
+          end
+        end
+      end
+
       context 'when branch does not exist' do
         it_behaves_like '404 response' do
           let(:request) { post api(route, current_user), branch: 'foo' }