From 366c9c5aaa004d04f63e3c4ff85d418e6f1f2ec5 Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar Date: Mon, 20 Mar 2017 09:41:46 -0700 Subject: [PATCH] using URI query to communicate payload When using GET verb, the request payload is now communicated as a base64 encoded string in the query parameter. Deleted the code that wrote/read a custom header field (payload-bin), since caches don't consider this field when looking for a cache hit. --- src/core/lib/channel/http_client_filter.c | 41 +++++++++++++----- src/core/lib/channel/http_server_filter.c | 52 ++++++++++++++++------- 2 files changed, 67 insertions(+), 26 deletions(-) diff --git a/src/core/lib/channel/http_client_filter.c b/src/core/lib/channel/http_client_filter.c index c031533dd86..d67b4c6e867 100644 --- a/src/core/lib/channel/http_client_filter.c +++ b/src/core/lib/channel/http_client_filter.c @@ -36,6 +36,7 @@ #include #include #include "src/core/lib/profiling/timers.h" +#include "src/core/lib/security/util/b64.h" #include "src/core/lib/slice/percent_encoding.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" @@ -56,7 +57,6 @@ typedef struct call_data { grpc_linked_mdelem te_trailers; grpc_linked_mdelem content_type; grpc_linked_mdelem user_agent; - grpc_linked_mdelem payload_bin; grpc_metadata_batch *recv_initial_metadata; grpc_metadata_batch *recv_trailing_metadata; @@ -292,16 +292,37 @@ static grpc_error *hc_mutate_op(grpc_exec_ctx *exec_ctx, continue_send_message(exec_ctx, elem); if (calld->send_message_blocked == false) { - /* when all the send_message data is available, then create a MDELEM and - append to headers */ - grpc_mdelem payload_bin = grpc_mdelem_from_slices( - exec_ctx, GRPC_MDSTR_GRPC_PAYLOAD_BIN, - grpc_slice_from_copied_buffer((const char *)calld->payload_bytes, - op->send_message->length)); - error = - grpc_metadata_batch_add_tail(exec_ctx, op->send_initial_metadata, - &calld->payload_bin, payload_bin); + /* when all the send_message data is available, then modify the path + * 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 */ + 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); + grpc_metadata_batch *b = op->send_initial_metadata; + error = grpc_metadata_batch_substitute(exec_ctx, b, b->idx.named.path, + mdelem_path_and_query); if (error != GRPC_ERROR_NONE) return error; + calld->on_complete = op->on_complete; op->on_complete = &calld->hc_on_complete; op->send_message = NULL; diff --git a/src/core/lib/channel/http_server_filter.c b/src/core/lib/channel/http_server_filter.c index fb70de8e96c..d7a4ba31b35 100644 --- a/src/core/lib/channel/http_server_filter.c +++ b/src/core/lib/channel/http_server_filter.c @@ -37,6 +37,7 @@ #include #include #include "src/core/lib/profiling/timers.h" +#include "src/core/lib/security/util/b64.h" #include "src/core/lib/slice/percent_encoding.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" @@ -51,8 +52,8 @@ typedef struct call_data { grpc_linked_mdelem status; grpc_linked_mdelem content_type; - /* did this request come with payload-bin */ - bool seen_payload_bin; + /* did this request come with path query containing request payload */ + bool seen_path_with_query; /* flag to ensure payload_bin is delivered only once */ bool payload_bin_delivered; @@ -61,7 +62,7 @@ typedef struct call_data { bool *recv_cacheable_request; /** Closure to call when finished with the hs_on_recv hook */ grpc_closure *on_done_recv; - /** Closure to call when we retrieve read message from the payload-bin header + /** Closure to call when we retrieve read message from the path URI */ grpc_closure *recv_message_ready; grpc_closure *on_complete; @@ -196,6 +197,35 @@ static grpc_error *server_filter_incoming_metadata(grpc_exec_ctx *exec_ctx, add_error(error_name, &error, grpc_error_set_str(GRPC_ERROR_CREATE("Missing header"), GRPC_ERROR_STR_KEY, ":path")); + } 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); + /* 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]))); + 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 + * */ + 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_slice_buffer_stream_init(&calld->read_stream, + &calld->read_slice_buffer, 0); + gpr_free(query_parts[1]); + calld->seen_path_with_query = true; } if (b->idx.named.host != NULL && b->idx.named.authority == NULL) { @@ -217,16 +247,6 @@ static grpc_error *server_filter_incoming_metadata(grpc_exec_ctx *exec_ctx, GRPC_ERROR_STR_KEY, ":authority")); } - if (b->idx.named.grpc_payload_bin != NULL) { - calld->seen_payload_bin = true; - grpc_slice_buffer_add(&calld->read_slice_buffer, - grpc_slice_ref_internal( - GRPC_MDVALUE(b->idx.named.grpc_payload_bin->md))); - grpc_slice_buffer_stream_init(&calld->read_stream, - &calld->read_slice_buffer, 0); - grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.grpc_payload_bin); - } - return error; } @@ -247,8 +267,8 @@ static void hs_on_complete(grpc_exec_ctx *exec_ctx, void *user_data, grpc_error *err) { grpc_call_element *elem = user_data; call_data *calld = elem->call_data; - /* Call recv_message_ready if we got the payload via the header field */ - if (calld->seen_payload_bin && calld->recv_message_ready != NULL) { + /* Call recv_message_ready if we got the payload via the path field */ + if (calld->seen_path_with_query && calld->recv_message_ready != NULL) { *calld->pp_recv_message = calld->payload_bin_delivered ? NULL : (grpc_byte_stream *)&calld->read_stream; @@ -263,7 +283,7 @@ static void hs_recv_message_ready(grpc_exec_ctx *exec_ctx, void *user_data, grpc_error *err) { grpc_call_element *elem = user_data; call_data *calld = elem->call_data; - if (calld->seen_payload_bin) { + if (calld->seen_path_with_query) { /* do nothing. This is probably a GET request, and payload will be returned in hs_on_complete callback. */ } else {