Progress towards mdstr elimination

src/core/ext/transport/chttp2/transport/parsing.c

@@ -42,6 +42,7 @@
 #include "src/core/ext/transport/chttp2/transport/http2_errors.h"
 #include "src/core/ext/transport/chttp2/transport/status_conversion.h"
 #include "src/core/lib/profiling/timers.h"
+#include "src/core/lib/slice/slice_string_helpers.h"
 #include "src/core/lib/transport/static_metadata.h"
 #include "src/core/lib/transport/timeout_encoding.h"
 
@@ -451,24 +452,32 @@ static void on_initial_header(grpc_exec_ctx *exec_ctx, void *tp,
 
   GPR_ASSERT(s != NULL);
 
-  GRPC_CHTTP2_IF_TRACING(gpr_log(
+  if (grpc_http_trace) {
-      GPR_INFO, "HTTP:%d:HDR:%s: %s: %s", s->id, t->is_client ? "CLI" : "SVR",
+    char *key = grpc_dump_slice(md->key, GPR_DUMP_ASCII);
-      grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value)));
+    char *value = grpc_dump_slice(md->value, GPR_DUMP_HEX | GPR_DUMP_ASCII);
+    gpr_log(GPR_INFO, "HTTP:%d:HDR:%s: %s: %s", s->id,
+            t->is_client ? "CLI" : "SVR", key, value);
+    gpr_free(key);
+    gpr_free(value);
+  }
 
