diff options
author | Alexis Reigel <alexis.reigel.ext@siemens.com> | 2017-12-04 12:51:39 +0100 |
---|---|---|
committer | Alexis Reigel <alexis.reigel.ext@siemens.com> | 2018-01-17 09:55:00 +0100 |
commit | a78abf3a0071d1705adc562f23be1f3a347c6db5 (patch) | |
tree | 553bac3b43e07507aabe3d833b988015e8749f85 | |
parent | ac92d70d9025e1c90bffa99c08bfc4cdb2fc36c9 (diff) | |
download | gitlab-ce-a78abf3a0071d1705adc562f23be1f3a347c6db5.tar.gz |
use safer .hooks_for instead of .public_send
with .public_send we can't make sure that the scope on the model
actually exists.
-rw-r--r-- | app/models/concerns/triggerable_hooks.rb | 6 | ||||
-rw-r--r-- | app/models/project.rb | 2 | ||||
-rw-r--r-- | app/services/system_hooks_service.rb | 2 | ||||
-rw-r--r-- | spec/models/concerns/triggerable_hooks_spec.rb | 18 |
4 files changed, 26 insertions, 2 deletions
diff --git a/app/models/concerns/triggerable_hooks.rb b/app/models/concerns/triggerable_hooks.rb index 6e02275d552..42f40c52d29 100644 --- a/app/models/concerns/triggerable_hooks.rb +++ b/app/models/concerns/triggerable_hooks.rb @@ -17,6 +17,12 @@ module TriggerableHooks class_methods do attr_reader :triggerable_hooks + def hooks_for(trigger) + return none unless self::TRIGGERS.keys.include?(trigger) + + public_send(trigger) # rubocop:disable GitlabSecurity/PublicSend + end + private def triggerable_hooks(hooks) diff --git a/app/models/project.rb b/app/models/project.rb index a8c634200ce..602c6a2795f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -970,7 +970,7 @@ class Project < ActiveRecord::Base def execute_hooks(data, hooks_scope = :push_hooks) run_after_commit_or_now do - hooks.public_send(hooks_scope).each do |hook| # rubocop:disable GitlabSecurity/PublicSend + hooks.hooks_for(hooks_scope).each do |hook| hook.async_execute(data, hooks_scope.to_s) end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 690918b4a00..68c4597a133 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -8,7 +8,7 @@ class SystemHooksService end def execute_hooks(data, hooks_scope = :all) - SystemHook.public_send(hooks_scope).find_each do |hook| # rubocop:disable GitlabSecurity/PublicSend + SystemHook.hooks_for(hooks_scope).find_each do |hook| hook.async_execute(data, 'system_hooks') end end diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb index 695df5e10aa..3ddf477e41e 100644 --- a/spec/models/concerns/triggerable_hooks_spec.rb +++ b/spec/models/concerns/triggerable_hooks_spec.rb @@ -14,4 +14,22 @@ RSpec.describe TriggerableHooks do expect(TestableHook).not_to respond_to :tag_push_hooks end end + + describe '.hooks_for' do + context 'the model has the required trigger scope' do + it 'returns the record' do + hook = TestableHook.create!(url: 'http://example.com', push_events: true) + + expect(TestableHook.hooks_for(:push_hooks)).to eq [hook] + end + end + + context 'the model does not have the required trigger scope' do + it 'returns an empty relation' do + TestableHook.create!(url: 'http://example.com', push_events: true) + + expect(TestableHook.hooks_for(:tag_push_hooks)).to eq [] + end + end + end end |