From f2747f1ce0e07c2b6f1d96ff104660575f835e67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=99=88=20=20jacopo=20beschi=20=F0=9F=99=89?= Date: Tue, 11 Sep 2018 22:02:09 +0000 Subject: Resolve "500 Internal Server Error: Cherrypick commit with empty branch name" --- .../unreleased/50677-fix-cherry-pick-branch-empty-name.yml | 5 +++++ lib/api/branches.rb | 10 +++++----- lib/api/commits.rb | 4 ++-- lib/api/files.rb | 10 +++++----- lib/api/pipeline_schedules.rb | 2 +- lib/api/triggers.rb | 2 +- spec/requests/api/commits_spec.rb | 8 ++++++++ 7 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 changelogs/unreleased/50677-fix-cherry-pick-branch-empty-name.yml 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' } -- cgit v1.2.1