logstash-plugins / logstash-filter-kv

Apache License 2.0
17 stars 42 forks source link

one of the buggiest line of code ever #19

Closed colinsurprenant closed 8 years ago

colinsurprenant commented 9 years ago

Such code, many wow, much bug.

https://github.com/logstash-plugins/logstash-filter-kv/blob/407e3c545575758476c21474e6a2fc410d2ebaf8/lib/logstash/filters/kv.rb#L260

How many errors can you count in this single line of code?

if !event =~ /[@value_split]/

I count 4.

colinsurprenant commented 9 years ago

that being said it is somewhat harmless since it will always return nil. it currently serve no other purpose that wasting cpu cyles :P

I assume the original intent was to short-circuit the parsing if the source string did not contain any @value_split. in its current state the short circuit will never work and has the inverse effect of systematically adding useless processing at each event.

colinsurprenant commented 9 years ago

I will not create more issues for more buggy constructs, but here's another one

https://github.com/logstash-plugins/logstash-filter-kv/blob/407e3c545575758476c21474e6a2fc410d2ebaf8/lib/logstash/filters/kv.rb#L285

again, probably harmless, but here value =~ /[@value_split]/ will always return 0 thus will always be truthy and again defeating the short-circuit intent.

colinsurprenant commented 9 years ago

will be fixed by #20

colinsurprenant commented 8 years ago

fixed