Handled syntax error also refactored code and added test cases for th…

…e changes

Signed-off-by: sanga17 <sausekar@msystechnologies.com>

lib/chef/knife/mixin/helper.rb

@@ -27,7 +27,6 @@ def set_mode(mode)
       def merge_values(json, file)
         values = {}
         values.merge!(values_from_file(file)) if file
-        validate_json(json)
         values.merge!(values_from_json(json)) if json
 
         values
@@ -40,6 +39,7 @@ def values_from_file(file)
       end
 
       def values_from_json(json)
+        validate_json(json)
         JSON.parse(json)
       rescue JSON::ParserError
         raise JSON::ParserError, "#{json} is not valid JSON!"
@@ -50,13 +50,13 @@ def values_from_json(json)
       def validate_json(json)
         begin
           evaled_json = eval(json)
-        rescue StandardError => e
-          puts e.message
+        rescue SyntaxError => e
+          raise ChefVault::Exceptions::InvalidValue, "#{json} is not valid JSON!"
         end
 
         if evaled_json.is_a?(Hash)
           evaled_json.each do |key, value|
-            next if printable?(value)
+            next unless printable?(value.to_s)
 
             msg = "Value '#{value}' of key '#{key}' contains non-printable characters. Check that backslashes are escaped with another backslash (e.g. C:\\\\Windows) in double-quoted strings."
             raise ChefVault::Exceptions::InvalidValue, msg
@@ -69,8 +69,7 @@ def validate_json(json)
       # returns true if string is free of non-printable characters (escape sequences)
       # this returns false for whitespace escape sequences as well, e.g. \n\t
       def printable?(string)
-        return false if string =~ /[^[:print:]]/
-        true
+        /[^[:print:]]/.match(string)
       end
     end
   end

spec/chef/helper_spec.rb

@@ -0,0 +1,24 @@
+require "spec_helper"
+require "chef/knife/mixin/helper"
+
+RSpec.describe ChefVault::Mixin::Helper do
+  include ChefVault::Mixin::Helper
+
+  let(:json_data) { '{"username": "root", "password": "abcabc"}' }
+  let(:json_data_control_char) { '{"username": "root", "password": "abc\abc"}' }
+  let(:buggy_json_data) { '{"username": "root", "password": "abc\abc"' }
+
+  describe "#validate_json" do
+    it "Raises InvalidValue Exception when invalid data provided" do
+      expect { validate_json(buggy_json_data) }.to raise_error(ChefVault::Exceptions::InvalidValue)
+    end
+
+    it "Raises InvalidValue Exception when value consist of control characters" do
+      expect { validate_json(json_data_control_char) }.to raise_error(ChefVault::Exceptions::InvalidValue)
+    end
+
+    it "Not to raise error if valid data provided" do
+      expect { validate_json(json_data) }.to_not raise_error
+    end
+  end
+end