-
-
Notifications
You must be signed in to change notification settings - Fork 272
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
Escape json posts in curl_json plugin #735
Conversation
| @@ -40,10 +40,22 @@ LoadPlugin "curl_json" | |||
| <% if @header -%> | |||
| Header "<%= @header %>" | |||
| <% end -%> | |||
| <%- | |||
| def valid_json?(json) | |||
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.
I'm not sure this is a great idea. Suggestions welcome!
| <% unless @post.nil? -%> | ||
| <% if valid_json?(@post) -%> | ||
| Post "<%= @post.gsub('\\"','\\\\\\\\\"').gsub!(%r{(?<!\\)"},'\"') %>" |
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.
I'm very very sorry! All those backslashes are needed though. See https://www.ruby-forum.com/topic/143645
| <% unless @post.nil? -%> | ||
| <% if valid_json?(@post) -%> | ||
| Post "<%= @post.gsub('\\"','\\\\\\\\\"').gsub!(%r{(?<!\\)"},'\"') %>" | ||
| <% else -%> | ||
| Post "<%= @post %>" |
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.
If it's not JSON, nothing is changed. If it is, it definitely won't work if we don't do any escaping.
| @@ -28,8 +28,8 @@ | |||
| verifypeer: 'false', | |||
| verifyhost: 'false', | |||
| cacert: '/path/to/ca.crt', | |||
| header: 'Accept: application/json', | |||
| post: '{secret: \"mysecret\"}', | |||
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.
This wasn't valid JSON.
1edf2ad
to
56f4954
Compare
| @@ -13,6 +13,7 @@ | |||
| let :facts do | |||
| { | |||
| osfamily: 'FreeBSD', | |||
| operatingsystem: 'FreeBSD', | |||
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.
Unrelated change to fix intermittent test failures.
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.
could you migrate this to rspec puppet facts?
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.
Love to, but there don't appear to be any. https://github.com/camptocamp/facterdb/tree/master/facts/2.5
*If* in the 'post' parameter we've been given a valid json string, it
definately won't work unless we do some escaping of double quotes.
If any strings in the JSON also contain (correctly escaped) double
quotes, even more backslashes are needed.
eg. The following JSON
```
{"param1":"something with a double quote \" in it"}
```
Would end up in the config file as
```
Post "{\"param1\":\"something with a double quote \\\" in it\"}"
```
56f4954
to
d2b03bc
Compare
|
@bastelfreak You happy with this? |
If in the 'post' parameter we've been given a valid json string, it
definately won't work unless we do some escaping of double quotes.
If any strings in the JSON also contain (correctly escaped) double
quotes, even more backslashes are needed.
eg. The following JSON
Would end up in the config file as