Outlaw illegal metadata characters

4 files changed, 28 insertions, 6 deletions

diff --git a/src/core/channel/compress_filter.h b/src/core/channel/compress_filter.h
index 0917e81ca4..415459bca6 100644
--- a/src/core/channel/compress_filter.h
+++ b/src/core/channel/compress_filter.h
@@ -36,7 +36,7 @@
 
 #include "src/core/channel/channel_stack.h"
 
-#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "internal:grpc-encoding-request"
+#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "grpc-internal-encoding-request"
 
 /** Compression filter for outgoing data.
  *
diff --git a/src/core/surface/call.c b/src/core/surface/call.c
index 33f277da46..4426bbbce9 100644
--- a/src/core/surface/call.c
+++ b/src/core/surface/call.c
@@ -1046,10 +1046,11 @@ static int prepare_application_metadata(grpc_call *call, size_t count,
                                                (const gpr_uint8 *)md->value,
                                                md->value_length, 1);
     if (!grpc_mdstr_is_legal_header(l->md->key)) {
-      gpr_log(GPR_ERROR, "attempt to send invalid metadata key");
+      gpr_log(GPR_ERROR, "attempt to send invalid metadata key: %s",
+              grpc_mdstr_as_c_string(l->md->key));
       return 0;
     } else if (!grpc_mdstr_is_bin_suffixed(l->md->key) &&
-               !grpc_mdstr_is_legal_header(l->md->value)) {
+               !grpc_mdstr_is_legal_nonbin_header(l->md->value)) {
       gpr_log(GPR_ERROR, "attempt to send invalid metadata value");
       return 0;
     }
diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c
index f92e87e9dd..d758351feb 100644
--- a/src/core/transport/metadata.c
+++ b/src/core/transport/metadata.c
@@ -681,16 +681,36 @@ void grpc_mdctx_locked_mdelem_unref(grpc_mdctx *ctx,
 
 void grpc_mdctx_unlock(grpc_mdctx *ctx) { unlock(ctx); }
 
-int grpc_mdstr_is_legal_header(grpc_mdstr *s) {
-  /* TODO(ctiller): consider caching this, or computing it on construction */
+static int conforms_to(grpc_mdstr *s, const gpr_uint8 *legal_bits) {
   const gpr_uint8 *p = GPR_SLICE_START_PTR(s->slice);
   const gpr_uint8 *e = GPR_SLICE_END_PTR(s->slice);
   for (; p != e; p++) {
-    if (*p < 32 || *p > 126) return 0;
+    int idx = *p;
+    int byte = idx / 8;
+    int bit = idx % 8;
+    if ((legal_bits[byte] & (1 << bit)) == 0) return 0;
   }
   return 1;
 }
 
+int grpc_mdstr_is_legal_header(grpc_mdstr *s) {
+  static const gpr_uint8 legal_header_bits[256 / 8] = {
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xff, 0x03, 0xfe, 0xff, 0xff,
+      0x07, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+  /* TODO(ctiller): consider caching this, or computing it on construction */
+  return conforms_to(s, legal_header_bits);
+}
+
+int grpc_mdstr_is_legal_nonbin_header(grpc_mdstr *s) {
+  static const gpr_uint8 legal_header_bits[256 / 8] = {
+      0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+  return conforms_to(s, legal_header_bits);
+}
+
 int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s) {
   /* TODO(ctiller): consider caching this */
   return grpc_is_binary_header((const char *)GPR_SLICE_START_PTR(s->slice),
diff --git a/src/core/transport/metadata.h b/src/core/transport/metadata.h
index a7af49ba55..eb17747be7 100644
--- a/src/core/transport/metadata.h
+++ b/src/core/transport/metadata.h
@@ -154,6 +154,7 @@ void grpc_mdelem_unref(grpc_mdelem *md);
 const char *grpc_mdstr_as_c_string(grpc_mdstr *s);
 
 int grpc_mdstr_is_legal_header(grpc_mdstr *s);
+int grpc_mdstr_is_legal_nonbin_header(grpc_mdstr *s);
 int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s);
 
 /* Batch mode metadata functions.