Skip to content
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

Inja transformation preserve escaped json #258

Merged
merged 9 commits into from
Jul 21, 2023
Merged

Conversation

jbohanon
Copy link
Contributor

@jbohanon jbohanon commented Jul 19, 2023

Users have the issue that when they transform a request or response body that includes escaped characters, for example this json request/response body

{
  "foo": "{\"bar\":\"baz\"}"
}

The characters lose their escaping when used in a template e.g. the template {"response":"{{ foo }}"} would produce the string

{"response":"{"bar":"baz"}"}

which is not valid JSON since the quotes are not escaped. Instead we expect to be able to produce

{"response":"{\"bar\":\"baz\"}"}

This PR provides 2 methods of achieving this

  • The new callback raw_string can be used to escape a particular value within a template.
    • e.g. the template would look like {"response":"{{ raw_string(foo) }}"}
  • The transformation itself can be configured to do this for all string values found in the body and rendered in the template.
    • This is the new setting escape_characters on the TransformationTemplate setting

@solo-changelog-bot
Copy link

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/5155

Comment on lines 259 to 261
// Use to render the output of a body transformation as JSON. This will cause
// rendered string values to be escaped in order to make valid JSON strings
bool render_body_as_json = 12;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should rename this. perhaps this has similar utility for folks who want the body rendered for, say, yaml

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

say escape_characters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it. I renamed the field

Copy link
Contributor

@kdorosh kdorosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm beyond naming nit

add comments; rename new proto field; add tests for nested json
Copy link
Contributor

@kdorosh kdorosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment on lines +356 to +361
// make sure to bail if we're not working with a raw string value
if(!input->is_string()) {
return input->get_ref<const std::string&>();
}

auto val = input->dump();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@kdorosh kdorosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soloio-bulldozer soloio-bulldozer bot merged commit 55855b8 into main Jul 21, 2023
@soloio-bulldozer soloio-bulldozer bot deleted the inja-body-as-json branch July 21, 2023 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants