Merge pull request #4633 from murgatroid99/core_validate_metadata

Expose core metadata validation functions in public headers

BUILD

@@ -384,6 +384,7 @@ cc_library(
     "src/core/surface/server.c",
     "src/core/surface/server_chttp2.c",
     "src/core/surface/server_create.c",
+    "src/core/surface/validate_metadata.c",
     "src/core/surface/version.c",
     "src/core/transport/byte_stream.c",
     "src/core/transport/chttp2/alpn.c",
@@ -655,6 +656,7 @@ cc_library(
     "src/core/surface/server.c",
     "src/core/surface/server_chttp2.c",
     "src/core/surface/server_create.c",
+    "src/core/surface/validate_metadata.c",
     "src/core/surface/version.c",
     "src/core/transport/byte_stream.c",
     "src/core/transport/chttp2/alpn.c",
@@ -1189,6 +1191,7 @@ objc_library(
     "src/core/surface/server.c",
     "src/core/surface/server_chttp2.c",
     "src/core/surface/server_create.c",
+    "src/core/surface/validate_metadata.c",
     "src/core/surface/version.c",
     "src/core/transport/byte_stream.c",
     "src/core/transport/chttp2/alpn.c",

Makefile

@@ -6441,6 +6441,7 @@ LIBGRPC_SRC = \
     src/core/surface/server.c \
     src/core/surface/server_chttp2.c \
     src/core/surface/server_create.c \
+    src/core/surface/validate_metadata.c \
     src/core/surface/version.c \
     src/core/transport/byte_stream.c \
     src/core/transport/chttp2/alpn.c \
@@ -6724,6 +6725,7 @@ LIBGRPC_UNSECURE_SRC = \
     src/core/surface/server.c \
     src/core/surface/server_chttp2.c \
     src/core/surface/server_create.c \
+    src/core/surface/validate_metadata.c \
     src/core/surface/version.c \
     src/core/transport/byte_stream.c \
     src/core/transport/chttp2/alpn.c \

binding.gyp

@@ -269,6 +269,7 @@
         'src/core/surface/server.c',
         'src/core/surface/server_chttp2.c',
         'src/core/surface/server_create.c',
+        'src/core/surface/validate_metadata.c',
         'src/core/surface/version.c',
         'src/core/transport/byte_stream.c',
         'src/core/transport/chttp2/alpn.c',

build.yaml

@@ -316,6 +316,7 @@ filegroups:
   - src/core/surface/server.c
   - src/core/surface/server_chttp2.c
   - src/core/surface/server_create.c
+  - src/core/surface/validate_metadata.c
   - src/core/surface/version.c
   - src/core/transport/byte_stream.c
   - src/core/transport/chttp2/alpn.c

gRPC.podspec

@@ -395,6 +395,7 @@ Pod::Spec.new do |s|
                       'src/core/surface/server.c',
                       'src/core/surface/server_chttp2.c',
                       'src/core/surface/server_create.c',
+                      'src/core/surface/validate_metadata.c',
                       'src/core/surface/version.c',
                       'src/core/transport/byte_stream.c',
                       'src/core/transport/chttp2/alpn.c',

grpc.gemspec

@@ -378,6 +378,7 @@ Gem::Specification.new do |s|
   s.files += %w( src/core/surface/server.c )
   s.files += %w( src/core/surface/server_chttp2.c )
   s.files += %w( src/core/surface/server_create.c )
+  s.files += %w( src/core/surface/validate_metadata.c )
   s.files += %w( src/core/surface/version.c )
   s.files += %w( src/core/transport/byte_stream.c )
   s.files += %w( src/core/transport/chttp2/alpn.c )

include/grpc/grpc.h

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -715,6 +715,16 @@ void grpc_server_destroy(grpc_server *server);
     thread-safety issues raised by it should not be of concern. */
 int grpc_tracer_set_enabled(const char *name, int enabled);
 
+/** Check whether a metadata key is legal (will be accepted by core) */
+int grpc_header_key_is_legal(const char *key, size_t length);
+
+/** Check whether a non-binary metadata value is legal (will be accepted by
+    core) */
+int grpc_header_nonbin_value_is_legal(const char *value, size_t length);
+
+/** Check whether a metadata key corresponds to a binary value */
+int grpc_is_binary_header(const char *key, size_t length);
+
 #ifdef __cplusplus
 }
 #endif

package.json

@@ -329,6 +329,7 @@
     "src/core/surface/server.c",
     "src/core/surface/server_chttp2.c",
     "src/core/surface/server_create.c",
+    "src/core/surface/validate_metadata.c",
     "src/core/surface/version.c",
     "src/core/transport/byte_stream.c",
     "src/core/transport/chttp2/alpn.c",

src/core/surface/call.c

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -37,6 +37,7 @@
 #include <string.h>
 
 #include <grpc/compression.h>
+#include <grpc/grpc.h>
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
@@ -562,12 +563,16 @@ static int prepare_application_metadata(grpc_call *call, int count,
     GPR_ASSERT(sizeof(grpc_linked_mdelem) == sizeof(md->internal_data));
     l->md = grpc_mdelem_from_string_and_buffer(
         md->key, (const gpr_uint8 *)md->value, md->value_length);
-    if (!grpc_mdstr_is_legal_header(l->md->key)) {
+    if (!grpc_header_key_is_legal(grpc_mdstr_as_c_string(l->md->key),
+                                  GRPC_MDSTR_LENGTH(l->md->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_nonbin_header(l->md->value)) {
+    } else if (!grpc_is_binary_header(grpc_mdstr_as_c_string(l->md->key),
+                                      GRPC_MDSTR_LENGTH(l->md->key)) &&
+               !grpc_header_nonbin_value_is_legal(
+                   grpc_mdstr_as_c_string(l->md->value),
+                   GRPC_MDSTR_LENGTH(l->md->value))) {
       gpr_log(GPR_ERROR, "attempt to send invalid metadata value");
       return 0;
     }

src/core/surface/validate_metadata.c

@@ -0,0 +1,70 @@
+/*
+ *
+ * Copyright 2016, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include <stdlib.h>
+#include <string.h>
+
+#include <grpc/support/port_platform.h>
+
+static int conforms_to(const char *s, size_t len, const gpr_uint8 *legal_bits) {
+  const char *p = s;
+  const char *e = s + len;
+  for (; p != e; p++) {
+    int idx = *p;
+    int byte = idx / 8;
+    int bit = idx % 8;
+    if ((legal_bits[byte] & (1 << bit)) == 0) return 0;
+  }
+  return 1;
+}
+
+int grpc_header_key_is_legal(const char *key, size_t length) {
+  static const gpr_uint8 legal_header_bits[256 / 8] = {
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xff, 0x03, 0x00, 0x00, 0x00,
+      0x80, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+  return conforms_to(key, length, legal_header_bits);
+}
+
+int grpc_header_nonbin_value_is_legal(const char *value, size_t length) {
+  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(value, length, legal_header_bits);
+}
+
+int grpc_is_binary_header(const char *key, size_t length) {
+  if (length < 5) return 0;
+  return 0 == memcmp(key + length - 4, "-bin", 4);
+}

src/core/transport/chttp2/bin_encoder.c

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -283,8 +283,3 @@ gpr_slice grpc_chttp2_base64_encode_and_huffman_compress(gpr_slice input) {
   GPR_ASSERT(in == GPR_SLICE_END_PTR(input));
   return output;
 }
-
-int grpc_is_binary_header(const char *key, size_t length) {
-  if (length < 5) return 0;
-  return 0 == memcmp(key + length - 4, "-bin", 4);
-}

src/core/transport/chttp2/bin_encoder.h

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -51,6 +51,4 @@ gpr_slice grpc_chttp2_huffman_compress(gpr_slice input);
    return y; */
 gpr_slice grpc_chttp2_base64_encode_and_huffman_compress(gpr_slice input);
 
-int grpc_is_binary_header(const char *key, size_t length);
-
 #endif /* GRPC_INTERNAL_CORE_TRANSPORT_CHTTP2_BIN_ENCODER_H */

src/core/transport/chttp2/hpack_encoder.c

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -36,6 +36,11 @@
 #include <assert.h>
 #include <string.h>
 
+/* This is here for grpc_is_binary_header
+ * TODO(murgatroid99): Remove this
+ */
+#include <grpc/grpc.h>
+
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/useful.h>

src/core/transport/chttp2/hpack_parser.c

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -38,6 +38,11 @@
 #include <string.h>
 #include <assert.h>
 
+/* This is here for grpc_is_binary_header
+ * TODO(murgatroid99): Remove this
+ */
+#include <grpc/grpc.h>
+
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/port_platform.h>

src/core/transport/metadata.c

@@ -688,37 +688,3 @@ gpr_slice grpc_mdstr_as_base64_encoded_and_huffman_compressed(grpc_mdstr *gs) {
   gpr_mu_unlock(&shard->mu);
   return slice;
 }
-
-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++) {
-    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, 0x00, 0x00, 0x00,
-      0x80, 0xfe, 0xff, 0xff, 0x07, 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_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),
-                               GPR_SLICE_LENGTH(s->slice));
-}

src/core/transport/metadata.h

@@ -142,6 +142,8 @@ void grpc_mdelem_unref(grpc_mdelem *md);
    Does not promise that the returned string has no embedded nulls however. */
 const char *grpc_mdstr_as_c_string(grpc_mdstr *s);
 
+#define GRPC_MDSTR_LENGTH(s) (GPR_SLICE_LENGTH(s->slice))
+
 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);

test/core/transport/chttp2/bin_encoder_test.c

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -35,6 +35,10 @@
 
 #include <string.h>
 
+/* This is here for grpc_is_binary_header
+ * TODO(murgatroid99): Remove this
+ */
+#include <grpc/grpc.h>
 #include "src/core/support/string.h"
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>

tools/doxygen/Doxyfile.core.internal

@@ -1013,6 +1013,7 @@ src/core/surface/metadata_array.c \
 src/core/surface/server.c \
 src/core/surface/server_chttp2.c \
 src/core/surface/server_create.c \
+src/core/surface/validate_metadata.c \
 src/core/surface/version.c \
 src/core/transport/byte_stream.c \
 src/core/transport/chttp2/alpn.c \

tools/run_tests/run_interop_tests.py

@@ -555,7 +555,7 @@ def aggregate_http2_results(stdout):
   match = re.search(r'\{"cases[^\]]*\]\}', stdout)
   if not match:
     return None
-    
+
   results = json.loads(match.group(0))
   skipped = 0
   passed = 0
@@ -748,7 +748,7 @@ try:
       for test_case in _HTTP2_TEST_CASES:
         if server_name == "go":
           # TODO(carl-mastrangelo): Reenable after https://github.com/grpc/grpc-go/issues/434
-          continue 
+          continue
         test_job = cloud_to_cloud_jobspec(http2Interop,
                                           test_case,
                                           server_name,
@@ -777,7 +777,7 @@ try:
       job[0].http2results = aggregate_http2_results(job[0].message)
 
   report_utils.render_interop_html_report(
-      set([str(l) for l in languages]), servers, _TEST_CASES, _AUTH_TEST_CASES, 
+      set([str(l) for l in languages]), servers, _TEST_CASES, _AUTH_TEST_CASES,
       _HTTP2_TEST_CASES, resultset, num_failures,
       args.cloud_to_prod_auth or args.cloud_to_prod, args.http2_interop)
 

tools/run_tests/sources_and_headers.json

@@ -17842,6 +17842,7 @@
       "src/core/surface/server_chttp2.c", 
       "src/core/surface/server_create.c", 
       "src/core/surface/surface_trace.h", 
+      "src/core/surface/validate_metadata.c", 
       "src/core/surface/version.c", 
       "src/core/transport/byte_stream.c", 
       "src/core/transport/byte_stream.h", 
@@ -18312,6 +18313,7 @@
       "src/core/surface/server_chttp2.c", 
       "src/core/surface/server_create.c", 
       "src/core/surface/surface_trace.h", 
+      "src/core/surface/validate_metadata.c", 
       "src/core/surface/version.c", 
       "src/core/transport/byte_stream.c", 
       "src/core/transport/byte_stream.h", 

vsprojects/vcxproj/grpc/grpc.vcxproj

@@ -621,6 +621,8 @@
     </ClCompile>
     <ClCompile Include="..\..\..\src\core\surface\server_create.c">
     </ClCompile>
+    <ClCompile Include="..\..\..\src\core\surface\validate_metadata.c">
+    </ClCompile>
     <ClCompile Include="..\..\..\src\core\surface\version.c">
     </ClCompile>
     <ClCompile Include="..\..\..\src\core\transport\byte_stream.c">

vsprojects/vcxproj/grpc/grpc.vcxproj.filters

@@ -352,6 +352,9 @@
     <ClCompile Include="..\..\..\src\core\surface\server_create.c">
       <Filter>src\core\surface</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\..\src\core\surface\validate_metadata.c">
+      <Filter>src\core\surface</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\..\src\core\surface\version.c">
       <Filter>src\core\surface</Filter>
     </ClCompile>

vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj

@@ -560,6 +560,8 @@
     </ClCompile>
     <ClCompile Include="..\..\..\src\core\surface\server_create.c">
     </ClCompile>
+    <ClCompile Include="..\..\..\src\core\surface\validate_metadata.c">
+    </ClCompile>
     <ClCompile Include="..\..\..\src\core\surface\version.c">
     </ClCompile>
     <ClCompile Include="..\..\..\src\core\transport\byte_stream.c">

vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters

@@ -292,6 +292,9 @@
     <ClCompile Include="..\..\..\src\core\surface\server_create.c">
       <Filter>src\core\surface</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\..\src\core\surface\validate_metadata.c">
+      <Filter>src\core\surface</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\..\src\core\surface\version.c">
       <Filter>src\core\surface</Filter>
     </ClCompile>