Better method of ensuring that args are not mutated by plugin initialize

add test for subclasses of codec, filter, input and output

Register codec clone, use String clone not dup so frozen strings stay so

Fixes #4444

Fixes #4473
This commit is contained in:
Guy Boertje 2016-01-13 14:44:20 +00:00
parent 2952abedc6
commit 681719bd9a
7 changed files with 54 additions and 8 deletions

View file

@ -11,7 +11,7 @@ module LogStash::Codecs; class Base < LogStash::Plugin
def initialize(params={}) def initialize(params={})
super super
config_init(params) config_init(@params)
register if respond_to?(:register) register if respond_to?(:register)
end end
@ -27,7 +27,7 @@ module LogStash::Codecs; class Base < LogStash::Plugin
raise "#{self.class}#encode must be overidden" raise "#{self.class}#encode must be overidden"
end # def encode end # def encode
public public
def close; end; def close; end;
# @param block [Proc(event, data)] the callback proc passing the original event and the encoded event # @param block [Proc(event, data)] the callback proc passing the original event and the encoded event

View file

@ -120,7 +120,7 @@ class LogStash::Filters::Base < LogStash::Plugin
public public
def initialize(params) def initialize(params)
super super
config_init(params) config_init(@params)
@threadsafe = true @threadsafe = true
end # def initialize end # def initialize

View file

@ -53,7 +53,7 @@ class LogStash::Inputs::Base < LogStash::Plugin
super super
@threadable = false @threadable = false
@stop_called = Concurrent::AtomicBoolean.new(false) @stop_called = Concurrent::AtomicBoolean.new(false)
config_init(params) config_init(@params)
@tags ||= [] @tags ||= []
end # def initialize end # def initialize

View file

@ -60,7 +60,7 @@ class LogStash::Outputs::Base < LogStash::Plugin
public public
def initialize(params={}) def initialize(params={})
super super
config_init(params) config_init(@params)
# If we're running with a single thread we must enforce single-threaded concurrency by default # If we're running with a single thread we must enforce single-threaded concurrency by default
# Maybe in a future version we'll assume output plugins are threadsafe # Maybe in a future version we'll assume output plugins are threadsafe
@ -88,4 +88,4 @@ class LogStash::Outputs::Base < LogStash::Plugin
# TODO: noop for now, remove this once we delete this call from all plugins # TODO: noop for now, remove this once we delete this call from all plugins
true true
end # def output? end # def output?
end # class LogStash::Outputs::Base end # class LogStash::Outputs::Base

View file

@ -24,7 +24,7 @@ class LogStash::Plugin
public public
def initialize(params=nil) def initialize(params=nil)
@params = params @params = LogStash::Util.deep_clone(params)
@logger = Cabin::Channel.get(LogStash) @logger = Cabin::Channel.get(LogStash)
end end

View file

@ -183,4 +183,21 @@ module LogStash::Util
o o
end end
end end
def self.deep_clone(o)
case o
when Hash
o.inject({}) {|h, (k,v)| h[k] = deep_clone(v); h }
when Array
o.map {|v| deep_clone(v) }
when Fixnum, Symbol, IO, TrueClass, FalseClass, NilClass
o
when LogStash::Codecs::Base
o.clone.tap {|c| c.register }
when String
o.clone #need to keep internal state e.g. frozen
else
Marshal.load(Marshal.dump(o))
end
end
end # module LogStash::Util end # module LogStash::Util

View file

@ -108,7 +108,7 @@ describe LogStash::Plugin do
subject.validate({}) subject.validate({})
end end
it 'logs a warning if the plugin use the milestone option' do it 'logs a warning if the plugin use the milestone option' do
expect_any_instance_of(Cabin::Channel).to receive(:warn) expect_any_instance_of(Cabin::Channel).to receive(:warn)
@ -120,4 +120,33 @@ describe LogStash::Plugin do
end end
end end
end end
describe "subclass initialize" do
let(:args) { Hash.new }
[
StromaeCodec = Class.new(LogStash::Codecs::Base) do
config_name "stromae"
config :foo_tag, :validate => :string, :default => "bar"
end,
StromaeFilter = Class.new(LogStash::Filters::Base) do
config_name "stromae"
config :foo_tag, :validate => :string, :default => "bar"
end,
StromaeInput = Class.new(LogStash::Inputs::Base) do
config_name "stromae"
config :foo_tag, :validate => :string, :default => "bar"
end,
StromaeOutput = Class.new(LogStash::Outputs::Base) do
config_name "stromae"
config :foo_tag, :validate => :string, :default => "bar"
end
].each do |klass|
it "subclass #{klass.name} does not modify params" do
instance = klass.new(args)
expect(args).to be_empty
end
end
end
end end