Fix some auth filtering bugs

4 files changed, 66 insertions, 13 deletions

diff --git a/src/core/lib/security/transport/client_auth_filter.c b/src/core/lib/security/transport/client_auth_filter.c
index 43054bcfb4..80fdbe68eb 100644
--- a/src/core/lib/security/transport/client_auth_filter.c
+++ b/src/core/lib/security/transport/client_auth_filter.c
@@ -102,7 +102,13 @@ static void bubble_up_error(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
   grpc_call_next_op(exec_ctx, elem, &calld->op);
 }
 
-static void add_error(grpc_error **combined, grpc_error *error) { abort(); }
+static void add_error(grpc_error **combined, grpc_error *error) {
+  if (error == GRPC_ERROR_NONE) return;
+  if (*combined == GRPC_ERROR_NONE) {
+    *combined = GRPC_ERROR_CREATE("Client auth metadata plugin error");
+  }
+  *combined = grpc_error_add_child(*combined, error);
+}
 
 static void on_credentials_metadata(grpc_exec_ctx *exec_ctx, void *user_data,
                                     grpc_credentials_md *md_elems,
diff --git a/src/core/lib/security/transport/server_auth_filter.c b/src/core/lib/security/transport/server_auth_filter.c
index ed9d92b74e..34f5bbf8f4 100644
--- a/src/core/lib/security/transport/server_auth_filter.c
+++ b/src/core/lib/security/transport/server_auth_filter.c
@@ -83,9 +83,9 @@ static grpc_metadata_array metadata_batch_to_md_array(
   return result;
 }
 
-#if 0
-static grpc_mdelem remove_consumed_md(grpc_exec_ctx *exec_ctx, void *user_data,
-                                      grpc_mdelem md) {
+static grpc_filtered_mdelem remove_consumed_md(grpc_exec_ctx *exec_ctx,
+                                               void *user_data,
+                                               grpc_mdelem md) {
   grpc_call_element *elem = user_data;
   call_data *calld = elem->call_data;
   size_t i;
@@ -93,11 +93,10 @@ static grpc_mdelem remove_consumed_md(grpc_exec_ctx *exec_ctx, void *user_data,
     const grpc_metadata *consumed_md = &calld->consumed_md[i];
     if (grpc_slice_eq(GRPC_MDKEY(md), consumed_md->key) &&
         grpc_slice_eq(GRPC_MDVALUE(md), consumed_md->value))
-      return GRPC_MDNULL;
+      return GRPC_FILTERED_REMOVE();
   }
-  return md;
+  return GRPC_FILTERED_MDELEM(md);
 }
-#endif
 
 static void destroy_op(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
   gpr_free(arg);
@@ -122,12 +121,12 @@ static void on_md_processing_done(
   if (status == GRPC_STATUS_OK) {
     calld->consumed_md = consumed_md;
     calld->num_consumed_md = num_consumed_md;
-#if 0
-    grpc_metadata_batch_filter(&exec_ctx, calld->recv_initial_metadata,
-                               remove_consumed_md, elem);
-#else
-    if (num_consumed_md) abort();
-#endif
+    /* TODO(ctiller): propagate error */
+    GRPC_LOG_IF_ERROR(
+        "grpc_metadata_batch_filter",
+        grpc_metadata_batch_filter(&exec_ctx, calld->recv_initial_metadata,
+                                   remove_consumed_md, elem,
+                                   "Response metadata filtering error"));
     grpc_metadata_array_destroy(&calld->md);
     grpc_exec_ctx_sched(&exec_ctx, calld->on_done_recv, GRPC_ERROR_NONE, NULL);
   } else {
diff --git a/src/core/lib/transport/metadata_batch.c b/src/core/lib/transport/metadata_batch.c
index a83f0e0f34..07b0dd3725 100644
--- a/src/core/lib/transport/metadata_batch.c
+++ b/src/core/lib/transport/metadata_batch.c
@@ -285,3 +285,33 @@ size_t grpc_metadata_batch_size(grpc_metadata_batch *batch) {
   }
   return size;
 }
+
+static void add_error(grpc_error **composite, grpc_error *error,
+                      const char *composite_error_string) {
+  if (error == GRPC_ERROR_NONE) return;
+  if (*composite == GRPC_ERROR_NONE) {
+    *composite = GRPC_ERROR_CREATE(composite_error_string);
+  }
+  *composite = grpc_error_add_child(*composite, error);
+}
+
+grpc_error *grpc_metadata_batch_filter(grpc_exec_ctx *exec_ctx,
+                                       grpc_metadata_batch *batch,
+                                       grpc_metadata_batch_filter_func func,
+                                       void *user_data,
+                                       const char *composite_error_string) {
+  grpc_linked_mdelem *l = batch->list.head;
+  grpc_error *error = GRPC_ERROR_NONE;
+  while (l) {
+    grpc_linked_mdelem *next = l->next;
+    grpc_filtered_mdelem new = func(exec_ctx, user_data, l->md);
+    add_error(&error, new.error, composite_error_string);
+    if (GRPC_MDISNULL(new.md)) {
+      grpc_metadata_batch_remove(exec_ctx, batch, l);
+    } else if (new.md.payload != l->md.payload) {
+      grpc_metadata_batch_substitute(exec_ctx, batch, l, new.md);
+    }
+    l = next;
+  }
+  return error;
+}
diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h
index d7f6485565..894a927d3e 100644
--- a/src/core/lib/transport/metadata_batch.h
+++ b/src/core/lib/transport/metadata_batch.h
@@ -133,6 +133,24 @@ grpc_error *grpc_metadata_batch_add_tail(
 
 grpc_error *grpc_attach_md_to_error(grpc_error *src, grpc_mdelem md);
 
+typedef struct {
+  grpc_error *error;
+  grpc_mdelem md;
+} grpc_filtered_mdelem;
+
+#define GRPC_FILTERED_ERROR(error) \
+  ((grpc_filtered_mdelem){(error), GRPC_MDNULL})
+#define GRPC_FILTERED_MDELEM(md) ((grpc_filtered_mdelem){GRPC_ERROR_NONE, (md)})
+#define GRPC_FILTERED_REMOVE() \
+  ((grpc_filtered_mdelem){GRPC_ERROR_NONE, GRPC_MDNULL})
+
+typedef grpc_filtered_mdelem (*grpc_metadata_batch_filter_func)(
+    grpc_exec_ctx *exec_ctx, void *user_data, grpc_mdelem elem);
+grpc_error *grpc_metadata_batch_filter(
+    grpc_exec_ctx *exec_ctx, grpc_metadata_batch *batch,
+    grpc_metadata_batch_filter_func func, void *user_data,
+    const char *composite_error_string) GRPC_MUST_USE_RESULT;
+
 #ifndef NDEBUG
 void grpc_metadata_batch_assert_ok(grpc_metadata_batch *comd);
 #else