-
Notifications
You must be signed in to change notification settings - Fork 161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed problem escaping string in JSON #347
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,5 +51,8 @@ class IdMismatch < Exceptions | |
|
||
class V1Format < Exceptions | ||
end | ||
|
||
class InvalidValue < Exceptions | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,10 +39,38 @@ 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!" | ||
end | ||
|
||
# I/P: json string | ||
# Raises `InvalidValue` if any of the json's values contain non-printable characters. | ||
def validate_json(json) | ||
begin | ||
evaled_json = eval(json) | ||
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 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 | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will fail unless all of the values of the json are strings. That is, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes @phiggins, it will work only for string values. any way to handle other formats? |
||
end | ||
|
||
# I/P: String | ||
# O/P: true/false | ||
# 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) | ||
/[^[:print:]]/.match(string) | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does calling
eval
on the json do, and what would cause a failure that is not fatal?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @phiggins, as our vault json contents string with an hash format, using eval converting to the hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a scenario where
eval
can fail andevaled_json
is still a valid value?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in vault we accept only hash format, there is less possibility of failing the eval method.