Merge pull request #12298 from markdroth/default_metadata_fix

Detect default metadata using the callout list.

5 files changed, 31 insertions, 10 deletions

diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c
index 80eb51ff0d..711938b278 100644
--- a/src/core/ext/transport/chttp2/transport/writing.c
+++ b/src/core/ext/transport/chttp2/transport/writing.c
@@ -154,17 +154,8 @@ static uint32_t target_write_size(grpc_chttp2_transport *t) {
 }
 
 // Returns true if initial_metadata contains only default headers.
-//
-// TODO(roth): The fact that we hard-code these particular headers here
-// is fairly ugly.  Need some better way to know which headers are
-// default, maybe via a bit in the static metadata table?
 static bool is_default_initial_metadata(grpc_metadata_batch *initial_metadata) {
-  int num_default_fields =
-      (initial_metadata->idx.named.status != NULL) +
-      (initial_metadata->idx.named.content_type != NULL) +
-      (initial_metadata->idx.named.grpc_encoding != NULL) +
-      (initial_metadata->idx.named.grpc_accept_encoding != NULL);
-  return (size_t)num_default_fields == initial_metadata->list.count;
+  return initial_metadata->list.default_count == initial_metadata->list.count;
 }
 
 grpc_chttp2_begin_write_result grpc_chttp2_begin_write(
diff --git a/src/core/lib/transport/metadata_batch.c b/src/core/lib/transport/metadata_batch.c
index 8f24b8527c..a077052561 100644
--- a/src/core/lib/transport/metadata_batch.c
+++ b/src/core/lib/transport/metadata_batch.c
@@ -105,6 +105,7 @@ static grpc_error *maybe_link_callout(grpc_metadata_batch *batch,
     return GRPC_ERROR_NONE;
   }
   if (batch->idx.array[idx] == NULL) {
+    if (grpc_static_callout_is_default[idx]) ++batch->list.default_count;
     batch->idx.array[idx] = storage;
     return GRPC_ERROR_NONE;
   }
@@ -120,6 +121,7 @@ static void maybe_unlink_callout(grpc_metadata_batch *batch,
   if (idx == GRPC_BATCH_CALLOUTS_COUNT) {
     return;
   }
+  if (grpc_static_callout_is_default[idx]) --batch->list.default_count;
   GPR_ASSERT(batch->idx.array[idx] != NULL);
   batch->idx.array[idx] = NULL;
 }
diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h
index 1b11a3e252..57d298c75c 100644
--- a/src/core/lib/transport/metadata_batch.h
+++ b/src/core/lib/transport/metadata_batch.h
@@ -41,6 +41,7 @@ typedef struct grpc_linked_mdelem {
 
 typedef struct grpc_mdelem_list {
   size_t count;
+  size_t default_count;  // Number of default keys.
   grpc_linked_mdelem *head;
   grpc_linked_mdelem *tail;
 } grpc_mdelem_list;
diff --git a/src/core/lib/transport/static_metadata.c b/src/core/lib/transport/static_metadata.c
index 28f05d5c44..b20d94aeac 100644
--- a/src/core/lib/transport/static_metadata.c
+++ b/src/core/lib/transport/static_metadata.c
@@ -823,6 +823,31 @@ grpc_mdelem_data grpc_static_mdelem_table[GRPC_STATIC_MDELEM_COUNT] = {
      {.refcount = &grpc_static_metadata_refcounts[97],
       .data.refcounted = {g_bytes + 1040, 13}}},
 };
+bool grpc_static_callout_is_default[GRPC_BATCH_CALLOUTS_COUNT] = {
+    true,  // :path
+    true,  // :method
+    true,  // :status
+    true,  // :authority
+    true,  // :scheme
+    true,  // te
+    true,  // grpc-message
+    true,  // grpc-status
+    true,  // grpc-payload-bin
+    true,  // grpc-encoding
+    true,  // grpc-accept-encoding
+    true,  // grpc-server-stats-bin
+    true,  // grpc-tags-bin
+    true,  // grpc-trace-bin
+    true,  // content-type
+    true,  // content-encoding
+    true,  // accept-encoding
+    true,  // grpc-internal-encoding-request
+    true,  // grpc-internal-stream-encoding-request
+    true,  // user-agent
+    true,  // host
+    true,  // lb-token
+};
+
 const uint8_t grpc_static_accept_encoding_metadata[8] = {0,  76, 77, 78,
                                                          79, 80, 81, 82};
 
diff --git a/src/core/lib/transport/static_metadata.h b/src/core/lib/transport/static_metadata.h
index 93ab90dff8..f03a9d23b1 100644
--- a/src/core/lib/transport/static_metadata.h
+++ b/src/core/lib/transport/static_metadata.h
@@ -571,6 +571,8 @@ typedef union {
              GRPC_BATCH_CALLOUTS_COUNT)                 \
        : GRPC_BATCH_CALLOUTS_COUNT)
 
+extern bool grpc_static_callout_is_default[GRPC_BATCH_CALLOUTS_COUNT];
+
 extern const uint8_t grpc_static_accept_encoding_metadata[8];
 #define GRPC_MDELEM_ACCEPT_ENCODING_FOR_ALGORITHMS(algs)                       \
   (GRPC_MAKE_MDELEM(                                                           \