http: Change sendLocalReply to send percent-encoded GrpcMessage

docs/root/intro/version_history.rst

@@ -15,6 +15,7 @@ Version history
 * http: fixed a bug where large unbufferable responses were not tracked in stats and logs correctly.
 * http: fixed a crashing bug where gRPC local replies would cause segfaults when upstream access logging was on.
 * http: mitigated a race condition with the :ref:`delayed_close_timeout<envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.delayed_close_timeout>` where it could trigger while actively flushing a pending write buffer for a downstream connection.
+* http: changed `sendLocalReply` to send percent-encoded `GrpcMessage`.
 * jwt_authn: make filter's parsing of JWT more flexible, allowing syntax like ``jwt=eyJhbGciOiJS...ZFnFIw,extra=7,realm=123``
 * redis: add support for Redis cluster custom cluster type.
 * redis: added :ref:`prefix routing <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.prefix_routes>` to enable routing commands based on their key's prefix to different upstream.

source/common/http/utility.cc

@@ -322,8 +322,9 @@ void Utility::sendLocalReply(
              enumToInt(grpc_status ? grpc_status.value()
                                    : Grpc::Utility::httpToGrpcStatus(enumToInt(response_code))))}}};
     if (!body_text.empty() && !is_head_request) {
-      // TODO: GrpcMessage should be percent-encoded
-      response_headers->insertGrpcMessage().value(body_text);
+      // TODO(dio): Probably it is worth to consider caching the encoded message based on gRPC
+      // status.
+      response_headers->insertGrpcMessage().value(PercentEncoding::encode(body_text));
     }
     encode_headers(std::move(response_headers), true); // Trailers only response
     return;
@@ -562,5 +563,62 @@ void Utility::traversePerFilterConfigGeneric(
   }
 }
 
+std::string Utility::PercentEncoding::encode(absl::string_view value) {
+  for (size_t i = 0; i < value.size(); ++i) {
+    const char& ch = value[i];
+    // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses.
+    if (ch < ' ' || ch >= '~' || ch == '%') {
+      return PercentEncoding::encode(value, i);
+    }
+  }
+  return std::string(value);
+}
+
+std::string Utility::PercentEncoding::encode(absl::string_view value, const size_t index) {
+  std::string encoded;
+  if (index > 0) {
+    absl::StrAppend(&encoded, value.substr(0, index - 1));
+  }
+
+  for (size_t i = index; i < value.size(); ++i) {
+    const char& ch = value[i];
+    if (ch < ' ' || ch >= '~' || ch == '%') {
+      // For consistency, URI producers should use uppercase hexadecimal digits for all
+      // percent-encodings. https://tools.ietf.org/html/rfc3986#section-2.1.
+      absl::StrAppend(&encoded, fmt::format("%{:02X}", ch));
+    } else {
+      encoded.push_back(ch);
+    }
+  }
+  return encoded;
+}
+
+std::string Utility::PercentEncoding::decode(absl::string_view encoded) {
+  std::string decoded;
+  decoded.reserve(encoded.size());
+  for (size_t i = 0; i < encoded.size(); ++i) {
+    char ch = encoded[i];
+    if (ch == '%' && i + 2 < encoded.size()) {
+      const char& hi = encoded[i + 1];
+      const char& lo = encoded[i + 2];
+      if (absl::ascii_isdigit(hi)) {
+        ch = hi - '0';
+      } else {
+        ch = absl::ascii_toupper(hi) - 'A' + 10;
+      }
+
+      ch *= 16;
+      if (absl::ascii_isdigit(lo)) {
+        ch += lo - '0';
+      } else {
+        ch += absl::ascii_toupper(lo) - 'A' + 10;
+      }
+      i += 2;
+    }
+    decoded.push_back(ch);
+  }
+  return decoded;
+}
+
 } // namespace Http
 } // namespace Envoy

source/common/http/utility.h

@@ -39,6 +39,28 @@ class Url {
   absl::string_view path_and_query_params_;
 };
 
