diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-12-10 11:57:59 +0000 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2018-12-17 16:11:37 +0100 |
commit | 71e9ad1c2782220746cb0c788cf05e6f3fc91091 (patch) | |
tree | c71c7c811311993930563cce121a13c43f520a4a | |
parent | 65663f695ab2cc28a390024090ae8c5587bd2fc9 (diff) | |
download | gitlab-ce-71e9ad1c2782220746cb0c788cf05e6f3fc91091.tar.gz |
Merge branch 'sh-reject-refs-with-invalid-chars' into 'master'
Check for valid refs in CommitController before doing anything
Closes gitaly#1425
See merge request gitlab-org/gitlab-ce!23680
-rw-r--r-- | app/controllers/concerns/renders_commits.rb | 6 | ||||
-rw-r--r-- | app/controllers/projects/commits_controller.rb | 5 | ||||
-rw-r--r-- | app/controllers/projects/compare_controller.rb | 6 | ||||
-rw-r--r-- | spec/controllers/projects/commits_controller_spec.rb | 30 |
4 files changed, 41 insertions, 6 deletions
diff --git a/app/controllers/concerns/renders_commits.rb b/app/controllers/concerns/renders_commits.rb index f48e0586211..ed9b898a2a3 100644 --- a/app/controllers/concerns/renders_commits.rb +++ b/app/controllers/concerns/renders_commits.rb @@ -26,4 +26,10 @@ module RendersCommits commits end + + def valid_ref?(ref_name) + return true unless ref_name.present? + + Gitlab::GitRefValidator.validate(ref_name) + end end diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index e40a1a1d744..2510a31c9b3 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -11,6 +11,7 @@ class Projects::CommitsController < Projects::ApplicationController before_action :require_non_empty_project before_action :assign_ref_vars, except: :commits_root before_action :authorize_download_code! + before_action :validate_ref!, except: :commits_root before_action :set_commits, except: :commits_root def commits_root @@ -54,6 +55,10 @@ class Projects::CommitsController < Projects::ApplicationController private + def validate_ref! + render_404 unless valid_ref?(@ref) + end + def set_commits render_404 unless @path.empty? || request.format == :atom || @repository.blob_at(@commit.id, @path) || @repository.tree(@commit.id, @path).entries.present? @limit, @offset = (params[:limit] || 40).to_i, (params[:offset] || 0).to_i diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 2917925947f..5586c2fc631 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -65,12 +65,6 @@ class Projects::CompareController < Projects::ApplicationController private - def valid_ref?(ref_name) - return true unless ref_name.present? - - Gitlab::GitRefValidator.validate(ref_name) - end - def validate_refs! valid = [head_ref, start_ref].map { |ref| valid_ref?(ref) } diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb index 5c72dab698c..80513650636 100644 --- a/spec/controllers/projects/commits_controller_spec.rb +++ b/spec/controllers/projects/commits_controller_spec.rb @@ -53,6 +53,12 @@ describe Projects::CommitsController do it { is_expected.to respond_with(:not_found) } end + + context "branch with invalid format, valid file" do + let(:id) { 'branch with space/README.md' } + + it { is_expected.to respond_with(:not_found) } + end end context "when the ref name ends in .atom" do @@ -94,6 +100,30 @@ describe Projects::CommitsController do end end end + + describe "GET /commits/:id/signatures" do + render_views + + before do + get(:signatures, + namespace_id: project.namespace, + project_id: project, + id: id, + format: :json) + end + + context "valid branch" do + let(:id) { 'master' } + + it { is_expected.to respond_with(:success) } + end + + context "invalid branch format" do + let(:id) { 'some branch' } + + it { is_expected.to respond_with(:not_found) } + end + end end context 'token authentication' do |