From b239da9ab37042ceaf8c5183c0f7511aab352c63 Mon Sep 17 00:00:00 2001
From: Makarand Dharmapurikar <makarandd@google.com>
Date: Mon, 20 Mar 2017 13:49:49 -0700
Subject: [PATCH] removed multiple allocs, refactored b64 encoder
Added new api to b64.h for directly encoding to memory.
Using slice_sub instead of string operations for separating
path from query.
---
src/core/lib/channel/http_client_filter.c | 58 +++++++++++++++--------
src/core/lib/channel/http_server_filter.c | 33 +++++++------
src/core/lib/security/util/b64.c | 25 ++++++++--
src/core/lib/security/util/b64.h | 11 +++++
4 files changed, 86 insertions(+), 41 deletions(-)
diff --git a/src/core/lib/channel/http_client_filter.c b/src/core/lib/channel/http_client_filter.c
index d67b4c6e86..37bc07da89 100644
--- a/src/core/lib/channel/http_client_filter.c
+++ b/src/core/lib/channel/http_client_filter.c
@@ -296,28 +296,44 @@ static grpc_error *hc_mutate_op(grpc_exec_ctx *exec_ctx,
* MDELEM by appending base64 encoded query to the path */
static const int k_url_safe = 1;
static const int k_multi_line = 0;
- static char *k_query_separator = "?";
- char *strs_to_concatenate[3];
- strs_to_concatenate[0] = grpc_dump_slice(
- GRPC_MDVALUE(op->send_initial_metadata->idx.named.path->md),
- GPR_DUMP_ASCII);
- strs_to_concatenate[1] = k_query_separator;
- strs_to_concatenate[2] = grpc_base64_encode(
- (const void *)calld->payload_bytes, op->send_message->length,
- k_url_safe, k_multi_line);
- size_t concatenated_len;
- char *path_with_query = gpr_strjoin((const char **)strs_to_concatenate,
- 3, &concatenated_len);
- gpr_log(GPR_DEBUG, "Path with query: %s\n", path_with_query);
- gpr_free(strs_to_concatenate[0]);
- gpr_free(strs_to_concatenate[2]);
-
- /* substitute previous path with the new path */
+ static const char *k_query_separator = "?";
+
+ grpc_slice path_slice =
+ GRPC_MDVALUE(op->send_initial_metadata->idx.named.path->md);
+ /* sum up individual component's lengths and allocate enough memory to
+ * hold combined path+query */
+ size_t estimated_len = GRPC_SLICE_LENGTH(path_slice);
+ estimated_len += strlen(k_query_separator);
+ estimated_len += grpc_base64_estimate_encoded_size(
+ op->send_message->length, k_url_safe, k_multi_line);
+ estimated_len += 1; /* for the trailing 0 */
+ grpc_slice path_with_query_slice = grpc_slice_malloc(estimated_len);
+
+ /* memcopy individual pieces into this slice */
+ char *write_ptr = (char *)GRPC_SLICE_START_PTR(path_with_query_slice);
+
+ char *original_path = (char *)GRPC_SLICE_START_PTR(path_slice);
+ memcpy(write_ptr, original_path, GRPC_SLICE_LENGTH(path_slice));
+ write_ptr += (int)GRPC_SLICE_LENGTH(path_slice);
+
+ memcpy(write_ptr, k_query_separator, strlen(k_query_separator));
+ write_ptr += strlen(k_query_separator);
+
+ grpc_base64_encode_core(write_ptr, calld->payload_bytes,
+ op->send_message->length, k_url_safe,
+ k_multi_line);
+
+ /* remove trailing unused memory and add trailing 0 to terminate string
+ */
+ char *t = (char *)GRPC_SLICE_START_PTR(path_with_query_slice);
+ size_t path_length = strlen(t) + 1;
+ *(t + path_length) = 0;
+ path_with_query_slice =
+ grpc_slice_sub(path_with_query_slice, 0, path_length);
+
+ /* substitute previous path with the new path+query */
grpc_mdelem mdelem_path_and_query = grpc_mdelem_from_slices(
- exec_ctx, GRPC_MDSTR_PATH,
- grpc_slice_from_copied_buffer((const char *)path_with_query,
- concatenated_len));
- gpr_free(path_with_query);
+ exec_ctx, GRPC_MDSTR_PATH, path_with_query_slice);
grpc_metadata_batch *b = op->send_initial_metadata;
error = grpc_metadata_batch_substitute(exec_ctx, b, b->idx.named.path,
mdelem_path_and_query);
diff --git a/src/core/lib/channel/http_server_filter.c b/src/core/lib/channel/http_server_filter.c
index d7a4ba31b3..31149aa116 100644
--- a/src/core/lib/channel/http_server_filter.c
+++ b/src/core/lib/channel/http_server_filter.c
@@ -200,31 +200,34 @@ static grpc_error *server_filter_incoming_metadata(grpc_exec_ctx *exec_ctx,
} else if (*calld->recv_cacheable_request == true) {
/* We have a cacheable request made with GET verb. The path contains the
* query parameter which is base64 encoded request payload. */
- char *path =
- grpc_dump_slice(GRPC_MDVALUE(b->idx.named.path->md), GPR_DUMP_ASCII);
- static const char *QUERY_SEPARATOR = "?";
- char **query_parts;
- size_t num_query_parts;
- gpr_string_split(path, QUERY_SEPARATOR, &query_parts, &num_query_parts);
- GPR_ASSERT(num_query_parts == 2);
+ static const char *k_query_separator = "?";
+ grpc_slice path_slice = GRPC_MDVALUE(b->idx.named.path->md);
+ char *path_ptr = (char *)GRPC_SLICE_START_PTR(path_slice);
+ size_t path_length = GRPC_SLICE_LENGTH(path_slice);
+ /* offset of the character '?' */
+ size_t offset = 0;
+ for (offset = 0; *path_ptr != k_query_separator[0] && offset < path_length;
+ path_ptr++, offset++)
+ ;
+ grpc_slice query_slice =
+ grpc_slice_sub(path_slice, offset + 1, path_length);
+
/* substitute path metadata with just the path (not query) */
grpc_mdelem mdelem_path_without_query = grpc_mdelem_from_slices(
- exec_ctx, GRPC_MDSTR_PATH,
- grpc_slice_from_copied_buffer((const char *)query_parts[0],
- strlen(query_parts[0])));
+ exec_ctx, GRPC_MDSTR_PATH, grpc_slice_sub(path_slice, 0, offset));
+
grpc_metadata_batch_substitute(exec_ctx, b, b->idx.named.path,
mdelem_path_without_query);
- gpr_free(query_parts[0]);
- /* decode query into payload and add it to the slice buffer to be returned
- * */
+ /* decode payload from query and add to the slice buffer to be returned */
static const int k_url_safe = 1;
grpc_slice_buffer_add(
&calld->read_slice_buffer,
- grpc_base64_decode(exec_ctx, (const char *)query_parts[1], k_url_safe));
+ grpc_base64_decode(exec_ctx,
+ (const char *)GRPC_SLICE_START_PTR(query_slice),
+ k_url_safe));
grpc_slice_buffer_stream_init(&calld->read_stream,
&calld->read_slice_buffer, 0);
- gpr_free(query_parts[1]);
calld->seen_path_with_query = true;
}
diff --git a/src/core/lib/security/util/b64.c b/src/core/lib/security/util/b64.c
index 09c8213131..a84e42b4f6 100644
--- a/src/core/lib/security/util/b64.c
+++ b/src/core/lib/security/util/b64.c
@@ -71,15 +71,31 @@ static const char base64_url_safe_chars[] =
char *grpc_base64_encode(const void *vdata, size_t data_size, int url_safe,
int multiline) {
- const unsigned char *data = vdata;
- const char *base64_chars =
- url_safe ? base64_url_safe_chars : base64_url_unsafe_chars;
+ size_t result_projected_size =
+ grpc_base64_estimate_encoded_size(data_size, url_safe, multiline);
+ char *result = gpr_malloc(result_projected_size);
+ grpc_base64_encode_core(result, vdata, data_size, url_safe, multiline);
+ return result;
+}
+
+size_t grpc_base64_estimate_encoded_size(size_t data_size, int url_safe,
+ int multiline) {
size_t result_projected_size =
4 * ((data_size + 3) / 3) +
2 * (multiline ? (data_size / (3 * GRPC_BASE64_MULTILINE_NUM_BLOCKS))
: 0) +
1;
- char *result = gpr_malloc(result_projected_size);
+ return result_projected_size;
+}
+
+void grpc_base64_encode_core(char *result, const void *vdata, size_t data_size,
+ int url_safe, int multiline) {
+ const unsigned char *data = vdata;
+ const char *base64_chars =
+ url_safe ? base64_url_safe_chars : base64_url_unsafe_chars;
+ size_t result_projected_size =
+ grpc_base64_estimate_encoded_size(data_size, url_safe, multiline);
+
char *current = result;
size_t num_blocks = 0;
size_t i = 0;
@@ -119,7 +135,6 @@ char *grpc_base64_encode(const void *vdata, size_t data_size, int url_safe,
GPR_ASSERT(current >= result);
GPR_ASSERT((uintptr_t)(current - result) < result_projected_size);
result[current - result] = '\0';
- return result;
}
grpc_slice grpc_base64_decode(grpc_exec_ctx *exec_ctx, const char *b64,
diff --git a/src/core/lib/security/util/b64.h b/src/core/lib/security/util/b64.h
index d42a136f61..e84c50271e 100644
--- a/src/core/lib/security/util/b64.h
+++ b/src/core/lib/security/util/b64.h
@@ -41,6 +41,17 @@
char *grpc_base64_encode(const void *data, size_t data_size, int url_safe,
int multiline);
+/* estimate the upper bound on size of base64 encoded data. The actual size
+ * is guaranteed to be less than or equal to the size returned here. */
+size_t grpc_base64_estimate_encoded_size(size_t data_size, int url_safe,
+ int multiline);
+
+/* Encodes data using base64 and write it to memory pointed to by result. It is
+ * the caller's responsiblity to allocate enough memory in |result| to fit the
+ * encoded data. */
+void grpc_base64_encode_core(char *result, const void *vdata, size_t data_size,
+ int url_safe, int multiline);
+
/* Decodes data according to the base64 specification. Returns an empty
slice in case of failure. */
grpc_slice grpc_base64_decode(grpc_exec_ctx *exec_ctx, const char *b64,
--
GitLab