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

3 files changed, 11 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) }