diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c index b8ab9871049..edb9104fe8f 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.c +++ b/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( - GPR_INFO, "HTTP:%d:HDR:%s: %s: %s", s->id, t->is_client ? "CLI" : "SVR", - grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value))); + if (grpc_http_trace) { + char *key = grpc_dump_slice(md->key, GPR_DUMP_ASCII); + 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'", - grpc_mdstr_as_c_string(md->value)); + char *val = grpc_dump_slice(md->value, GPR_DUMP_ASCII); + 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( - GPR_INFO, "HTTP:%d:TRL:%s: %s: %s", s->id, t->is_client ? "CLI" : "SVR", - grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value))); + if (grpc_http_trace) { + char *key = grpc_dump_slice(md->key, GPR_DUMP_ASCII); + 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; } diff --git a/src/core/lib/security/credentials/plugin/plugin_credentials.c b/src/core/lib/security/credentials/plugin/plugin_credentials.c index f90d7dce83f..4fc02b2659b 100644 --- a/src/core/lib/security/credentials/plugin/plugin_credentials.c +++ b/src/core/lib/security/credentials/plugin/plugin_credentials.c @@ -42,6 +42,7 @@ #include #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))) { - gpr_log(GPR_ERROR, "Plugin added invalid metadata key: %s", md[i].key); + if (!grpc_header_key_slice_is_legal(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)) && - !grpc_header_nonbin_value_is_legal(md[i].value, - md[i].value_length)) { + } else if (!grpc_slice_is_binary_header(md[i].key) && + !grpc_header_nonbin_value_slice_is_legal(md[i].value)) { 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].value = - grpc_slice_from_copied_buffer(md[i].value, md[i].value_length); + md_array[i].key = grpc_slice_ref(md[i].key); + md_array[i].value = grpc_slice_ref(md[i].value); } r->cb(&exec_ctx, r->user_data, md_array, num_md, GRPC_CREDENTIALS_OK, NULL); diff --git a/src/core/lib/security/transport/client_auth_filter.c b/src/core/lib/security/transport/client_auth_filter.c index e9e77182b61..6fc73734bca 100644 --- a/src/core/lib/security/transport/client_auth_filter.c +++ b/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, - grpc_mdstr_as_c_string(calld->host), service); + sc->url_scheme == NULL ? "" : sc->url_scheme, 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 (calld->host != NULL) GRPC_MDSTR_UNREF(exec_ctx, calld->host); - calld->host = GRPC_MDSTR_REF(md->value); - } else if (md->key == GRPC_MDSTR_PATH) { - if (calld->method != NULL) GRPC_MDSTR_UNREF(exec_ctx, calld->method); - calld->method = GRPC_MDSTR_REF(md->value); + if (grpc_slice_cmp(md->key, GRPC_MDSTR_AUTHORITY) == 0) { + if (calld->have_host) { + grpc_slice_unref_internal(exec_ctx, calld->host); + } + calld->host = grpc_slice_ref_internal(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) { - const char *call_host = grpc_mdstr_as_c_string(calld->host); + if (calld->have_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) { - GRPC_MDSTR_UNREF(exec_ctx, calld->host); + if (calld->have_host) { + grpc_slice_unref_internal(exec_ctx, calld->host); } - if (calld->method != NULL) { - GRPC_MDSTR_UNREF(exec_ctx, calld->method); + if (calld->have_method) { + grpc_slice_unref_internal(exec_ctx, calld->method); } reset_auth_metadata_context(&calld->auth_md_context); } diff --git a/src/core/lib/transport/timeout_encoding.c b/src/core/lib/transport/timeout_encoding.c index b58ebbd0a8f..b2060be59e5 100644 --- a/src/core/lib/transport/timeout_encoding.c +++ b/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': diff --git a/src/core/lib/transport/timeout_encoding.h b/src/core/lib/transport/timeout_encoding.h index 92f02f6ecd1..838892595f0 100644 --- a/src/core/lib/transport/timeout_encoding.h +++ b/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 */