Merge pull request #1786 from rnt/fix/calling-yajl-parser-only-once

Calling yajl parser only once

7 files changed, 104 insertions, 95 deletions

diff --git a/lib/ohai/mixin/alibaba_metadata.rb b/lib/ohai/mixin/alibaba_metadata.rb
index 983dfd91..d371431c 100644
--- a/lib/ohai/mixin/alibaba_metadata.rb
+++ b/lib/ohai/mixin/alibaba_metadata.rb
@@ -18,6 +18,9 @@
 
 require "net/http" unless defined?(Net::HTTP)
 
+require_relative "../mixin/json_helper"
+include Ohai::Mixin::JsonHelper
+
 module Ohai
   module Mixin
     #
@@ -40,44 +43,28 @@ module Ohai
 
       def fetch_metadata(id = "")
         response = http_get(id)
-        return nil unless response.code == "200"
-
-        if json?(response.body)
-          data = String(response.body)
-          parser = FFI_Yajl::Parser.new
-          parser.parse(data)
-        elsif response.body.include?("\n")
-          temp = {}
-          response.body.split("\n").each do |sub_attr|
-            temp[sanitize_key(sub_attr)] = fetch_metadata("#{id}/#{sub_attr}")
+        if response.code == "200"
+          json_data = parse_json(response.body)
+          if json_data.nil?
+            logger.warn("Mixin AlibabaMetadata: Metadata response is NOT valid JSON for id='#{id}'")
+            if response.body.include?("\n")
+              temp = {}
+              response.body.split("\n").each do |sub_attr|
+                temp[sanitize_key(sub_attr)] = fetch_metadata("#{id}/#{sub_attr}")
+              end
+              temp
+            else
+              response.body
+            end
+          else
+            json_data
           end
-          temp
         else
-          response.body
+          logger.warn("Mixin AlibabaMetadata: Received response code #{response.code} requesting metadata for id='#{id}'")
+          nil
         end
       end
 
-      # @param [String] data that might be JSON
-      #
-      # @return [Boolean] is the data JSON or not?
-      def json?(data)
-        data = String(data)
-        parser = FFI_Yajl::Parser.new
-        begin
-          parser.parse(data)
-          true
-        rescue FFI_Yajl::ParseError
-          false
-        end
-      end
-
-      # @param data [String]
-      #
-      # @return [Boolean] is there a trailing /?
-      def has_trailing_slash?(data)
-        !!( data =~ %r{/$} )
-      end
-
       def sanitize_key(key)
         key.gsub(%r{\-|/}, "_")
       end
diff --git a/lib/ohai/mixin/azure_metadata.rb b/lib/ohai/mixin/azure_metadata.rb
index 7212bda2..12a8ab6b 100644
--- a/lib/ohai/mixin/azure_metadata.rb
+++ b/lib/ohai/mixin/azure_metadata.rb
@@ -18,6 +18,9 @@
 
 require "net/http" unless defined?(Net::HTTP)
 
+require_relative "../mixin/json_helper"
+include Ohai::Mixin::JsonHelper
+
 module Ohai
   module Mixin
     #
@@ -81,26 +84,16 @@ module Ohai
         conn.get(uri, { "Metadata" => "true" })
       end
 
-      # parse JSON data from a String to a Hash
-      #
-      # @param [String] response_body json as string to parse
-      #
-      # @return [Hash]
-      def parse_json(response_body)
-        data = String(response_body)
-        parser = FFI_Yajl::Parser.new
-        parser.parse(data)
-      rescue FFI_Yajl::ParseError
-        logger.warn("Mixin AzureMetadata: Metadata response is NOT valid JSON")
-        nil
-      end
-
-      def fetch_metadata(api_version = nil)
+      def fetch_metadata(_api_version = nil)
         metadata_url = "/metadata/instance?api-version=#{best_api_version}"
         logger.trace("Mixin AzureMetadata: Fetching metadata from host #{AZURE_METADATA_ADDR} at #{metadata_url}")
         response = http_get(metadata_url)
         if response.code == "200"
-          parse_json(response.body)
+          json_data = parse_json(response.body)
+          if json_data.nil?
+            logger.warn("Mixin AzureMetadata: Metadata response is NOT valid JSON")
+          end
+          json_data
         else
           logger.warn("Mixin AzureMetadata: Received response code #{response.code} requesting metadata")
           nil
diff --git a/lib/ohai/mixin/ec2_metadata.rb b/lib/ohai/mixin/ec2_metadata.rb
index 14c7932e..703ec8d8 100644
--- a/lib/ohai/mixin/ec2_metadata.rb
+++ b/lib/ohai/mixin/ec2_metadata.rb
@@ -20,6 +20,9 @@
 
 require "net/http" unless defined?(Net::HTTP)
 
+require_relative "../mixin/json_helper"
+include Ohai::Mixin::JsonHelper
+
 module Ohai
   module Mixin
     ##
@@ -229,9 +232,14 @@ module Ohai
         @fetch_dynamic_data ||= begin
           response = http_client.get("/#{best_api_version}/dynamic/instance-identity/document/", { 'X-aws-ec2-metadata-token': v2_token })
 
-          if json?(response.body) && response.code == "200"
-            FFI_Yajl::Parser.parse(response.body)
+          if response.code == "200"
+            json_data = parse_json(response.body, {})
+            if json_data.nil?
+              logger.warn("Mixin Ec2Metadata: Metadata response is NOT valid JSON")
+            end
+            json_data
           else
