Merge branch 'master' into limit_metadata_size
diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h
index 4c73730..b5203b6 100644
--- a/include/grpc/impl/codegen/grpc_types.h
+++ b/include/grpc/impl/codegen/grpc_types.h
@@ -152,6 +152,8 @@
channel). If this parameter is specified and the underlying is not an SSL
channel, it will just be ignored. */
#define GRPC_SSL_TARGET_NAME_OVERRIDE_ARG "grpc.ssl_target_name_override"
+/* Maximum metadata size */
+#define GRPC_ARG_MAX_METADATA_SIZE "grpc.max_metadata_size"
/** Result of a grpc call. If the caller satisfies the prerequisites of a
particular operation, the grpc_call_error returned will be GRPC_CALL_OK.
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index fcf2abf..6ee5e06 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -65,8 +65,8 @@
((grpc_chttp2_transport *)((char *)(tw)-offsetof(grpc_chttp2_transport, \
writing)))
-#define TRANSPORT_FROM_PARSING(tw) \
- ((grpc_chttp2_transport *)((char *)(tw)-offsetof(grpc_chttp2_transport, \
+#define TRANSPORT_FROM_PARSING(tp) \
+ ((grpc_chttp2_transport *)((char *)(tp)-offsetof(grpc_chttp2_transport, \
parsing)))
#define TRANSPORT_FROM_GLOBAL(tg) \
@@ -378,6 +378,18 @@
&t->writing.hpack_compressor,
(uint32_t)channel_args->args[i].value.integer);
}
+ } else if (0 == strcmp(channel_args->args[i].key,
+ GRPC_ARG_MAX_METADATA_SIZE)) {
+ if (channel_args->args[i].type != GRPC_ARG_INTEGER) {
+ gpr_log(GPR_ERROR, "%s: must be an integer",
+ GRPC_ARG_MAX_METADATA_SIZE);
+ } else if (channel_args->args[i].value.integer < 0) {
+ gpr_log(GPR_ERROR, "%s: must be non-negative",
+ GRPC_ARG_MAX_METADATA_SIZE);
+ } else {
+ push_setting(t, GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE,
+ (uint32_t)channel_args->args[i].value.integer);
+ }
}
}
}
@@ -925,24 +937,37 @@
stream_global->send_initial_metadata_finished =
add_closure_barrier(on_complete);
stream_global->send_initial_metadata = op->send_initial_metadata;
- if (contains_non_ok_status(transport_global, op->send_initial_metadata)) {
- stream_global->seen_error = 1;
- grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
- }
- if (!stream_global->write_closed) {
- if (transport_global->is_client) {
- GPR_ASSERT(stream_global->id == 0);
- grpc_chttp2_list_add_waiting_for_concurrency(transport_global,
- stream_global);
- maybe_start_some_streams(exec_ctx, transport_global);
- } else {
- GPR_ASSERT(stream_global->id != 0);
- grpc_chttp2_become_writable(transport_global, stream_global);
- }
+ const size_t metadata_size = grpc_metadata_batch_size(
+ op->send_initial_metadata);
+ const size_t metadata_peer_limit =
+ transport_global->settings[GRPC_PEER_SETTINGS]
+ [GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE];
+ if (metadata_size > metadata_peer_limit) {
+ gpr_log(GPR_DEBUG,
+ "initial metadata size exceeds peer limit (%lu vs. %lu)",
+ metadata_size, metadata_peer_limit);
+ cancel_from_api(exec_ctx, transport_global, stream_global,
+ GRPC_STATUS_RESOURCE_EXHAUSTED);
} else {
- grpc_chttp2_complete_closure_step(
- exec_ctx, stream_global,
- &stream_global->send_initial_metadata_finished, 0);
+ if (contains_non_ok_status(transport_global, op->send_initial_metadata)) {
+ stream_global->seen_error = true;
+ grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
+ }
+ if (!stream_global->write_closed) {
+ if (transport_global->is_client) {
+ GPR_ASSERT(stream_global->id == 0);
+ grpc_chttp2_list_add_waiting_for_concurrency(transport_global,
+ stream_global);
+ maybe_start_some_streams(exec_ctx, transport_global);
+ } else {
+ GPR_ASSERT(stream_global->id != 0);
+ grpc_chttp2_become_writable(transport_global, stream_global);
+ }
+ } else {
+ grpc_chttp2_complete_closure_step(
+ exec_ctx, stream_global,
+ &stream_global->send_initial_metadata_finished, 0);
+ }
}
}
@@ -966,19 +991,33 @@
stream_global->send_trailing_metadata_finished =
add_closure_barrier(on_complete);
stream_global->send_trailing_metadata = op->send_trailing_metadata;
- if (contains_non_ok_status(transport_global, op->send_trailing_metadata)) {
- stream_global->seen_error = 1;
- grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
- }
- if (stream_global->write_closed) {
- grpc_chttp2_complete_closure_step(
- exec_ctx, stream_global,
- &stream_global->send_trailing_metadata_finished,
- grpc_metadata_batch_is_empty(op->send_trailing_metadata));
- } else if (stream_global->id != 0) {
- /* TODO(ctiller): check if there's flow control for any outstanding
- bytes before going writable */
- grpc_chttp2_become_writable(transport_global, stream_global);
+ const size_t metadata_size = grpc_metadata_batch_size(
+ op->send_trailing_metadata);
+ const size_t metadata_peer_limit =
+ transport_global->settings[GRPC_PEER_SETTINGS]
+ [GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE];
+ if (metadata_size > metadata_peer_limit) {
+ gpr_log(GPR_DEBUG,
+ "trailing metadata size exceeds peer limit (%lu vs. %lu)",
+ metadata_size, metadata_peer_limit);
+ cancel_from_api(exec_ctx, transport_global, stream_global,
+ GRPC_STATUS_RESOURCE_EXHAUSTED);
+ } else {
+ if (contains_non_ok_status(transport_global,
+ op->send_trailing_metadata)) {
+ stream_global->seen_error = true;
+ grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
+ }
+ if (stream_global->write_closed) {
+ grpc_chttp2_complete_closure_step(
+ exec_ctx, stream_global,
+ &stream_global->send_trailing_metadata_finished,
+ grpc_metadata_batch_is_empty(op->send_trailing_metadata));
+ } else if (stream_global->id != 0) {
+ /* TODO(ctiller): check if there's flow control for any outstanding
+ bytes before going writable */
+ grpc_chttp2_become_writable(transport_global, stream_global);
+ }
}
}
@@ -1141,6 +1180,16 @@
grpc_chttp2_list_pop_check_read_ops(transport_global, &stream_global)) {
if (stream_global->recv_initial_metadata_ready != NULL &&
stream_global->published_initial_metadata) {
+ if (stream_global->seen_error) {
+ while ((bs = grpc_chttp2_incoming_frame_queue_pop(
+ &stream_global->incoming_frames)) != NULL) {
+ incoming_byte_stream_destroy_locked(exec_ctx, NULL, NULL, bs);
+ }
+ if (stream_global->exceeded_metadata_size) {
+ cancel_from_api(exec_ctx, transport_global, stream_global,
+ GRPC_STATUS_RESOURCE_EXHAUSTED);
+ }
+ }
grpc_chttp2_incoming_metadata_buffer_publish(
&stream_global->received_initial_metadata,
stream_global->recv_initial_metadata);
@@ -1170,10 +1219,15 @@
}
if (stream_global->recv_trailing_metadata_finished != NULL &&
stream_global->read_closed && stream_global->write_closed) {
- while (stream_global->seen_error &&
- (bs = grpc_chttp2_incoming_frame_queue_pop(
- &stream_global->incoming_frames)) != NULL) {
- incoming_byte_stream_destroy_locked(exec_ctx, NULL, NULL, bs);
+ if (stream_global->seen_error) {
+ while ((bs = grpc_chttp2_incoming_frame_queue_pop(
+ &stream_global->incoming_frames)) != NULL) {
+ incoming_byte_stream_destroy_locked(exec_ctx, NULL, NULL, bs);
+ }
+ if (stream_global->exceeded_metadata_size) {
+ cancel_from_api(exec_ctx, transport_global, stream_global,
+ GRPC_STATUS_RESOURCE_EXHAUSTED);
+ }
}
if (stream_global->all_incoming_byte_streams_finished) {
grpc_chttp2_incoming_metadata_buffer_publish(
@@ -1249,7 +1303,7 @@
NULL);
}
if (status != GRPC_STATUS_OK && !stream_global->seen_error) {
- stream_global->seen_error = 1;
+ stream_global->seen_error = true;
grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
}
grpc_chttp2_mark_stream_closed(exec_ctx, transport_global, stream_global, 1,
@@ -1261,7 +1315,7 @@
grpc_chttp2_stream_global *stream_global,
grpc_status_code status, gpr_slice *slice) {
if (status != GRPC_STATUS_OK) {
- stream_global->seen_error = 1;
+ stream_global->seen_error = true;
grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
}
/* stream_global->recv_trailing_metadata_finished gives us a
diff --git a/src/core/ext/transport/chttp2/transport/frame_settings.c b/src/core/ext/transport/chttp2/transport/frame_settings.c
index a3c1e15..7fa6624 100644
--- a/src/core/ext/transport/chttp2/transport/frame_settings.c
+++ b/src/core/ext/transport/chttp2/transport/frame_settings.c
@@ -44,6 +44,7 @@
#include "src/core/ext/transport/chttp2/transport/http2_errors.h"
#include "src/core/lib/debug/trace.h"
+#define DEFAULT_MAX_HEADER_LIST_SIZE (16 * 1024)
#define MAX_MAX_HEADER_LIST_SIZE (1024 * 1024 * 1024)
/* HTTP/2 mandated initial connection settings */
@@ -62,7 +63,7 @@
GRPC_CHTTP2_FLOW_CONTROL_ERROR},
{"MAX_FRAME_SIZE", 16384, 16384, 16777215,
GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE, GRPC_CHTTP2_PROTOCOL_ERROR},
- {"MAX_HEADER_LIST_SIZE", MAX_MAX_HEADER_LIST_SIZE, 0,
+ {"MAX_HEADER_LIST_SIZE", DEFAULT_MAX_HEADER_LIST_SIZE, 0,
MAX_MAX_HEADER_LIST_SIZE, GRPC_CHTTP2_CLAMP_INVALID_VALUE,
GRPC_CHTTP2_PROTOCOL_ERROR},
};
diff --git a/src/core/ext/transport/chttp2/transport/incoming_metadata.c b/src/core/ext/transport/chttp2/transport/incoming_metadata.c
index db21744..3e463a7 100644
--- a/src/core/ext/transport/chttp2/transport/incoming_metadata.c
+++ b/src/core/ext/transport/chttp2/transport/incoming_metadata.c
@@ -65,6 +65,7 @@
gpr_realloc(buffer->elems, sizeof(*buffer->elems) * buffer->capacity);
}
buffer->elems[buffer->count++].md = elem;
+ buffer->size += GRPC_MDELEM_LENGTH(elem);
}
void grpc_chttp2_incoming_metadata_buffer_set_deadline(
diff --git a/src/core/ext/transport/chttp2/transport/incoming_metadata.h b/src/core/ext/transport/chttp2/transport/incoming_metadata.h
index 17ecf8e..7db5db8 100644
--- a/src/core/ext/transport/chttp2/transport/incoming_metadata.h
+++ b/src/core/ext/transport/chttp2/transport/incoming_metadata.h
@@ -42,6 +42,7 @@
size_t capacity;
gpr_timespec deadline;
int published;
+ size_t size; /* total size of metadata */
} grpc_chttp2_incoming_metadata_buffer;
/** assumes everything initially zeroed */
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index 7a80846..d8f17a3 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -432,6 +432,7 @@
/** has this stream seen an error? if 1, then pending incoming frames
can be thrown away */
bool seen_error;
+ bool exceeded_metadata_size;
bool published_initial_metadata;
bool published_trailing_metadata;
@@ -478,7 +479,8 @@
/** which metadata did we get (on this parse) */
uint8_t got_metadata_on_parse[2];
/** should we raise the seen_error flag in transport_global */
- uint8_t seen_error;
+ bool seen_error;
+ bool exceeded_metadata_size;
/** window available for peer to send to us */
int64_t incoming_window;
/** parsing state for data frames */
diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c
index e827a43..f101873 100644
--- a/src/core/ext/transport/chttp2/transport/parsing.c
+++ b/src/core/ext/transport/chttp2/transport/parsing.c
@@ -45,6 +45,10 @@
#include "src/core/lib/profiling/timers.h"
#include "src/core/lib/transport/static_metadata.h"
+#define TRANSPORT_FROM_PARSING(tp) \
+ ((grpc_chttp2_transport *)((char *)(tp)-offsetof(grpc_chttp2_transport, \
+ parsing)))
+
static int init_frame_parser(grpc_exec_ctx *exec_ctx,
grpc_chttp2_transport_parsing *transport_parsing);
static int init_header_frame_parser(
@@ -167,7 +171,9 @@
while (grpc_chttp2_list_pop_parsing_seen_stream(
transport_global, transport_parsing, &stream_global, &stream_parsing)) {
if (stream_parsing->seen_error) {
- stream_global->seen_error = 1;
+ stream_global->seen_error = true;
+ stream_global->exceeded_metadata_size =
+ stream_parsing->exceeded_metadata_size;
grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
}
@@ -603,7 +609,7 @@
if (md->key == GRPC_MDSTR_GRPC_STATUS && md != GRPC_MDELEM_GRPC_STATUS_0) {
/* TODO(ctiller): check for a status like " 0" */
- stream_parsing->seen_error = 1;
+ stream_parsing->seen_error = true;
}
if (md->key == GRPC_MDSTR_GRPC_TIMEOUT) {
@@ -624,8 +630,19 @@
gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), *cached_timeout));
GRPC_MDELEM_UNREF(md);
} else {
- grpc_chttp2_incoming_metadata_buffer_add(
- &stream_parsing->metadata_buffer[0], md);
+ const size_t new_size = stream_parsing->metadata_buffer[0].size +
+ GRPC_MDELEM_LENGTH(md);
+ grpc_chttp2_transport_global *transport_global =
+ &TRANSPORT_FROM_PARSING(transport_parsing)->global;
+ if (new_size > transport_global->settings
+ [GRPC_LOCAL_SETTINGS][GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE]) {
+ stream_parsing->seen_error = true;
+ stream_parsing->exceeded_metadata_size = true;
+ GRPC_MDELEM_UNREF(md);
+ } else {
+ grpc_chttp2_incoming_metadata_buffer_add(
+ &stream_parsing->metadata_buffer[0], md);
+ }
}
grpc_chttp2_list_add_parsing_seen_stream(transport_parsing, stream_parsing);
@@ -649,11 +666,22 @@
if (md->key == GRPC_MDSTR_GRPC_STATUS && md != GRPC_MDELEM_GRPC_STATUS_0) {
/* TODO(ctiller): check for a status like " 0" */
- stream_parsing->seen_error = 1;
+ stream_parsing->seen_error = true;
}
- grpc_chttp2_incoming_metadata_buffer_add(&stream_parsing->metadata_buffer[1],
- md);
+ const size_t new_size = stream_parsing->metadata_buffer[1].size +
+ GRPC_MDELEM_LENGTH(md);
+ grpc_chttp2_transport_global *transport_global =
+ &TRANSPORT_FROM_PARSING(transport_parsing)->global;
+ if (new_size > transport_global->settings
+ [GRPC_LOCAL_SETTINGS][GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE]) {
+ stream_parsing->seen_error = true;
+ stream_parsing->exceeded_metadata_size = true;
+ GRPC_MDELEM_UNREF(md);
+ } else {
+ grpc_chttp2_incoming_metadata_buffer_add(
+ &stream_parsing->metadata_buffer[1], md);
+ }
grpc_chttp2_list_add_parsing_seen_stream(transport_parsing, stream_parsing);
diff --git a/src/core/lib/transport/metadata.h b/src/core/lib/transport/metadata.h
index 713d9e6..4ecbbd1 100644
--- a/src/core/lib/transport/metadata.h
+++ b/src/core/lib/transport/metadata.h
@@ -147,6 +147,10 @@
#define GRPC_MDSTR_LENGTH(s) (GPR_SLICE_LENGTH(s->slice))
+/* We add 32 bytes of padding as per RFC-7540 section 6.5.2. */
+#define GRPC_MDELEM_LENGTH(e) (GRPC_MDSTR_LENGTH((e)->key) + \
+ GRPC_MDSTR_LENGTH((e)->value) + 32)
+
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);
diff --git a/src/core/lib/transport/metadata_batch.c b/src/core/lib/transport/metadata_batch.c
index 4567221..4e1cd8e 100644
--- a/src/core/lib/transport/metadata_batch.c
+++ b/src/core/lib/transport/metadata_batch.c
@@ -192,3 +192,12 @@
gpr_time_cmp(gpr_inf_future(batch->deadline.clock_type),
batch->deadline) == 0;
}
+
+size_t grpc_metadata_batch_size(grpc_metadata_batch *batch) {
+ size_t size = 0;
+ for (grpc_linked_mdelem* elem = batch->list.head;
+ elem != NULL; elem = elem->next) {
+ size += GRPC_MDELEM_LENGTH(elem->md);
+ }
+ return size;
+}
diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h
index b626688..7af823f 100644
--- a/src/core/lib/transport/metadata_batch.h
+++ b/src/core/lib/transport/metadata_batch.h
@@ -66,6 +66,9 @@
void grpc_metadata_batch_clear(grpc_metadata_batch *batch);
int grpc_metadata_batch_is_empty(grpc_metadata_batch *batch);
+/* Returns the transport size of the batch. */
+size_t grpc_metadata_batch_size(grpc_metadata_batch *batch);
+
/** Moves the metadata information from \a src to \a dst. Upon return, \a src is
* zeroed. */
void grpc_metadata_batch_move(grpc_metadata_batch *dst,
diff --git a/test/core/end2end/cq_verifier.c b/test/core/end2end/cq_verifier.c
index 77afe58..5f1a332 100644
--- a/test/core/end2end/cq_verifier.c
+++ b/test/core/end2end/cq_verifier.c
@@ -107,6 +107,14 @@
return has_metadata(array->metadata, array->count, key, value);
}
+int contains_metadata_key(grpc_metadata_array *array, const char *key) {
+ for (size_t i = 0; i < array->count; ++i) {
+ if (strcmp(array->metadata[i].key, key) == 0)
+ return 1;
+ }
+ return 0;
+}
+
static gpr_slice merge_slices(gpr_slice *slices, size_t nslices) {
size_t i;
size_t len = 0;
diff --git a/test/core/end2end/cq_verifier.h b/test/core/end2end/cq_verifier.h
index b3e07c4..a540659 100644
--- a/test/core/end2end/cq_verifier.h
+++ b/test/core/end2end/cq_verifier.h
@@ -62,5 +62,6 @@
int byte_buffer_eq_string(grpc_byte_buffer *byte_buffer, const char *string);
int contains_metadata(grpc_metadata_array *array, const char *key,
const char *value);
+int contains_metadata_key(grpc_metadata_array *array, const char *key);
#endif /* GRPC_TEST_CORE_END2END_CQ_VERIFIER_H */
diff --git a/test/core/end2end/tests/large_metadata.c b/test/core/end2end/tests/large_metadata.c
index 0e5d6b4..b78d5b8 100644
--- a/test/core/end2end/tests/large_metadata.c
+++ b/test/core/end2end/tests/large_metadata.c
@@ -97,7 +97,7 @@
grpc_completion_queue_destroy(f->cq);
}
-/* Request with a large amount of metadata.*/
+/* Request with a large amount of metadata. */
static void test_request_with_large_metadata(grpc_end2end_test_config config) {
grpc_call *c;
grpc_call *s;
@@ -106,8 +106,12 @@
grpc_raw_byte_buffer_create(&request_payload_slice, 1);
gpr_timespec deadline = five_seconds_time();
grpc_metadata meta;
- grpc_end2end_test_fixture f =
- begin_test(config, "test_request_with_large_metadata", NULL, NULL);
+ const size_t large_size = 64 * 1024;
+ grpc_arg arg = { GRPC_ARG_INTEGER, GRPC_ARG_MAX_METADATA_SIZE,
+ { .integer=(int)large_size + 1024 } };
+ grpc_channel_args args = { 1, &arg };
+ grpc_end2end_test_fixture f = begin_test(
+ config, "test_request_with_large_metadata", &args, &args);
cq_verifier *cqv = cq_verifier_create(f.cq);
grpc_op ops[6];
grpc_op *op;
@@ -121,7 +125,6 @@
char *details = NULL;
size_t details_capacity = 0;
int was_cancelled = 2;
- const size_t large_size = 64 * 1024;
c = grpc_channel_create_call(f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq,
"/foo", "foo.test.google.fr", deadline, NULL);
@@ -138,6 +141,7 @@
grpc_metadata_array_init(&request_metadata_recv);
grpc_call_details_init(&call_details);
+ /* Client: send request. */
op = ops;
op->op = GRPC_OP_SEND_INITIAL_METADATA;
op->data.send_initial_metadata.count = 1;
@@ -174,9 +178,11 @@
grpc_server_request_call(f.server, &s, &call_details,
&request_metadata_recv, f.cq, f.cq, tag(101));
GPR_ASSERT(GRPC_CALL_OK == error);
+
cq_expect_completion(cqv, tag(101), 1);
cq_verify(cqv);
+ /* Server: send initial metadata and receive request. */
op = ops;
op->op = GRPC_OP_SEND_INITIAL_METADATA;
op->data.send_initial_metadata.count = 0;
@@ -194,6 +200,8 @@
cq_expect_completion(cqv, tag(102), 1);
cq_verify(cqv);
+ /* Server: receive close and send status. This should trigger
+ completion of request on client. */
op = ops;
op->op = GRPC_OP_RECV_CLOSE_ON_SERVER;
op->data.recv_close_on_server.cancelled = &was_cancelled;