+class PercentEncoding {
+public:
+  /**
+   * Encodes string view to its percent encoded representation.
+   * @param value supplies string to be encoded.
+   * @return std::string percent-encoded string based on
+   * https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses.
+   */
+  static std::string encode(absl::string_view value);
+
+  /**
+   * Decodes string view from its percent encoded representation.
+   * @param encoded supplies string to be decoded.
+   * @return std::string decoded string https://tools.ietf.org/html/rfc3986#section-2.1.
+   */
+  static std::string decode(absl::string_view value);
+
+private:
+  // Encodes string view to its percent encoded representation, with start index.
+  static std::string encode(absl::string_view value, const size_t index);
+};
+
 /**
  * Append to x-forwarded-for header.
  * @param headers supplies the headers to append to.

test/common/http/utility_test.cc

@@ -478,6 +478,33 @@ TEST(HttpUtility, SendLocalGrpcReply) {
                           absl::nullopt, false);
 }
 
+TEST(HttpUtility, SendLocalGrpcReplyWithUpstreamJsonPayload) {
+  MockStreamDecoderFilterCallbacks callbacks;
+  bool is_reset = false;
+
+  std::string json = R"EOF(
+{
+    "error": {
+        "code": 401,
+        "message": "Unauthorized"
+    }
+}
+  )EOF";
+
+  EXPECT_CALL(callbacks, encodeHeaders_(_, true))
+      .WillOnce(Invoke([&](const HeaderMap& headers, bool) -> void {
+        EXPECT_EQ(headers.Status()->value().getStringView(), "200");
+        EXPECT_NE(headers.GrpcStatus(), nullptr);
+        EXPECT_EQ(headers.GrpcStatus()->value().getStringView(),
+                  std::to_string(enumToInt(Grpc::Status::GrpcStatus::Unauthenticated)));
+        EXPECT_NE(headers.GrpcMessage(), nullptr);
+        const auto& encoded = Utility::PercentEncoding::encode(json);
+        EXPECT_EQ(headers.GrpcMessage()->value().getStringView(), encoded);
+      }));
+  Utility::sendLocalReply(true, callbacks, is_reset, Http::Code::Unauthorized, json, absl::nullopt,
+                          false);
+}
+
 TEST(HttpUtility, RateLimitedGrpcStatus) {
   MockStreamDecoderFilterCallbacks callbacks;
 
@@ -810,5 +837,32 @@ TEST(Url, ParsingTest) {
               "/path?query=param&query2=param2#fragment");
 }
 
+void validatePercentEncodingEncodeDecode(absl::string_view source,
+                                         absl::string_view expected_encoded) {
+  EXPECT_EQ(Utility::PercentEncoding::encode(source), expected_encoded);
+  EXPECT_EQ(Utility::PercentEncoding::decode(expected_encoded), source);
+}
+
+TEST(PercentEncoding, EncodeDecode) {
+  const std::string json = R"EOF(
+{
+    "error": {
+        "code": 401,
+        "message": "Unauthorized"
+    }
+}
+  )EOF";
+  validatePercentEncodingEncodeDecode(json, "%0A{%0A    \"error\": {%0A        \"code\": 401,%0A   "
+                                            "     \"message\": \"Unauthorized\"%0A    }%0A}%0A  ");
+  validatePercentEncodingEncodeDecode("too large", "too large");
+  validatePercentEncodingEncodeDecode("_-ok-_", "_-ok-_");
+}
+
+TEST(PercentEncoding, Trailing) {
+  EXPECT_EQ(Utility::PercentEncoding::decode("too%20lar%20"), "too lar ");
+  EXPECT_EQ(Utility::PercentEncoding::decode("too%20larg%e"), "too larg%e");
+  EXPECT_EQ(Utility::PercentEncoding::decode("too%20large%"), "too large%");
+}
+
 } // namespace Http
 } // namespace Envoy

test/extensions/filters/http/grpc_http1_reverse_bridge/reverse_bridge_test.cc

@@ -4,6 +4,7 @@
 #include "common/buffer/buffer_impl.h"
 #include "common/grpc/codec.h"
 #include "common/http/header_map_impl.h"
+#include "common/http/utility.h"
 
 #include "extensions/filters/http/grpc_http1_reverse_bridge/filter.h"
 #include "extensions/filters/http/well_known_names.h"
@@ -67,7 +68,9 @@ TEST_F(ReverseBridgeTest, InvalidGrcpRequest) {
     EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).WillOnce(Invoke([](auto& headers, auto) {
       EXPECT_THAT(headers, HeaderValueOf(Http::Headers::get().Status, "200"));
       EXPECT_THAT(headers, HeaderValueOf(Http::Headers::get().GrpcStatus, "2"));
-      EXPECT_THAT(headers, HeaderValueOf(Http::Headers::get().GrpcMessage, "invalid request body"));
+      EXPECT_THAT(headers,
+                  HeaderValueOf(Http::Headers::get().GrpcMessage,
+                                Http::Utility::PercentEncoding::encode("invalid request body")));
     }));
     EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, filter_->decodeData(buffer, false));
     EXPECT_EQ(decoder_callbacks_.details_, "grpc_bridge_data_too_small");