+            logger.warn("Mixin Ec2Metadata: Received response code #{response.code} requesting metadata")
             {}
           end
         end
diff --git a/lib/ohai/mixin/gce_metadata.rb b/lib/ohai/mixin/gce_metadata.rb
index 64b35f92..1f662223 100644
--- a/lib/ohai/mixin/gce_metadata.rb
+++ b/lib/ohai/mixin/gce_metadata.rb
@@ -17,6 +17,9 @@
 
 require "net/http" unless defined?(Net::HTTP)
 
+require_relative "../mixin/json_helper"
+include Ohai::Mixin::JsonHelper
+
 module Ohai
   module Mixin
     module GCEMetadata
@@ -37,34 +40,25 @@ module Ohai
 
       def fetch_metadata(id = "")
         response = http_get("#{GCE_METADATA_URL}/#{id}")
-        return nil unless response.code == "200"
-
-        if json?(response.body)
-          data = String(response.body)
-          parser = FFI_Yajl::Parser.new
-          parser.parse(data)
-        elsif has_trailing_slash?(id) || (id == "")
-          temp = {}
-          response.body.split("\n").each do |sub_attr|
-            temp[sanitize_key(sub_attr)] = fetch_metadata("#{id}#{sub_attr}")
+        if response.code == "200"
+          json_data = parse_json(response.body)
+          if json_data.nil?
+            logger.warn("Mixin GCEMetadata: Metadata response is NOT valid JSON for id='#{id}'")
+            if has_trailing_slash?(id) || (id == "")
+              temp = {}
+              response.body.split("\n").each do |sub_attr|
+                temp[sanitize_key(sub_attr)] = fetch_metadata("#{id}#{sub_attr}")
+              end
+              temp
+            else
+              response.body
+            end
+          else
+            json_data
           end
-          temp
         else
-          response.body
-        end
-      end
-
-      # @param [String] data that might be JSON
-      #
-      # @return [Boolean] is the data JSON or not?
-      def json?(data)
-        data = String(data)
-        parser = FFI_Yajl::Parser.new
-        begin
-          parser.parse(data)
-          true
-        rescue FFI_Yajl::ParseError
-          false
+          logger.warn("Mixin GCEMetadata: Received response code #{response.code} requesting metadata for id='#{id}'")
+          nil
         end
       end
 
diff --git a/lib/ohai/mixin/json_helper.rb b/lib/ohai/mixin/json_helper.rb
new file mode 100644
index 00000000..5ca9aae7
--- /dev/null
+++ b/lib/ohai/mixin/json_helper.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+#
+# Author:: Renato Covarrubias (<rnt@rnt.cl>)
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+module Ohai
+  module Mixin
+    module JsonHelper
+      # parse JSON data from a String to a Hash
+      #
+      # @param [String] response_body json as string to parse
+      # @param [Object] return_on_parse_error value to return if parsing fails
+      #
+      # @return [Hash]
+      def parse_json(response_body, return_on_parse_error = nil)
+        data = String(response_body)
+        parser = FFI_Yajl::Parser.new
+        parser.parse(data)
+      rescue FFI_Yajl::ParseError
+        return_on_parse_error
+      end
+    end
+  end
+end
diff --git a/lib/ohai/mixin/oci_metadata.rb b/lib/ohai/mixin/oci_metadata.rb
index b04243a3..c65c5754 100644
--- a/lib/ohai/mixin/oci_metadata.rb
+++ b/lib/ohai/mixin/oci_metadata.rb
@@ -18,6 +18,9 @@
 
 require "net/http" unless defined?(Net::HTTP)
 
+require_relative "../mixin/json_helper"
+include Ohai::Mixin::JsonHelper
+
 module Ohai
   module Mixin
     module OCIMetadata
@@ -38,27 +41,15 @@ module Ohai
         )
       end
 
-      # parse JSON data from a String to a Hash
-      #
-      # @param [String] response_body json as string to parse
-      #
-      # @return [Hash]
-      def parse_json(response_body)
-        data = String(response_body)
-        parser = FFI_Yajl::Parser.new
-        parser.parse(data)
-      rescue FFI_Yajl::ParseError
-        logger.warn("Mixin OciMetadata: Metadata response is NOT valid JSON")
-        nil
-      end
-
       # Fetch metadata from api
       def fetch_metadata(metadata = "instance")
         response = http_get("#{OCI_METADATA_URL}/#{metadata}")
-        return nil unless response.code == "200"
-
         if response.code == "200"
-          parse_json(response.body)
+          json_data = parse_json(response.body)
+          if json_data.nil?
+            logger.warn("Mixin OciMetadata: Metadata response is NOT valid JSON")
+          end
+          json_data
         else
           logger.warn("Mixin OciMetadata: Received response code #{response.code} requesting metadata")
           nil
diff --git a/spec/unit/mixin/oci_metadata_spec.rb b/spec/unit/mixin/oci_metadata_spec.rb
index 3135b028..227c3160 100644
--- a/spec/unit/mixin/oci_metadata_spec.rb
+++ b/spec/unit/mixin/oci_metadata_spec.rb
@@ -49,7 +49,7 @@ describe Ohai::Mixin::OCIMetadata do
       http_mock = double("http", { code: "404" })
       allow(mixin).to receive(:http_get).and_return(http_mock)
 
-      expect(mixin.logger).not_to receive(:warn)
+      expect(mixin.logger).to receive(:warn)
       vals = mixin.fetch_metadata
       expect(vals).to eq(nil)
     end