Merge pull request #8630 from ctiller/newlines

Support percent-encoding status messages

4 files changed, 84 insertions, 14 deletions

diff --git a/src/core/lib/channel/http_client_filter.c b/src/core/lib/channel/http_client_filter.c
index f57d7c2453..dbe0d25211 100644
--- a/src/core/lib/channel/http_client_filter.c
+++ b/src/core/lib/channel/http_client_filter.c
@@ -36,6 +36,7 @@
 #include <grpc/support/string_util.h>
 #include <string.h>
 #include "src/core/lib/profiling/timers.h"
+#include "src/core/lib/slice/percent_encoding.h"
 #include "src/core/lib/support/string.h"
 #include "src/core/lib/transport/static_metadata.h"
 #include "src/core/lib/transport/transport_impl.h"
@@ -56,6 +57,7 @@ typedef struct call_data {
   grpc_linked_mdelem payload_bin;
 
   grpc_metadata_batch *recv_initial_metadata;
+  grpc_metadata_batch *recv_trailing_metadata;
   uint8_t *payload_bytes;
 
   /* Vars to read data off of send_message */
@@ -69,14 +71,16 @@ typedef struct call_data {
   bool send_message_blocked;
 
   /** Closure to call when finished with the hc_on_recv hook */
-  grpc_closure *on_done_recv;
+  grpc_closure *on_done_recv_initial_metadata;
+  grpc_closure *on_done_recv_trailing_metadata;
   grpc_closure *on_complete;
   grpc_closure *post_send;
 
   /** Receive closures are chained: we inject this closure as the on_done_recv
       up-call on transport_op, and remember to call our on_done_recv member
       after handling it. */
-  grpc_closure hc_on_recv;
+  grpc_closure hc_on_recv_initial_metadata;
+  grpc_closure hc_on_recv_trailing_metadata;
   grpc_closure hc_on_complete;
   grpc_closure got_slice;
   grpc_closure send_done;
@@ -106,6 +110,16 @@ static grpc_mdelem *client_recv_filter(void *user_data, grpc_mdelem *md) {
     grpc_call_element_send_close_with_message(a->exec_ctx, a->elem,
                                               GRPC_STATUS_CANCELLED, &message);
     return NULL;
+  } else if (md->key == GRPC_MDSTR_GRPC_MESSAGE) {
+    grpc_slice pct_decoded_msg =
+        grpc_permissive_percent_decode_slice(md->value->slice);
+    if (grpc_slice_is_equivalent(pct_decoded_msg, md->value->slice)) {
+      grpc_slice_unref(pct_decoded_msg);
+      return md;
+    } else {
+      return grpc_mdelem_from_metadata_strings(
+          GRPC_MDSTR_GRPC_MESSAGE, grpc_mdstr_from_slice(pct_decoded_msg));
+    }
   } else if (md == GRPC_MDELEM_CONTENT_TYPE_APPLICATION_SLASH_GRPC) {
     return NULL;
   } else if (md->key == GRPC_MDSTR_CONTENT_TYPE) {
@@ -129,8 +143,8 @@ static grpc_mdelem *client_recv_filter(void *user_data, grpc_mdelem *md) {
   return md;
 }
 
-static void hc_on_recv(grpc_exec_ctx *exec_ctx, void *user_data,
-                       grpc_error *error) {
+static void hc_on_recv_initial_metadata(grpc_exec_ctx *exec_ctx,
+                                        void *user_data, grpc_error *error) {
   grpc_call_element *elem = user_data;
   call_data *calld = elem->call_data;
   client_recv_filter_args a;
@@ -138,7 +152,21 @@ static void hc_on_recv(grpc_exec_ctx *exec_ctx, void *user_data,
   a.exec_ctx = exec_ctx;
   grpc_metadata_batch_filter(calld->recv_initial_metadata, client_recv_filter,
                              &a);
-  calld->on_done_recv->cb(exec_ctx, calld->on_done_recv->cb_arg, error);
+  grpc_closure_run(exec_ctx, calld->on_done_recv_initial_metadata,
+                   GRPC_ERROR_REF(error));
+}
+
+static void hc_on_recv_trailing_metadata(grpc_exec_ctx *exec_ctx,
+                                         void *user_data, grpc_error *error) {
+  grpc_call_element *elem = user_data;
+  call_data *calld = elem->call_data;
+  client_recv_filter_args a;
+  a.elem = elem;
+  a.exec_ctx = exec_ctx;
+  grpc_metadata_batch_filter(calld->recv_trailing_metadata, client_recv_filter,
+                             &a);
+  grpc_closure_run(exec_ctx, calld->on_done_recv_trailing_metadata,
+                   GRPC_ERROR_REF(error));
 }
 
 static void hc_on_complete(grpc_exec_ctx *exec_ctx, void *user_data,
@@ -281,8 +309,15 @@ static void hc_mutate_op(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
   if (op->recv_initial_metadata != NULL) {
     /* substitute our callback for the higher callback */
     calld->recv_initial_metadata = op->recv_initial_metadata;
-    calld->on_done_recv = op->recv_initial_metadata_ready;
-    op->recv_initial_metadata_ready = &calld->hc_on_recv;
+    calld->on_done_recv_initial_metadata = op->recv_initial_metadata_ready;
+    op->recv_initial_metadata_ready = &calld->hc_on_recv_initial_metadata;
+  }
+
+  if (op->recv_trailing_metadata != NULL) {
+    /* substitute our callback for the higher callback */
+    calld->recv_trailing_metadata = op->recv_trailing_metadata;
+    calld->on_done_recv_trailing_metadata = op->on_complete;
+    op->on_complete = &calld->hc_on_recv_trailing_metadata;
   }
 }
 
@@ -308,11 +343,15 @@ static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx,
                                   grpc_call_element *elem,
                                   grpc_call_element_args *args) {
   call_data *calld = elem->call_data;
-  calld->on_done_recv = NULL;
+  calld->on_done_recv_initial_metadata = NULL;
+  calld->on_done_recv_trailing_metadata = NULL;
   calld->on_complete = NULL;
   calld->payload_bytes = NULL;
   grpc_slice_buffer_init(&calld->slices);
-  grpc_closure_init(&calld->hc_on_recv, hc_on_recv, elem);
+  grpc_closure_init(&calld->hc_on_recv_initial_metadata,
+                    hc_on_recv_initial_metadata, elem);
+  grpc_closure_init(&calld->hc_on_recv_trailing_metadata,
+                    hc_on_recv_trailing_metadata, elem);
   grpc_closure_init(&calld->hc_on_complete, hc_on_complete, elem);
   grpc_closure_init(&calld->got_slice, got_slice, elem);
   grpc_closure_init(&calld->send_done, send_done, elem);
diff --git a/src/core/lib/channel/http_server_filter.c b/src/core/lib/channel/http_server_filter.c
index 6a33689fec..b42ff06039 100644
--- a/src/core/lib/channel/http_server_filter.c
+++ b/src/core/lib/channel/http_server_filter.c
@@ -37,6 +37,7 @@
 #include <grpc/support/log.h>
 #include <string.h>
 #include "src/core/lib/profiling/timers.h"
+#include "src/core/lib/slice/percent_encoding.h"
 #include "src/core/lib/transport/static_metadata.h"
 
 #define EXPECTED_CONTENT_TYPE "application/grpc"
@@ -86,6 +87,23 @@ typedef struct {
   grpc_exec_ctx *exec_ctx;
 } server_filter_args;
 
+static grpc_mdelem *server_filter_outgoing_metadata(void *user_data,
+                                                    grpc_mdelem *md) {
+  if (md->key == GRPC_MDSTR_GRPC_MESSAGE) {
+    grpc_slice pct_encoded_msg = grpc_percent_encode_slice(
+        md->value->slice, grpc_compatible_percent_encoding_unreserved_bytes);
+    if (grpc_slice_is_equivalent(pct_encoded_msg, md->value->slice)) {
+      grpc_slice_unref(pct_encoded_msg);
+      return md;
+    } else {
+      return grpc_mdelem_from_metadata_strings(
+          GRPC_MDSTR_GRPC_MESSAGE, grpc_mdstr_from_slice(pct_encoded_msg));
+    }
+  } else {
+    return md;
+  }
+}
+
 static grpc_mdelem *server_filter(void *user_data, grpc_mdelem *md) {
   server_filter_args *a = user_data;
   grpc_call_element *elem = a->elem;
@@ -254,7 +272,7 @@ static void hs_recv_message_ready(grpc_exec_ctx *exec_ctx, void *user_data,
   }
 }
 
-static void hs_mutate_op(grpc_call_element *elem,
+static void hs_mutate_op(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
                          grpc_transport_stream_op *op) {
   /* grab pointers to our data from the call element */
   call_data *calld = elem->call_data;
@@ -290,6 +308,12 @@ static void hs_mutate_op(grpc_call_element *elem,
       op->on_complete = &calld->hs_on_complete;
     }
   }
+
+  if (op->send_trailing_metadata) {
+    server_filter_args a = {elem, exec_ctx};
+    grpc_metadata_batch_filter(op->send_trailing_metadata,
+                               server_filter_outgoing_metadata, &a);
+  }
 }
 
 static void hs_start_transport_op(grpc_exec_ctx *exec_ctx,
@@ -297,7 +321,7 @@ static void hs_start_transport_op(grpc_exec_ctx *exec_ctx,
                                   grpc_transport_stream_op *op) {
   GRPC_CALL_LOG_OP(GPR_INFO, elem, op);
   GPR_TIMER_BEGIN("hs_start_transport_op", 0);
-  hs_mutate_op(elem, op);
+  hs_mutate_op(exec_ctx, elem, op);
   grpc_call_next_op(exec_ctx, elem, op);
   GPR_TIMER_END("hs_start_transport_op", 0);
 }
diff --git a/src/core/lib/slice/slice.c b/src/core/lib/slice/slice.c
index 3dac18df61..52977e6d9a 100644
--- a/src/core/lib/slice/slice.c
+++ b/src/core/lib/slice/slice.c
@@ -348,3 +348,11 @@ int grpc_slice_str_cmp(grpc_slice a, const char *b) {
   if (d != 0) return d;
   return memcmp(GRPC_SLICE_START_PTR(a), b, b_length);
 }
+
+int grpc_slice_is_equivalent(grpc_slice a, grpc_slice b) {
+  if (a.refcount == NULL || b.refcount == NULL) {
+    return grpc_slice_cmp(a, b) == 0;
+  }
+  return a.data.refcounted.length == b.data.refcounted.length &&
+         a.data.refcounted.bytes == b.data.refcounted.bytes;
+}
diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c
index 8d2386e6a0..1e0f3eeca5 100644
--- a/src/core/lib/surface/call.c
+++ b/src/core/lib/surface/call.c
@@ -637,9 +637,6 @@ static int prepare_application_metadata(grpc_call *call, int count,
     if (call->send_extra_metadata_count == 0) {
       prepend_extra_metadata = 0;
     } else {
-      for (i = 0; i < call->send_extra_metadata_count; i++) {
-        GRPC_MDELEM_REF(call->send_extra_metadata[i].md);
-      }
       for (i = 1; i < call->send_extra_metadata_count; i++) {
         call->send_extra_metadata[i].prev = &call->send_extra_metadata[i - 1];
       }
@@ -685,6 +682,7 @@ static int prepare_application_metadata(grpc_call *call, int count,
           &call->send_extra_metadata[call->send_extra_metadata_count - 1];
       batch->list.head->prev = NULL;
       batch->list.tail->next = NULL;
+      call->send_extra_metadata_count = 0;
       break;
     case 3: {
       /* prepend AND md */
@@ -700,6 +698,7 @@ static int prepare_application_metadata(grpc_call *call, int count,
       batch->list.tail = linked_from_md(last_md);
       batch->list.head->prev = NULL;
       batch->list.tail->next = NULL;
+      call->send_extra_metadata_count = 0;
       break;
     }
     default: