4.1.0 improvments

CHANGELOG.md

@@ -1,3 +1,11 @@
+## 4.1.0
+ - bugfix: catch xpath-expression syntax error instead of impacting pipeline. Fix #19
+ - bugfix: report xml parsing error when parsing only with xpath and store_xml => false (using Nokogiri internally). Fix #10
+ - config: allow to create xpath-expression from event data using dynamic syntax
+ - config: fail at startup if store_xml => false and no xpath config specified, as the filter would do nothing
+ - internal: do not parse document with Nokogiri when xpath contains no expressions
+ - internal: restructure tests using contexts
+
 ## 4.0.0
   - breaking,config: New configuration `suppress_empty`. Default to true change default behaviour of the plugin in favor of avoiding mapping conflicts when reaching elasticsearch
   - config: New configuration `force_content`. By default the filter expands attributes differently from content in xml elements.

lib/logstash/filters/xml.rb

@@ -110,6 +110,15 @@ def register
         :error => "When the 'store_xml' configuration option is true, 'target' must also be set"
       )
     end
+
+    if xpath.empty? && !@store_xml
+      raise LogStash::ConfigurationError, I18n.t(
+        "logstash.agent.configuration.invalid_plugin_register",
+        :plugin => "filter",
+        :type => "xml",
+        :error => "When the 'store_xml' configuration option is false, 'xpath' must contains elements"
+      )
+    end
   end
 
   def filter(event)
@@ -139,18 +148,27 @@ def filter(event)
     # Do nothing with an empty string.
     return if value.strip.empty?
 
-    if @xpath
+    unless @xpath.empty?
       begin
         doc = Nokogiri::XML(value, nil, value.encoding.to_s)
+        raise doc.errors unless doc.errors.empty?
       rescue => e
         event.tag(XMLPARSEFAILURE_TAG)
         @logger.warn("Error parsing xml", :source => @source, :value => value, :exception => e, :backtrace => e.backtrace)
         return
       end
       doc.remove_namespaces! if @remove_namespaces
 
-      @xpath.each do |xpath_src, xpath_dest|
-        nodeset = @namespaces.empty? ? doc.xpath(xpath_src) : doc.xpath(xpath_src, @namespaces)
+      @xpath.each do |xpath_syntax, xpath_dest|
+        xpath_src = event.sprintf(xpath_syntax)
+        begin
+          nodeset = @namespaces.empty? ? doc.xpath(xpath_src) : doc.xpath(xpath_src, @namespaces)
+        rescue Nokogiri::XML::XPath::SyntaxError => e
+            event.tag("_xpathsyntaxfailure")
+            @logger.warn("Trouble parsing the xpath expression", :source => @source, :value => value, :xpath => xpath_src,
+                      :exception => e, :backtrace => e.backtrace)
+            return
+        end
 
         # If asking xpath for a String, like "name(/*)", we get back a
         # String instead of a NodeSet.  We normalize that here.

logstash-filter-xml.gemspec

@@ -1,7 +1,7 @@
 Gem::Specification.new do |s|
 
   s.name            = 'logstash-filter-xml'
-  s.version         = '4.0.0'
+  s.version         = '4.1.0'
   s.licenses        = ['Apache License (2.0)']
   s.summary         = "Takes a field that contains XML and expands it into an actual datastructure."
   s.description     = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gemname. This gem is not a stand-alone program"