From 6fcfde4872b0d1b50047f7c379d986257b5437cf Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Wed, 19 Jul 2023 09:50:30 -0400 Subject: [PATCH 1/9] add option to json escape after rendering --- .../http/transformation/inja_transformer.cc | 10 ++++++++++ .../http/transformation/inja_transformer.h | 3 ++- .../http/transformation/inja_transformer_test.cc | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index e7fd638b7..957648a77 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/source/extensions/filters/http/transformation/inja_transformer.cc @@ -606,6 +606,16 @@ void InjaTransformer::transform(Http::RequestOrResponseHeaderMap &header_map, // replace body. we do it here so that headers and dynamic metadata have the // original body. if (maybe_body.has_value()) { + json body_json; + if (render_body_as_json_) { + try { + body_json = json(maybe_body.value().toString()); + } catch (json::parse_error parse_exception) { + ENVOY_STREAM_LOG(debug, "unable to parse body as json; returning as raw {}", callbacks, maybe_body.value().toString()); + } + maybe_body.emplace(body_json.dump()); + } + // remove content length, as we have new body. header_map.removeContentLength(); // replace body diff --git a/source/extensions/filters/http/transformation/inja_transformer.h b/source/extensions/filters/http/transformation/inja_transformer.h index 3a3a844af..5a35c3fd6 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.h +++ b/source/extensions/filters/http/transformation/inja_transformer.h @@ -86,7 +86,7 @@ class Extractor : Logger::Loggable { const std::regex extract_regex_; }; -class InjaTransformer : public Transformer { +class InjaTransformer : public Transformer, Logger::Loggable { public: InjaTransformer(const envoy::api::v2::filter::http::TransformationTemplate &transformation, Envoy::Random::RandomGenerator &rng, @@ -115,6 +115,7 @@ class InjaTransformer : public Transformer { std::vector headers_to_remove_; std::vector dynamic_metadata_; std::unordered_map environ_; + bool render_body_as_json_{}; envoy::api::v2::filter::http::TransformationTemplate::RequestBodyParse parse_body_behavior_; diff --git a/test/extensions/filters/http/transformation/inja_transformer_test.cc b/test/extensions/filters/http/transformation/inja_transformer_test.cc index 2f2f4b19c..a9c8f82e4 100644 --- a/test/extensions/filters/http/transformation/inja_transformer_test.cc +++ b/test/extensions/filters/http/transformation/inja_transformer_test.cc @@ -1147,6 +1147,21 @@ TEST_F(InjaTransformerTest, ParseUsingJsonPointerSyntax) { EXPECT_EQ(body.toString(), "online--slash--tilde"); } +TEST_F(InjaTransformerTest, RenderBodyAsJson) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + transformation.mutable_body()->set_text( + "{\"Value\":\"{{ value }}\"}"); + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + + Buffer::OwnedImpl body("{\"value\":\"foo\"}"); + transformer.transform(headers, &headers, body, callbacks); + EXPECT_EQ(body.toString(), "{\\\"Value\\\":\\\"foo\\\"}"); +} + + } // namespace Transformation } // namespace HttpFilters } // namespace Extensions From f240b92f0178d0a7068832f338473c16409fd805 Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Wed, 19 Jul 2023 17:06:10 -0400 Subject: [PATCH 2/9] use new inja feature to escape strings --- .../v2/transformation_filter.proto | 11 ++- bazel/repository_locations.bzl | 5 +- .../http/transformation/inja_transformer.cc | 24 ++++--- .../http/transformation/inja_transformer.h | 6 +- .../transformation/inja_transformer_test.cc | 12 ++-- .../transformation_filter_integration_test.cc | 70 +++++++++++++++++-- 6 files changed, 105 insertions(+), 23 deletions(-) diff --git a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto index 5151088d0..cfe104852 100644 --- a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto +++ b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto @@ -204,9 +204,9 @@ message TransformationTemplate { InjaTemplate value = 2; } - // Use this attribute to transform request/response headers. It consists of - // an array of string/template objects. Use this attribute to define multiple - // templates for a single header. Header template(s) defined here will be appended to any + // Use this attribute to transform request/response headers. It consists of + // an array of string/template objects. Use this attribute to define multiple + // templates for a single header. Header template(s) defined here will be appended to any // existing headers with the same header name, not replace existing ones. repeated HeaderToAppend headers_to_append = 10; @@ -255,6 +255,11 @@ message TransformationTemplate { } // Use this field to set Dynamic Metadata. repeated DynamicMetadataValue dynamic_metadata_values = 9; + + // Use to render the output of a body transformation as JSON. This will cause + // rendered values to be escaped in order to make valid JSON strings + bool render_body_as_json = 12; + } // Defines an [Inja template](https://github.com/pantor/inja) that will be diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 0a8b62d66..c951e4d1c 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -5,7 +5,10 @@ REPOSITORY_LOCATIONS = dict( remote = "https://github.com/envoyproxy/envoy", ), inja = dict( - commit = "5a18986825fc7e5d2916ff345c4369e4b6ea7122", # v3.4 + JSON Pointers + # Includes unmerged modifications for + # - JSON pointer syntax support + # - Allowing escaped strings + commit = "3aa95b8b58a525f86f79cb547bf937176c9cc7ff", # v3.4.0-patch1 remote = "https://github.com/solo-io/inja", # solo-io fork including the changes ), json = dict( diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index 957648a77..69a37acdf 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/source/extensions/filters/http/transformation/inja_transformer.cc @@ -143,6 +143,9 @@ TransformerInstance::TransformerInstance(ThreadLocal::Slot &tls, Envoy::Random:: env_.add_callback("replace_with_random", 2, [this](Arguments &args) { return replace_with_random_callback(args); }); + env_.add_callback("json_escaped", 1, [this](Arguments &args) { + return json_escaped_callback(args); + }); } json TransformerInstance::header_callback(const inja::Arguments &args) const { @@ -343,6 +346,11 @@ std::string& TransformerInstance::random_for_pattern(const std::string& pattern) return found->second; } +json TransformerInstance::json_escaped_callback(const inja::Arguments &args) const { + const std::string &input = args.at(0)->get_ref(); + return json::parse(input).dump(); +} + // parse calls Inja::Environment::parse which uses non-const references to member // data fields. This method is NOT SAFE to call outside of the InjaTransformer // constructor since doing so could cause Inja::Environment member fields to be @@ -372,12 +380,15 @@ InjaTransformer::InjaTransformer(const TransformationTemplate &transformation, passthrough_body_(transformation.has_passthrough()), parse_body_behavior_(transformation.parse_body_behavior()), ignore_error_on_parse_(transformation.ignore_error_on_parse()), + render_body_as_json_(transformation.render_body_as_json()), tls_(tls.allocateSlot()), instance_(std::make_unique(*tls_, rng)) { if (advanced_templates_) { instance_->set_element_notation(inja::ElementNotation::Pointer); } + instance_->set_escape_strings(render_body_as_json_); + tls_->set([](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { return std::make_shared(); }); @@ -558,6 +569,8 @@ void InjaTransformer::transform(Http::RequestOrResponseHeaderMap &header_map, if (body_template_.has_value()) { std::string output = instance_->render(body_template_.value()); + + std::cout << "output: " << output << std::endl; maybe_body.emplace(output); } else if (merged_extractors_to_body_) { std::string output = json_body.dump(); @@ -606,15 +619,8 @@ void InjaTransformer::transform(Http::RequestOrResponseHeaderMap &header_map, // replace body. we do it here so that headers and dynamic metadata have the // original body. if (maybe_body.has_value()) { - json body_json; - if (render_body_as_json_) { - try { - body_json = json(maybe_body.value().toString()); - } catch (json::parse_error parse_exception) { - ENVOY_STREAM_LOG(debug, "unable to parse body as json; returning as raw {}", callbacks, maybe_body.value().toString()); - } - maybe_body.emplace(body_json.dump()); - } + auto body_string = maybe_body.value().toString(); + std::cout << "body_string: " << body_string << std::endl; // remove content length, as we have new body. header_map.removeContentLength(); diff --git a/source/extensions/filters/http/transformation/inja_transformer.h b/source/extensions/filters/http/transformation/inja_transformer.h index 5a35c3fd6..2591b2c3a 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.h +++ b/source/extensions/filters/http/transformation/inja_transformer.h @@ -47,6 +47,9 @@ class TransformerInstance { void set_element_notation(inja::ElementNotation notation) { env_.set_element_notation(notation); }; + void set_escape_strings(bool escape_strings) { + env_.set_escape_strings(escape_strings); + }; private: // header_value(name) @@ -62,6 +65,7 @@ class TransformerInstance { nlohmann::json substring_callback(const inja::Arguments &args) const; nlohmann::json replace_with_random_callback(const inja::Arguments &args); std::string& random_for_pattern(const std::string& pattern); + nlohmann::json json_escaped_callback(const inja::Arguments &args) const; inja::Environment env_; absl::flat_hash_map pattern_replacements_; @@ -115,11 +119,11 @@ class InjaTransformer : public Transformer, Logger::Loggable std::vector headers_to_remove_; std::vector dynamic_metadata_; std::unordered_map environ_; - bool render_body_as_json_{}; envoy::api::v2::filter::http::TransformationTemplate::RequestBodyParse parse_body_behavior_; bool ignore_error_on_parse_; + bool render_body_as_json_{}; absl::optional body_template_; bool merged_extractors_to_body_{}; diff --git a/test/extensions/filters/http/transformation/inja_transformer_test.cc b/test/extensions/filters/http/transformation/inja_transformer_test.cc index a9c8f82e4..d50f54ff5 100644 --- a/test/extensions/filters/http/transformation/inja_transformer_test.cc +++ b/test/extensions/filters/http/transformation/inja_transformer_test.cc @@ -1151,17 +1151,21 @@ TEST_F(InjaTransformerTest, RenderBodyAsJson) { Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; TransformationTemplate transformation; transformation.mutable_body()->set_text( - "{\"Value\":\"{{ value }}\"}"); + R"EOF({"Value":"{{ value }}"})EOF"); + transformation.set_render_body_as_json(true); InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); NiceMock callbacks; - Buffer::OwnedImpl body("{\"value\":\"foo\"}"); + Buffer::OwnedImpl body(R"({"value":"\"foo\""})"_json.dump()); + auto expected_body = R"({"Value":"\"foo\""})"_json.dump(); + std::cout << "body.toString() before: " << body.toString() << std::endl; transformer.transform(headers, &headers, body, callbacks); - EXPECT_EQ(body.toString(), "{\\\"Value\\\":\\\"foo\\\"}"); + std::cout << "body.toString() after: " << body.toString() << std::endl; + std::cout << "expected_body: " << expected_body << std::endl; + EXPECT_EQ(body.toString(), expected_body); } - } // namespace Transformation } // namespace HttpFilters } // namespace Extensions diff --git a/test/integration/transformation_filter_integration_test.cc b/test/integration/transformation_filter_integration_test.cc index 5c66daf65..b45efca9a 100644 --- a/test/integration/transformation_filter_integration_test.cc +++ b/test/integration/transformation_filter_integration_test.cc @@ -32,14 +32,14 @@ const std::string DEFAULT_TRANSFORMATION = text: abc {{extraction("ext1")}} )EOF"; -const std::string DUPLICATE_HEADER_TRANSFORMATION = +const std::string DUPLICATE_HEADER_TRANSFORMATION = R"EOF( request_transformation: transformation_template: advanced_templates: true headers_to_append: - key: x-solo - value: + value: text: appended header 1 - key: x-solo value: @@ -124,6 +124,22 @@ const std::string PASSTHROUGH_TRANSFORMATION = passthrough: {} )EOF"; +const std::string RENDER_BODY_AS_JSON_TRANSFORMATION = + R"EOF( + request_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: true + body: + text: "{\"Foo\":\"{{ foo }}\"}" + response_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: true + body: + text: "{\"Bar\":\"{{ bar }}\"}" +)EOF"; + const std::string DEFAULT_FILTER_TRANSFORMATION = R"EOF( {} @@ -238,7 +254,7 @@ TEST_P(TransformationFilterIntegrationTest, TransformHeaderOnlyRequest) { auto response = codec_client_->makeHeaderOnlyRequest(request_headers); processRequest(response); - + std::string xsolo_header(upstream_request_->headers() .get(Http::LowerCaseString("x-solo"))[0] ->value() @@ -457,7 +473,7 @@ TEST_P(TransformationFilterIntegrationTest, BodyHeaderTransform) { json actual_request = json::parse(upstream_request_->body().toString()); actual_request["headers"]["x-request-id"] = ""; // zero out random headers - auto expected_request = R"( + auto expected_request = R"( { "body":"{\"abc\":\"efg\"}", "headers": { @@ -481,7 +497,7 @@ TEST_P(TransformationFilterIntegrationTest, BodyHeaderTransform) { // remove the `x-envoy-upstream-service-time` since its // value depends on how long the test took to run actual_response["headers"].erase("x-envoy-upstream-service-time"); - auto expected_response = R"( + auto expected_response = R"( { "headers": { ":status":"200", @@ -493,4 +509,48 @@ TEST_P(TransformationFilterIntegrationTest, BodyHeaderTransform) { } +TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransform) { + using json = nlohmann::json; + transformation_string_ = + R"EOF( + request_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: true + body: + text: "{\"Foo\":\"{{ foo }}\"}" + response_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: true + body: + text: "{\"Bar\":\"{{ bar }}\"}" +)EOF"; + initialize(); + std::string origReqBody = R"EOF({"foo":"\"bar\""})EOF"; + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":authority", "www.solo.io"}, + {":path", "/"}}; + auto encoder_decoder = codec_client_->startRequest(request_headers); + + auto downstream_request = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + + Buffer::OwnedImpl data(origReqBody); + codec_client_->sendData(*downstream_request, data, true); + + processRequest(response, R"EOF({"bar":"\"baz\""})EOF"); + + auto actual_request_body = json::parse(upstream_request_->body().toString()); + auto expected_request_body = R"({"Foo":"\"bar\""})"_json; + + // make sure response works as well, with no metadata set: + EXPECT_EQ(expected_request_body, actual_request_body); + + json actual_response_body = json::parse(response->body()); + // remove the `x-envoy-upstream-service-time` since its + // value depends on how long the test took to run + auto expected_response_body = R"({"Bar":"\"baz\""})"_json; + EXPECT_EQ(expected_response_body, actual_response_body); +} } // namespace Envoy From a0f835f64307de1fe16cfab316a3698109c04504 Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Wed, 19 Jul 2023 17:09:38 -0400 Subject: [PATCH 3/9] cleanup --- .../http/transformation/inja_transformer.cc | 13 ------------- .../http/transformation/inja_transformer.h | 1 - .../http/transformation/inja_transformer_test.cc | 3 --- .../transformation_filter_integration_test.cc | 16 ---------------- 4 files changed, 33 deletions(-) diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index 69a37acdf..21ca781e0 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/source/extensions/filters/http/transformation/inja_transformer.cc @@ -143,9 +143,6 @@ TransformerInstance::TransformerInstance(ThreadLocal::Slot &tls, Envoy::Random:: env_.add_callback("replace_with_random", 2, [this](Arguments &args) { return replace_with_random_callback(args); }); - env_.add_callback("json_escaped", 1, [this](Arguments &args) { - return json_escaped_callback(args); - }); } json TransformerInstance::header_callback(const inja::Arguments &args) const { @@ -346,11 +343,6 @@ std::string& TransformerInstance::random_for_pattern(const std::string& pattern) return found->second; } -json TransformerInstance::json_escaped_callback(const inja::Arguments &args) const { - const std::string &input = args.at(0)->get_ref(); - return json::parse(input).dump(); -} - // parse calls Inja::Environment::parse which uses non-const references to member // data fields. This method is NOT SAFE to call outside of the InjaTransformer // constructor since doing so could cause Inja::Environment member fields to be @@ -569,8 +561,6 @@ void InjaTransformer::transform(Http::RequestOrResponseHeaderMap &header_map, if (body_template_.has_value()) { std::string output = instance_->render(body_template_.value()); - - std::cout << "output: " << output << std::endl; maybe_body.emplace(output); } else if (merged_extractors_to_body_) { std::string output = json_body.dump(); @@ -619,9 +609,6 @@ void InjaTransformer::transform(Http::RequestOrResponseHeaderMap &header_map, // replace body. we do it here so that headers and dynamic metadata have the // original body. if (maybe_body.has_value()) { - auto body_string = maybe_body.value().toString(); - std::cout << "body_string: " << body_string << std::endl; - // remove content length, as we have new body. header_map.removeContentLength(); // replace body diff --git a/source/extensions/filters/http/transformation/inja_transformer.h b/source/extensions/filters/http/transformation/inja_transformer.h index 2591b2c3a..b7eb372a2 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.h +++ b/source/extensions/filters/http/transformation/inja_transformer.h @@ -65,7 +65,6 @@ class TransformerInstance { nlohmann::json substring_callback(const inja::Arguments &args) const; nlohmann::json replace_with_random_callback(const inja::Arguments &args); std::string& random_for_pattern(const std::string& pattern); - nlohmann::json json_escaped_callback(const inja::Arguments &args) const; inja::Environment env_; absl::flat_hash_map pattern_replacements_; diff --git a/test/extensions/filters/http/transformation/inja_transformer_test.cc b/test/extensions/filters/http/transformation/inja_transformer_test.cc index d50f54ff5..049f06732 100644 --- a/test/extensions/filters/http/transformation/inja_transformer_test.cc +++ b/test/extensions/filters/http/transformation/inja_transformer_test.cc @@ -1159,10 +1159,7 @@ TEST_F(InjaTransformerTest, RenderBodyAsJson) { Buffer::OwnedImpl body(R"({"value":"\"foo\""})"_json.dump()); auto expected_body = R"({"Value":"\"foo\""})"_json.dump(); - std::cout << "body.toString() before: " << body.toString() << std::endl; transformer.transform(headers, &headers, body, callbacks); - std::cout << "body.toString() after: " << body.toString() << std::endl; - std::cout << "expected_body: " << expected_body << std::endl; EXPECT_EQ(body.toString(), expected_body); } diff --git a/test/integration/transformation_filter_integration_test.cc b/test/integration/transformation_filter_integration_test.cc index b45efca9a..c8aa2c91d 100644 --- a/test/integration/transformation_filter_integration_test.cc +++ b/test/integration/transformation_filter_integration_test.cc @@ -124,22 +124,6 @@ const std::string PASSTHROUGH_TRANSFORMATION = passthrough: {} )EOF"; -const std::string RENDER_BODY_AS_JSON_TRANSFORMATION = - R"EOF( - request_transformation: - transformation_template: - advanced_templates: false - render_body_as_json: true - body: - text: "{\"Foo\":\"{{ foo }}\"}" - response_transformation: - transformation_template: - advanced_templates: false - render_body_as_json: true - body: - text: "{\"Bar\":\"{{ bar }}\"}" -)EOF"; - const std::string DEFAULT_FILTER_TRANSFORMATION = R"EOF( {} From 0e887fdbbdafcdf0aa2ba5a9bd46d6b58a44fd85 Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Thu, 20 Jul 2023 12:29:33 -0400 Subject: [PATCH 4/9] add to_json callback to visualize options --- .../http/transformation/inja_transformer.cc | 28 ++++++++++++ .../http/transformation/inja_transformer.h | 1 + .../transformation/inja_transformer_test.cc | 34 ++++++++++++++ .../transformation_filter_integration_test.cc | 45 +++++++++++++++++++ 4 files changed, 108 insertions(+) diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index 21ca781e0..0e38241cd 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/source/extensions/filters/http/transformation/inja_transformer.cc @@ -143,6 +143,9 @@ TransformerInstance::TransformerInstance(ThreadLocal::Slot &tls, Envoy::Random:: env_.add_callback("replace_with_random", 2, [this](Arguments &args) { return replace_with_random_callback(args); }); + env_.add_callback("to_json", 1, [this](Arguments &args) { + return tojson_callback(args); + }); } json TransformerInstance::header_callback(const inja::Arguments &args) const { @@ -343,6 +346,31 @@ std::string& TransformerInstance::random_for_pattern(const std::string& pattern) return found->second; } +json TransformerInstance::tojson_callback(const inja::Arguments &args) const { + const std::string &input = args.at(0)->get_ref(); + std::cout << "modifying input: \n" << input << std::endl; + // in order to get the escaping the way we want, we must cast directly to a json object + // since using parse will cause the surrounding " to be stripped + auto parsed = json(input); + std::cout << "modified input (as string): \n" << parsed.get() << std::endl; + + auto val = parsed.dump(); + std::cout << "modified input (as dump): \n" << val << std::endl; + + // This block is optional. It makes it such that a template must have surrounding + // " characters. This is logical to me since we expect the value we get out of the + // context (body) to be placed in exactly as-is. HOWEVER, the behavior of jinja + // is such that the quotes added by .dumps() are left in, so to mirror that impl + // exactly, we should not strip them. + + /* // strip the leading and trailing " characters that are added by dump() */ + /* // if C++20 is adopted, val.starts_with and val.ends_with would clean this up a bit */ + /* val = val.substr(0,1) == "\"" && val.substr(val.length()-1,1) == "\"" */ + /* ? val.substr(1, val.length()-2) */ + /* : val; */ + return val; +} + // parse calls Inja::Environment::parse which uses non-const references to member // data fields. This method is NOT SAFE to call outside of the InjaTransformer // constructor since doing so could cause Inja::Environment member fields to be diff --git a/source/extensions/filters/http/transformation/inja_transformer.h b/source/extensions/filters/http/transformation/inja_transformer.h index b7eb372a2..ece26aa33 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.h +++ b/source/extensions/filters/http/transformation/inja_transformer.h @@ -65,6 +65,7 @@ class TransformerInstance { nlohmann::json substring_callback(const inja::Arguments &args) const; nlohmann::json replace_with_random_callback(const inja::Arguments &args); std::string& random_for_pattern(const std::string& pattern); + nlohmann::json tojson_callback(const inja::Arguments &args) const; inja::Environment env_; absl::flat_hash_map pattern_replacements_; diff --git a/test/extensions/filters/http/transformation/inja_transformer_test.cc b/test/extensions/filters/http/transformation/inja_transformer_test.cc index 049f06732..badfb5bfc 100644 --- a/test/extensions/filters/http/transformation/inja_transformer_test.cc +++ b/test/extensions/filters/http/transformation/inja_transformer_test.cc @@ -1163,6 +1163,40 @@ TEST_F(InjaTransformerTest, RenderBodyAsJson) { EXPECT_EQ(body.toString(), expected_body); } +TEST_F(InjaTransformerTest, RenderBodyAsJsonPreEscapedInput) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + transformation.mutable_body()->set_text( + R"EOF({"Value":"{{ value }}"})EOF"); + transformation.set_render_body_as_json(false); + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + + Buffer::OwnedImpl body(R"({"value":"\\\"foo\\\""})"_json.dump()); + auto expected_body = R"({"Value":"\"foo\""})"_json.dump(); + transformer.transform(headers, &headers, body, callbacks); + std::cout << expected_body << std::endl; + EXPECT_EQ(body.toString(), expected_body); +} + +TEST_F(InjaTransformerTest, RenderBodyAsJsonWithCallback) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + transformation.mutable_body()->set_text( + R"EOF({"Value":{{ to_json(value) }}})EOF"); + transformation.set_render_body_as_json(false); + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + + Buffer::OwnedImpl body(R"({"value":"\"foo\""})"_json.dump()); + auto expected_body = R"({"Value":"\"foo\""})"_json.dump(); + transformer.transform(headers, &headers, body, callbacks); + std::cout << "expected_body: " << expected_body << std::endl; + EXPECT_EQ(body.toString(), expected_body); +} + } // namespace Transformation } // namespace HttpFilters } // namespace Extensions diff --git a/test/integration/transformation_filter_integration_test.cc b/test/integration/transformation_filter_integration_test.cc index c8aa2c91d..07a526a9a 100644 --- a/test/integration/transformation_filter_integration_test.cc +++ b/test/integration/transformation_filter_integration_test.cc @@ -537,4 +537,49 @@ TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransform) { auto expected_response_body = R"({"Bar":"\"baz\""})"_json; EXPECT_EQ(expected_response_body, actual_response_body); } + +TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransformEscapedInput) { + using json = nlohmann::json; + transformation_string_ = + R"EOF( + request_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: false + body: + text: "{\"Foo\":\"{{ foo }}\"}" + response_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: false + body: + text: "{\"Bar\":\"{{ bar }}\"}" +)EOF"; + initialize(); + std::string origReqBody = R"EOF({"foo":"\\\"bar\\\""})EOF"; + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":authority", "www.solo.io"}, + {":path", "/"}}; + auto encoder_decoder = codec_client_->startRequest(request_headers); + + auto downstream_request = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + + Buffer::OwnedImpl data(origReqBody); + codec_client_->sendData(*downstream_request, data, true); + + processRequest(response, R"EOF({"bar":"\\\"baz\\\""})EOF"); + + auto actual_request_body = json::parse(upstream_request_->body().toString()); + auto expected_request_body = R"({"Foo":"\"bar\""})"_json; + + // make sure response works as well, with no metadata set: + EXPECT_EQ(expected_request_body, actual_request_body); + + json actual_response_body = json::parse(response->body()); + // remove the `x-envoy-upstream-service-time` since its + // value depends on how long the test took to run + auto expected_response_body = R"({"Bar":"\"baz\""})"_json; + EXPECT_EQ(expected_response_body, actual_response_body); +} } // namespace Envoy From a3fa99354dd78f2e1b44ec54805db43c65450b1e Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Thu, 20 Jul 2023 15:00:22 -0400 Subject: [PATCH 5/9] do strip " on callback --- .../http/transformation/inja_transformer.cc | 45 ++++++++++--------- .../http/transformation/inja_transformer.h | 4 +- .../transformation/inja_transformer_test.cc | 24 +++++++--- 3 files changed, 45 insertions(+), 28 deletions(-) diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index 0e38241cd..3ea73a5dd 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/source/extensions/filters/http/transformation/inja_transformer.cc @@ -143,8 +143,8 @@ TransformerInstance::TransformerInstance(ThreadLocal::Slot &tls, Envoy::Random:: env_.add_callback("replace_with_random", 2, [this](Arguments &args) { return replace_with_random_callback(args); }); - env_.add_callback("to_json", 1, [this](Arguments &args) { - return tojson_callback(args); + env_.add_callback("raw_string", 1, [this](Arguments &args) { + return raw_string_callback(args); }); } @@ -346,28 +346,31 @@ std::string& TransformerInstance::random_for_pattern(const std::string& pattern) return found->second; } -json TransformerInstance::tojson_callback(const inja::Arguments &args) const { +json TransformerInstance::raw_string_callback(const inja::Arguments &args) const { const std::string &input = args.at(0)->get_ref(); - std::cout << "modifying input: \n" << input << std::endl; + // in order to get the escaping the way we want, we must cast directly to a json object // since using parse will cause the surrounding " to be stripped - auto parsed = json(input); - std::cout << "modified input (as string): \n" << parsed.get() << std::endl; - - auto val = parsed.dump(); - std::cout << "modified input (as dump): \n" << val << std::endl; - - // This block is optional. It makes it such that a template must have surrounding - // " characters. This is logical to me since we expect the value we get out of the - // context (body) to be placed in exactly as-is. HOWEVER, the behavior of jinja - // is such that the quotes added by .dumps() are left in, so to mirror that impl - // exactly, we should not strip them. - - /* // strip the leading and trailing " characters that are added by dump() */ - /* // if C++20 is adopted, val.starts_with and val.ends_with would clean this up a bit */ - /* val = val.substr(0,1) == "\"" && val.substr(val.length()-1,1) == "\"" */ - /* ? val.substr(1, val.length()-2) */ - /* : val; */ + auto j = json(input); + + // make sure to bail if we're not working with a raw string value + if(!j.is_string()) { + return input; + } + + auto val = j.dump(); + + // This block makes it such that a template must have surrounding " characters + // around the raw string. This is reasonable since we expect the value we get out of the + // context (body) to be placed in exactly as-is. HOWEVER, the behavior of the jinja + // filter is such that the quotes added by .dumps() are left in. For that reason, + // this callback is NOT named to_json to avoid confusion with that behavior. + + // strip the leading and trailing " characters that are added by dump() + // if C++20 is adopted, val.starts_with and val.ends_with would clean this up a bit + val = val.substr(0,1) == "\"" && val.substr(val.length()-1,1) == "\"" + ? val.substr(1, val.length()-2) + : val; return val; } diff --git a/source/extensions/filters/http/transformation/inja_transformer.h b/source/extensions/filters/http/transformation/inja_transformer.h index ece26aa33..3c06964de 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.h +++ b/source/extensions/filters/http/transformation/inja_transformer.h @@ -65,7 +65,7 @@ class TransformerInstance { nlohmann::json substring_callback(const inja::Arguments &args) const; nlohmann::json replace_with_random_callback(const inja::Arguments &args); std::string& random_for_pattern(const std::string& pattern); - nlohmann::json tojson_callback(const inja::Arguments &args) const; + nlohmann::json raw_string_callback(const inja::Arguments &args) const; inja::Environment env_; absl::flat_hash_map pattern_replacements_; @@ -90,7 +90,7 @@ class Extractor : Logger::Loggable { const std::regex extract_regex_; }; -class InjaTransformer : public Transformer, Logger::Loggable { +class InjaTransformer : public Transformer { public: InjaTransformer(const envoy::api::v2::filter::http::TransformationTemplate &transformation, Envoy::Random::RandomGenerator &rng, diff --git a/test/extensions/filters/http/transformation/inja_transformer_test.cc b/test/extensions/filters/http/transformation/inja_transformer_test.cc index badfb5bfc..6ed41132d 100644 --- a/test/extensions/filters/http/transformation/inja_transformer_test.cc +++ b/test/extensions/filters/http/transformation/inja_transformer_test.cc @@ -1163,7 +1163,7 @@ TEST_F(InjaTransformerTest, RenderBodyAsJson) { EXPECT_EQ(body.toString(), expected_body); } -TEST_F(InjaTransformerTest, RenderBodyAsJsonPreEscapedInput) { +TEST_F(InjaTransformerTest, RenderBodyAsJsonDoubleEscapedInput) { Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; TransformationTemplate transformation; transformation.mutable_body()->set_text( @@ -1176,15 +1176,14 @@ TEST_F(InjaTransformerTest, RenderBodyAsJsonPreEscapedInput) { Buffer::OwnedImpl body(R"({"value":"\\\"foo\\\""})"_json.dump()); auto expected_body = R"({"Value":"\"foo\""})"_json.dump(); transformer.transform(headers, &headers, body, callbacks); - std::cout << expected_body << std::endl; EXPECT_EQ(body.toString(), expected_body); } -TEST_F(InjaTransformerTest, RenderBodyAsJsonWithCallback) { +TEST_F(InjaTransformerTest, RawStringCallback) { Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; TransformationTemplate transformation; transformation.mutable_body()->set_text( - R"EOF({"Value":{{ to_json(value) }}})EOF"); + R"EOF({"Value":"{{ raw_string(value) }}"})EOF"); transformation.set_render_body_as_json(false); InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); @@ -1193,7 +1192,22 @@ TEST_F(InjaTransformerTest, RenderBodyAsJsonWithCallback) { Buffer::OwnedImpl body(R"({"value":"\"foo\""})"_json.dump()); auto expected_body = R"({"Value":"\"foo\""})"_json.dump(); transformer.transform(headers, &headers, body, callbacks); - std::cout << "expected_body: " << expected_body << std::endl; + EXPECT_EQ(body.toString(), expected_body); +} + +TEST_F(InjaTransformerTest, RenderBodyAsJsonRawStringCallback) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + transformation.mutable_body()->set_text( + R"EOF({"Value":"{{ raw_string(value) }}"})EOF"); + transformation.set_render_body_as_json(true); + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + + Buffer::OwnedImpl body(R"({"value":"\"foo\""})"_json.dump()); + auto expected_body = R"({"Value":"\\\"foo\\\""})"_json.dump(); + transformer.transform(headers, &headers, body, callbacks); EXPECT_EQ(body.toString(), expected_body); } From 75e9bb2d8b6a6b4c7e658d1807fb6d8bc036c94a Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Thu, 20 Jul 2023 15:56:47 -0400 Subject: [PATCH 6/9] use const json * arg directly instead of getting string and casting to json --- .../http/transformation/inja_transformer.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index 3ea73a5dd..77428cadf 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/source/extensions/filters/http/transformation/inja_transformer.cc @@ -347,18 +347,16 @@ std::string& TransformerInstance::random_for_pattern(const std::string& pattern) } json TransformerInstance::raw_string_callback(const inja::Arguments &args) const { - const std::string &input = args.at(0)->get_ref(); - - // in order to get the escaping the way we want, we must cast directly to a json object - // since using parse will cause the surrounding " to be stripped - auto j = json(input); + // inja::Arguments is a std::vector, so we can get the json + // value from the args directly. + const auto& input = args.at(0); // make sure to bail if we're not working with a raw string value - if(!j.is_string()) { - return input; + if(!input->is_string()) { + return input->get_ref(); } - auto val = j.dump(); + auto val = input->dump(); // This block makes it such that a template must have surrounding " characters // around the raw string. This is reasonable since we expect the value we get out of the From a25c18d7cab560533a343b631463c60bedabde5c Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Thu, 20 Jul 2023 16:13:41 -0400 Subject: [PATCH 7/9] add proto comment; add integration tests and clean up tests --- .../v2/transformation_filter.proto | 2 +- .../transformation_filter_integration_test.cc | 96 +++++++++++++++++-- 2 files changed, 88 insertions(+), 10 deletions(-) diff --git a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto index cfe104852..076be8187 100644 --- a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto +++ b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto @@ -257,7 +257,7 @@ message TransformationTemplate { repeated DynamicMetadataValue dynamic_metadata_values = 9; // Use to render the output of a body transformation as JSON. This will cause - // rendered values to be escaped in order to make valid JSON strings + // rendered string values to be escaped in order to make valid JSON strings bool render_body_as_json = 12; } diff --git a/test/integration/transformation_filter_integration_test.cc b/test/integration/transformation_filter_integration_test.cc index 07a526a9a..6f70e35ab 100644 --- a/test/integration/transformation_filter_integration_test.cc +++ b/test/integration/transformation_filter_integration_test.cc @@ -527,13 +527,10 @@ TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransform) { auto actual_request_body = json::parse(upstream_request_->body().toString()); auto expected_request_body = R"({"Foo":"\"bar\""})"_json; - - // make sure response works as well, with no metadata set: EXPECT_EQ(expected_request_body, actual_request_body); - json actual_response_body = json::parse(response->body()); - // remove the `x-envoy-upstream-service-time` since its - // value depends on how long the test took to run + // make sure response works as well + auto actual_response_body = json::parse(response->body()); auto expected_response_body = R"({"Bar":"\"baz\""})"_json; EXPECT_EQ(expected_response_body, actual_response_body); } @@ -572,14 +569,95 @@ TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransformEscapedInpu auto actual_request_body = json::parse(upstream_request_->body().toString()); auto expected_request_body = R"({"Foo":"\"bar\""})"_json; + EXPECT_EQ(expected_request_body, actual_request_body); - // make sure response works as well, with no metadata set: + // make sure response works as well + auto actual_response_body = json::parse(response->body()); + auto expected_response_body = R"({"Bar":"\"baz\""})"_json; + EXPECT_EQ(expected_response_body, actual_response_body); +} + +TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonRawStringCallback) { + using json = nlohmann::json; + transformation_string_ = + R"EOF( + request_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: false + body: + text: "{\"Foo\":\"{{ raw_string(foo) }}\"}" + response_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: false + body: + text: "{\"Bar\":\"{{ raw_string(bar) }}\"}" +)EOF"; + initialize(); + std::string origReqBody = R"EOF({"foo":"\"bar\""})EOF"; + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":authority", "www.solo.io"}, + {":path", "/"}}; + auto encoder_decoder = codec_client_->startRequest(request_headers); + + auto downstream_request = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + + Buffer::OwnedImpl data(origReqBody); + codec_client_->sendData(*downstream_request, data, true); + + processRequest(response, R"EOF({"bar":"\"baz\""})EOF"); + + auto actual_request_body = json::parse(upstream_request_->body().toString()); + auto expected_request_body = R"({"Foo":"\"bar\""})"_json; EXPECT_EQ(expected_request_body, actual_request_body); - json actual_response_body = json::parse(response->body()); - // remove the `x-envoy-upstream-service-time` since its - // value depends on how long the test took to run + // make sure response works as well + auto actual_response_body = json::parse(response->body()); auto expected_response_body = R"({"Bar":"\"baz\""})"_json; EXPECT_EQ(expected_response_body, actual_response_body); } + +TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransformRawStringCallback) { + using json = nlohmann::json; + transformation_string_ = + R"EOF( + request_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: true + body: + text: "{\"Foo\":\"{{ raw_string(foo) }}\"}" + response_transformation: + transformation_template: + advanced_templates: false + render_body_as_json: true + body: + text: "{\"Bar\":\"{{ raw_string(bar) }}\"}" +)EOF"; + initialize(); + std::string origReqBody = R"EOF({"foo":"\"bar\""})EOF"; + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":authority", "www.solo.io"}, + {":path", "/"}}; + auto encoder_decoder = codec_client_->startRequest(request_headers); + + auto downstream_request = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + + Buffer::OwnedImpl data(origReqBody); + codec_client_->sendData(*downstream_request, data, true); + + processRequest(response, R"EOF({"bar":"\"baz\""})EOF"); + + auto actual_request_body = json::parse(upstream_request_->body().toString()); + auto expected_request_body = R"({"Foo":"\\\"bar\\\""})"_json; + EXPECT_EQ(expected_request_body, actual_request_body); + + // make sure response works as well + auto actual_response_body = json::parse(response->body()); + auto expected_response_body = R"({"Bar":"\\\"baz\\\""})"_json; + EXPECT_EQ(expected_response_body, actual_response_body); +} } // namespace Envoy From 601ebe3ee504f48873be4d99a6826f4b1d63b2d0 Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Thu, 20 Jul 2023 16:17:29 -0400 Subject: [PATCH 8/9] add changelog --- changelog/v1.26.3-patch2/inja-escaped-body.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changelog/v1.26.3-patch2/inja-escaped-body.yaml diff --git a/changelog/v1.26.3-patch2/inja-escaped-body.yaml b/changelog/v1.26.3-patch2/inja-escaped-body.yaml new file mode 100644 index 000000000..3bbfbbc18 --- /dev/null +++ b/changelog/v1.26.3-patch2/inja-escaped-body.yaml @@ -0,0 +1,11 @@ +changelog: +- type: DEPENDENCY_BUMP + dependencyOwner: solo-io + dependencyRepo: inja + dependencyTag: v3.4.0-patch2 # not yet released DO NOT MERGE +- type: NEW_FEATURE + issueLink: https://github.com/solo-io/solo-projects/issues/5155 + resolvesIssue: false + description: >- + Inja callback `raw_string` added to permit fine-grained escaping of input strings. + Also, configuration added to escape all strings for a given transformation. From 46f2a5f2597f208270d96a91568d61c2c17550a0 Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Fri, 21 Jul 2023 15:46:56 -0400 Subject: [PATCH 9/9] PR review feedback add comments; rename new proto field; add tests for nested json --- .../v2/transformation_filter.proto | 6 +- .../http/transformation/inja_transformer.cc | 8 ++- .../http/transformation/inja_transformer.h | 3 +- .../transformation/inja_transformer_test.cc | 45 +++++++++++-- .../transformation_filter_integration_test.cc | 67 +++++++++++++++---- 5 files changed, 103 insertions(+), 26 deletions(-) diff --git a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto index 076be8187..facab9f42 100644 --- a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto +++ b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto @@ -256,9 +256,9 @@ message TransformationTemplate { // Use this field to set Dynamic Metadata. repeated DynamicMetadataValue dynamic_metadata_values = 9; - // 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; + // Use to escape the output of a body transformation. This will cause + // rendered string values to be escaped in order to make valid JSON/YAML strings + bool escape_characters = 12; } diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index 77428cadf..2767dc8c0 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/source/extensions/filters/http/transformation/inja_transformer.cc @@ -348,7 +348,9 @@ std::string& TransformerInstance::random_for_pattern(const std::string& pattern) json TransformerInstance::raw_string_callback(const inja::Arguments &args) const { // inja::Arguments is a std::vector, so we can get the json - // value from the args directly. + // value from the args directly. We are guaranteed to have exactly one argument + // because Inja will throw a Parser error in any other case. + // https://github.com/pantor/inja/blob/v3.4.0/include/inja/parser.hpp#L228-L231 const auto& input = args.at(0); // make sure to bail if we're not working with a raw string value @@ -401,14 +403,14 @@ InjaTransformer::InjaTransformer(const TransformationTemplate &transformation, passthrough_body_(transformation.has_passthrough()), parse_body_behavior_(transformation.parse_body_behavior()), ignore_error_on_parse_(transformation.ignore_error_on_parse()), - render_body_as_json_(transformation.render_body_as_json()), + escape_characters_(transformation.escape_characters()), tls_(tls.allocateSlot()), instance_(std::make_unique(*tls_, rng)) { if (advanced_templates_) { instance_->set_element_notation(inja::ElementNotation::Pointer); } - instance_->set_escape_strings(render_body_as_json_); + instance_->set_escape_strings(escape_characters_); tls_->set([](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { return std::make_shared(); diff --git a/source/extensions/filters/http/transformation/inja_transformer.h b/source/extensions/filters/http/transformation/inja_transformer.h index 3c06964de..4dbfe0e11 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.h +++ b/source/extensions/filters/http/transformation/inja_transformer.h @@ -47,6 +47,7 @@ class TransformerInstance { void set_element_notation(inja::ElementNotation notation) { env_.set_element_notation(notation); }; + // Sets the config for rendering strings raw or unescaped void set_escape_strings(bool escape_strings) { env_.set_escape_strings(escape_strings); }; @@ -123,7 +124,7 @@ class InjaTransformer : public Transformer { envoy::api::v2::filter::http::TransformationTemplate::RequestBodyParse parse_body_behavior_; bool ignore_error_on_parse_; - bool render_body_as_json_{}; + bool escape_characters_{}; absl::optional body_template_; bool merged_extractors_to_body_{}; diff --git a/test/extensions/filters/http/transformation/inja_transformer_test.cc b/test/extensions/filters/http/transformation/inja_transformer_test.cc index 6ed41132d..367ca6a85 100644 --- a/test/extensions/filters/http/transformation/inja_transformer_test.cc +++ b/test/extensions/filters/http/transformation/inja_transformer_test.cc @@ -1147,12 +1147,12 @@ TEST_F(InjaTransformerTest, ParseUsingJsonPointerSyntax) { EXPECT_EQ(body.toString(), "online--slash--tilde"); } -TEST_F(InjaTransformerTest, RenderBodyAsJson) { +TEST_F(InjaTransformerTest, EscapeCharacters) { Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; TransformationTemplate transformation; transformation.mutable_body()->set_text( R"EOF({"Value":"{{ value }}"})EOF"); - transformation.set_render_body_as_json(true); + transformation.set_escape_characters(true); InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); NiceMock callbacks; @@ -1163,12 +1163,28 @@ TEST_F(InjaTransformerTest, RenderBodyAsJson) { EXPECT_EQ(body.toString(), expected_body); } -TEST_F(InjaTransformerTest, RenderBodyAsJsonDoubleEscapedInput) { +TEST_F(InjaTransformerTest, EscapeCharactersNestedJson) { Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; TransformationTemplate transformation; transformation.mutable_body()->set_text( R"EOF({"Value":"{{ value }}"})EOF"); - transformation.set_render_body_as_json(false); + transformation.set_escape_characters(true); + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + + NiceMock callbacks; + + Buffer::OwnedImpl body(R"({"value":"{\"foo\":{\"bar\":\"\\\"baz\\\"\"}}"})"_json.dump()); + auto expected_body = R"({"Value":"{\"foo\":{\"bar\":\"\\\"baz\\\"\"}}"})"_json.dump(); + transformer.transform(headers, &headers, body, callbacks); + EXPECT_EQ(body.toString(), expected_body); +} + +TEST_F(InjaTransformerTest, EscapeCharactersDoubleEscapedInput) { + Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; + TransformationTemplate transformation; + transformation.mutable_body()->set_text( + R"EOF({"Value":"{{ value }}"})EOF"); + transformation.set_escape_characters(false); InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); NiceMock callbacks; @@ -1184,7 +1200,7 @@ TEST_F(InjaTransformerTest, RawStringCallback) { TransformationTemplate transformation; transformation.mutable_body()->set_text( R"EOF({"Value":"{{ raw_string(value) }}"})EOF"); - transformation.set_render_body_as_json(false); + transformation.set_escape_characters(false); InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); NiceMock callbacks; @@ -1195,12 +1211,27 @@ TEST_F(InjaTransformerTest, RawStringCallback) { EXPECT_EQ(body.toString(), expected_body); } -TEST_F(InjaTransformerTest, RenderBodyAsJsonRawStringCallback) { +TEST_F(InjaTransformerTest, RawStringCallbackTooManyArguments) { + TransformationTemplate transformation; + transformation.mutable_body()->set_text( + R"EOF({% set bad="extra argument to function" %}{"Value":"{{ raw_string(value, bad) }}"})EOF"); + EXPECT_THROW_WITH_REGEX(InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_), EnvoyException, ".*unknown function raw_string.*") +} + +TEST_F(InjaTransformerTest, RawStringCallbackZeroArguments) { + TransformationTemplate transformation; + transformation.mutable_body()->set_text( + R"EOF({"Value":"{{ raw_string() }}"})EOF"); + EXPECT_THROW_WITH_REGEX(InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_), EnvoyException, ".*unknown function raw_string.*") +} + + +TEST_F(InjaTransformerTest, EscapeCharactersRawStringCallback) { Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}}; TransformationTemplate transformation; transformation.mutable_body()->set_text( R"EOF({"Value":"{{ raw_string(value) }}"})EOF"); - transformation.set_render_body_as_json(true); + transformation.set_escape_characters(true); InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); NiceMock callbacks; diff --git a/test/integration/transformation_filter_integration_test.cc b/test/integration/transformation_filter_integration_test.cc index 6f70e35ab..15da9a4d6 100644 --- a/test/integration/transformation_filter_integration_test.cc +++ b/test/integration/transformation_filter_integration_test.cc @@ -493,20 +493,63 @@ TEST_P(TransformationFilterIntegrationTest, BodyHeaderTransform) { } -TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransform) { +TEST_P(TransformationFilterIntegrationTest, EscapeCharactersTransformNestedJson) { using json = nlohmann::json; transformation_string_ = R"EOF( request_transformation: transformation_template: advanced_templates: false - render_body_as_json: true + escape_characters: true body: text: "{\"Foo\":\"{{ foo }}\"}" response_transformation: transformation_template: advanced_templates: false - render_body_as_json: true + escape_characters: true + body: + text: "{\"Bar\":\"{{ bar }}\"}" +)EOF"; + initialize(); + std::string origReqBody = R"EOF({"foo":"{\"bar\":{\"bat\":\"\\\"baq\\\"\"}}"})EOF"; + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":authority", "www.solo.io"}, + {":path", "/"}}; + auto encoder_decoder = codec_client_->startRequest(request_headers); + + auto downstream_request = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + + Buffer::OwnedImpl data(origReqBody); + codec_client_->sendData(*downstream_request, data, true); + + processRequest(response, R"EOF({"bar":"{\"bat\":{\"baq\":\"\\\"baz\\\"\"}}"})EOF"); + + auto actual_request_body = json::parse(upstream_request_->body().toString()); + auto expected_request_body = R"({"Foo":"{\"bar\":{\"bat\":\"\\\"baq\\\"\"}}"})"_json; + EXPECT_EQ(expected_request_body, actual_request_body); + + // make sure response works as well + auto actual_response_body = json::parse(response->body()); + auto expected_response_body = R"({"Bar":"{\"bat\":{\"baq\":\"\\\"baz\\\"\"}}"})"_json; + EXPECT_EQ(expected_response_body, actual_response_body); +} + + +TEST_P(TransformationFilterIntegrationTest, EscapeCharactersTransform) { + using json = nlohmann::json; + transformation_string_ = + R"EOF( + request_transformation: + transformation_template: + advanced_templates: false + escape_characters: true + body: + text: "{\"Foo\":\"{{ foo }}\"}" + response_transformation: + transformation_template: + advanced_templates: false + escape_characters: true body: text: "{\"Bar\":\"{{ bar }}\"}" )EOF"; @@ -535,20 +578,20 @@ TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransform) { EXPECT_EQ(expected_response_body, actual_response_body); } -TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransformEscapedInput) { +TEST_P(TransformationFilterIntegrationTest, EscapeCharactersTransformEscapedInput) { using json = nlohmann::json; transformation_string_ = R"EOF( request_transformation: transformation_template: advanced_templates: false - render_body_as_json: false + escape_characters: false body: text: "{\"Foo\":\"{{ foo }}\"}" response_transformation: transformation_template: advanced_templates: false - render_body_as_json: false + escape_characters: false body: text: "{\"Bar\":\"{{ bar }}\"}" )EOF"; @@ -577,20 +620,20 @@ TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransformEscapedInpu EXPECT_EQ(expected_response_body, actual_response_body); } -TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonRawStringCallback) { +TEST_P(TransformationFilterIntegrationTest, EscapeCharactersRawStringCallback) { using json = nlohmann::json; transformation_string_ = R"EOF( request_transformation: transformation_template: advanced_templates: false - render_body_as_json: false + escape_characters: false body: text: "{\"Foo\":\"{{ raw_string(foo) }}\"}" response_transformation: transformation_template: advanced_templates: false - render_body_as_json: false + escape_characters: false body: text: "{\"Bar\":\"{{ raw_string(bar) }}\"}" )EOF"; @@ -619,20 +662,20 @@ TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonRawStringCallback) { EXPECT_EQ(expected_response_body, actual_response_body); } -TEST_P(TransformationFilterIntegrationTest, RenderBodyAsJsonTransformRawStringCallback) { +TEST_P(TransformationFilterIntegrationTest, EscapeCharactersTransformRawStringCallback) { using json = nlohmann::json; transformation_string_ = R"EOF( request_transformation: transformation_template: advanced_templates: false - render_body_as_json: true + escape_characters: true body: text: "{\"Foo\":\"{{ raw_string(foo) }}\"}" response_transformation: transformation_template: advanced_templates: false - render_body_as_json: true + escape_characters: true body: text: "{\"Bar\":\"{{ raw_string(bar) }}\"}" )EOF";