-  if (md->key == GRPC_MDSTR_GRPC_STATUS && md != GRPC_MDELEM_GRPC_STATUS_0) {
+  if (grpc_slice_cmp(md->key, GRPC_MDSTR_GRPC_STATUS) == 0 &&
+      md != GRPC_MDELEM_GRPC_STATUS_0) {
     /* TODO(ctiller): check for a status like " 0" */
     s->seen_error = true;
   }
 
-  if (md->key == GRPC_MDSTR_GRPC_TIMEOUT) {
+  if (grpc_slice_cmp(md->key, GRPC_MDSTR_GRPC_TIMEOUT) == 0) {
     gpr_timespec *cached_timeout = grpc_mdelem_get_user_data(md, free_timeout);
     if (!cached_timeout) {
       /* not already parsed: parse it now, and store the result away */
       cached_timeout = gpr_malloc(sizeof(gpr_timespec));
-      if (!grpc_http2_decode_timeout(grpc_mdstr_as_c_string(md->value),
+      if (!grpc_http2_decode_timeout(GRPC_SLICE_START_PTR(md->value),
+                                     GRPC_SLICE_LENGTH(md->value),
                                      cached_timeout)) {
-        gpr_log(GPR_ERROR, "Ignoring bad timeout value '%s'",
+        char *val = grpc_dump_slice(md->value, GPR_DUMP_ASCII);
-                grpc_mdstr_as_c_string(md->value));
+        gpr_log(GPR_ERROR, "Ignoring bad timeout value '%s'", val);
+        gpr_free(val);
         *cached_timeout = gpr_inf_future(GPR_TIMESPAN);
       }
       cached_timeout =
@@ -513,11 +522,17 @@ static void on_trailing_header(grpc_exec_ctx *exec_ctx, void *tp,
 
   GPR_ASSERT(s != NULL);
 
-  GRPC_CHTTP2_IF_TRACING(gpr_log(
+  if (grpc_http_trace) {
-      GPR_INFO, "HTTP:%d:TRL:%s: %s: %s", s->id, t->is_client ? "CLI" : "SVR",
+    char *key = grpc_dump_slice(md->key, GPR_DUMP_ASCII);
-      grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value)));
+    char *value = grpc_dump_slice(md->value, GPR_DUMP_HEX | GPR_DUMP_ASCII);
+    gpr_log(GPR_INFO, "HTTP:%d:TRL:%s: %s: %s", s->id,
+            t->is_client ? "CLI" : "SVR", key, value);
+    gpr_free(key);
+    gpr_free(value);
+  }
 
-  if (md->key == GRPC_MDSTR_GRPC_STATUS && md != GRPC_MDELEM_GRPC_STATUS_0) {
+  if (grpc_slice_cmp(md->key, GRPC_MDSTR_GRPC_STATUS) == 0 &&
+      md != GRPC_MDELEM_GRPC_STATUS_0) {
     /* TODO(ctiller): check for a status like " 0" */
     s->seen_error = true;
   }

src/core/lib/security/credentials/plugin/plugin_credentials.c

@@ -42,6 +42,7 @@
 #include <grpc/support/sync.h>
 
 #include "src/core/lib/slice/slice_internal.h"
+#include "src/core/lib/slice/slice_string_helpers.h"
 #include "src/core/lib/surface/api_trace.h"
 
 typedef struct {
@@ -77,13 +78,14 @@ static void plugin_md_request_metadata_ready(void *request,
     bool seen_illegal_header = false;
     grpc_credentials_md *md_array = NULL;
     for (i = 0; i < num_md; i++) {
-      if (!grpc_header_key_is_legal(md[i].key, strlen(md[i].key))) {
+      if (!grpc_header_key_slice_is_legal(md[i].key)) {
-        gpr_log(GPR_ERROR, "Plugin added invalid metadata key: %s", md[i].key);
+        char *key = grpc_dump_slice(md[i].key, GPR_DUMP_ASCII);
+        gpr_log(GPR_ERROR, "Plugin added invalid metadata key: %s", key);
+        gpr_free(key);
         seen_illegal_header = true;
         break;
-      } else if (!grpc_is_binary_header(md[i].key, strlen(md[i].key)) &&
+      } else if (!grpc_slice_is_binary_header(md[i].key) &&
-                 !grpc_header_nonbin_value_is_legal(md[i].value,
+                 !grpc_header_nonbin_value_slice_is_legal(md[i].value)) {
-                                                    md[i].value_length)) {
         gpr_log(GPR_ERROR, "Plugin added invalid metadata value.");
         seen_illegal_header = true;
         break;
@@ -95,9 +97,8 @@ static void plugin_md_request_metadata_ready(void *request,
     } else if (num_md > 0) {
       md_array = gpr_malloc(num_md * sizeof(grpc_credentials_md));
       for (i = 0; i < num_md; i++) {
-        md_array[i].key = grpc_slice_from_copied_string(md[i].key);
+        md_array[i].key = grpc_slice_ref(md[i].key);
-        md_array[i].value =
+        md_array[i].value = grpc_slice_ref(md[i].value);
-            grpc_slice_from_copied_buffer(md[i].value, md[i].value_length);
       }
       r->cb(&exec_ctx, r->user_data, md_array, num_md, GRPC_CREDENTIALS_OK,
             NULL);

src/core/lib/security/transport/client_auth_filter.c

@@ -45,6 +45,7 @@
 #include "src/core/lib/security/credentials/credentials.h"
 #include "src/core/lib/security/transport/security_connector.h"
 #include "src/core/lib/slice/slice_internal.h"
+#include "src/core/lib/slice/slice_string_helpers.h"
 #include "src/core/lib/support/string.h"
 #include "src/core/lib/surface/call.h"
 #include "src/core/lib/transport/static_metadata.h"
@@ -54,6 +55,8 @@
 /* We can have a per-call credentials. */
 typedef struct {
   grpc_call_credentials *creds;
+  bool have_host;
+  bool have_method;
   grpc_slice host;
   grpc_slice method;
   /* pollset{_set} bound to this call; if we need to make external
@@ -133,7 +136,7 @@ static void on_credentials_metadata(grpc_exec_ctx *exec_ctx, void *user_data,
 void build_auth_metadata_context(grpc_security_connector *sc,
                                  grpc_auth_context *auth_context,
                                  call_data *calld) {
-  char *service = gpr_strdup(grpc_mdstr_as_c_string(calld->method));
+  char *service = grpc_dump_slice(calld->method, GPR_DUMP_ASCII);
   char *last_slash = strrchr(service, '/');
   char *method_name = NULL;
   char *service_url = NULL;
@@ -149,14 +152,15 @@ void build_auth_metadata_context(grpc_security_connector *sc,
     method_name = gpr_strdup(last_slash + 1);
   }
   if (method_name == NULL) method_name = gpr_strdup("");
+  char *host = grpc_dump_slice(calld->host, GPR_DUMP_ASCII);
   gpr_asprintf(&service_url, "%s://%s%s",
-               sc->url_scheme == NULL ? "" : sc->url_scheme,
+               sc->url_scheme == NULL ? "" : sc->url_scheme, host, service);
-               grpc_mdstr_as_c_string(calld->host), service);
   calld->auth_md_context.service_url = service_url;
   calld->auth_md_context.method_name = method_name;
   calld->auth_md_context.channel_auth_context =
       GRPC_AUTH_CONTEXT_REF(auth_context, "grpc_auth_metadata_context");
   gpr_free(service);
+  gpr_free(host);
 }
 
 static void send_security_metadata(grpc_exec_ctx *exec_ctx,
@@ -207,8 +211,10 @@ static void on_host_checked(grpc_exec_ctx *exec_ctx, void *user_data,
     send_security_metadata(exec_ctx, elem, &calld->op);
   } else {
     char *error_msg;
+    char *host = grpc_dump_slice(calld->host, GPR_DUMP_ASCII);
     gpr_asprintf(&error_msg, "Invalid host %s set in :authority metadata.",
-                 grpc_mdstr_as_c_string(calld->host));
+                 host);
+    gpr_free(host);
     bubble_up_error(exec_ctx, elem, GRPC_STATUS_UNAUTHENTICATED, error_msg);
     gpr_free(error_msg);
   }
@@ -250,20 +256,27 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx,
       grpc_mdelem *md = l->md;
       /* Pointer comparison is OK for md_elems created from the same context.
        */
-      if (md->key == GRPC_MDSTR_AUTHORITY) {
+      if (grpc_slice_cmp(md->key, GRPC_MDSTR_AUTHORITY) == 0) {
-        if (calld->host != NULL) GRPC_MDSTR_UNREF(exec_ctx, calld->host);
+        if (calld->have_host) {
-        calld->host = GRPC_MDSTR_REF(md->value);
+          grpc_slice_unref_internal(exec_ctx, calld->host);
-      } else if (md->key == GRPC_MDSTR_PATH) {
+        }
-        if (calld->method != NULL) GRPC_MDSTR_UNREF(exec_ctx, calld->method);
+        calld->host = grpc_slice_ref_internal(md->value);
-        calld->method = GRPC_MDSTR_REF(md->value);
+        calld->have_host = true;
+      } else if (grpc_slice_cmp(md->key, GRPC_MDSTR_PATH) == 0) {
+        if (calld->have_method) {
+          grpc_slice_unref_internal(exec_ctx, calld->method);
+        }
+        calld->method = grpc_slice_ref_internal(md->value);
+        calld->have_method = true;
       }
     }
-    if (calld->host != NULL) {
+    if (calld->have_host) {
-      const char *call_host = grpc_mdstr_as_c_string(calld->host);
+      char *call_host = grpc_dump_slice(calld->host, GPR_DUMP_ASCII);
       calld->op = *op; /* Copy op (originates from the caller's stack). */
       grpc_channel_security_connector_check_call_host(
           exec_ctx, chand->security_connector, call_host, chand->auth_context,
           on_host_checked, elem);
+      gpr_free(call_host);
       GPR_TIMER_END("auth_start_transport_op", 0);
       return; /* early exit */
     }
@@ -296,11 +309,11 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
                               void *ignored) {
   call_data *calld = elem->call_data;
   grpc_call_credentials_unref(exec_ctx, calld->creds);
-  if (calld->host != NULL) {
+  if (calld->have_host) {
-    GRPC_MDSTR_UNREF(exec_ctx, calld->host);
+    grpc_slice_unref_internal(exec_ctx, calld->host);
   }
-  if (calld->method != NULL) {
+  if (calld->have_method) {
-    GRPC_MDSTR_UNREF(exec_ctx, calld->method);
+    grpc_slice_unref_internal(exec_ctx, calld->method);
   }
   reset_auth_metadata_context(&calld->auth_md_context);
 }

src/core/lib/transport/timeout_encoding.c

@@ -136,15 +136,17 @@ static int is_all_whitespace(const char *p) {
   return *p == 0;
 }
 
-int grpc_http2_decode_timeout(const char *buffer, gpr_timespec *timeout) {
+int grpc_http2_decode_timeout(const uint8_t *buffer, size_t length,
+                              gpr_timespec *timeout) {
   int32_t x = 0;
-  const uint8_t *p = (const uint8_t *)buffer;
+  const uint8_t *p = buffer;
+  const uint8_t *end = p + length;
   int have_digit = 0;
   /* skip whitespace */
-  for (; *p == ' '; p++)
+  for (; p != end && *p == ' '; p++)
     ;
   /* decode numeric part */
-  for (; *p >= '0' && *p <= '9'; p++) {
+  for (; p != end && *p >= '0' && *p <= '9'; p++) {
     int32_t digit = (int32_t)(*p - (uint8_t)'0');
     have_digit = 1;
     /* spec allows max. 8 digits, but we allow values up to 1,000,000,000 */
@@ -158,8 +160,9 @@ int grpc_http2_decode_timeout(const char *buffer, gpr_timespec *timeout) {
   }
   if (!have_digit) return 0;
   /* skip whitespace */
-  for (; *p == ' '; p++)
+  for (; p != end && *p == ' '; p++)
     ;
+  if (p == end) return 0;
   /* decode unit specifier */
   switch (*p) {
     case 'n':

src/core/lib/transport/timeout_encoding.h

@@ -42,6 +42,7 @@
 /* Encode/decode timeouts to the GRPC over HTTP/2 format;
    encoding may round up arbitrarily */
 void grpc_http2_encode_timeout(gpr_timespec timeout, char *buffer);
-int grpc_http2_decode_timeout(const char *buffer, gpr_timespec *timeout);
+int grpc_http2_decode_timeout(const uint8_t *buffer, size_t length,
+                              gpr_timespec *timeout);
 
 #endif /* GRPC_CORE_LIB_TRANSPORT_TIMEOUT_ENCODING_H */