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

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

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 :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'
+        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

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])

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!

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'

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

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' }