Skip to content

Commit

Permalink
Better method of ensuring that args are not mutated by plugin initialize
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Guy Boertje committed Jan 14, 2016
1 parent 2952abe commit 681719b
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 8 deletions.
4 changes: 2 additions & 2 deletions logstash-core/lib/logstash/codecs/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module LogStash::Codecs; class Base < LogStash::Plugin

def initialize(params={})
super
config_init(params)
config_init(@params)
register if respond_to?(:register)
end

Expand All @@ -27,7 +27,7 @@ def encode(event)
raise "#{self.class}#encode must be overidden"
end # def encode

public
public
def close; end;

# @param block [Proc(event, data)] the callback proc passing the original event and the encoded event
Expand Down
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/filters/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class LogStash::Filters::Base < LogStash::Plugin
public
def initialize(params)
super
config_init(params)
config_init(@params)
@threadsafe = true
end # def initialize

Expand Down
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/inputs/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def initialize(params={})
super
@threadable = false
@stop_called = Concurrent::AtomicBoolean.new(false)
config_init(params)
config_init(@params)
@tags ||= []
end # def initialize

Expand Down
4 changes: 2 additions & 2 deletions logstash-core/lib/logstash/outputs/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def workers_not_supported(message=nil)
public
def initialize(params={})
super
config_init(params)
config_init(@params)

# 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
Expand Down Expand Up @@ -88,4 +88,4 @@ def output?(event)
# TODO: noop for now, remove this once we delete this call from all plugins
true
end # def output?
end # class LogStash::Outputs::Base
end # class LogStash::Outputs::Base
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def eql?(other)

public
def initialize(params=nil)
@params = params
@params = LogStash::Util.deep_clone(params)
@logger = Cabin::Channel.get(LogStash)
end

Expand Down
17 changes: 17 additions & 0 deletions logstash-core/lib/logstash/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,21 @@ def self.stringify_symbols(o)
o
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
31 changes: 30 additions & 1 deletion logstash-core/spec/logstash/plugin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class LogStash::Filters::MyTestFilter < LogStash::Filters::Base

subject.validate({})
end


it 'logs a warning if the plugin use the milestone option' do
expect_any_instance_of(Cabin::Channel).to receive(:warn)
Expand All @@ -120,4 +120,33 @@ class LogStash::Filters::Stromae < LogStash::Filters::Base
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

0 comments on commit 681719b

Please sign in to comment.