Skip to content

Commit

Permalink
Merge pull request grpc#6320 from markdroth/limit_metadata_size
Browse files Browse the repository at this point in the history 
Add support for max metadata size.
  • Loading branch information
@jtattermusch
jtattermusch committed May 10, 2016
2 parents 5e743b2 + 067cce5 commit cfbe020
Show file tree
Hide file tree
Showing 39 changed files with 1,349 additions and 247 deletions.
24 changes: 24 additions & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,7 @@ connection_prefix_bad_client_test: $(BINDIR)/$(CONFIG)/connection_prefix_bad_cli
head_of_line_blocking_bad_client_test: $(BINDIR)/$(CONFIG)/head_of_line_blocking_bad_client_test
headers_bad_client_test: $(BINDIR)/$(CONFIG)/headers_bad_client_test
initial_settings_frame_bad_client_test: $(BINDIR)/$(CONFIG)/initial_settings_frame_bad_client_test
large_metadata_bad_client_test: $(BINDIR)/$(CONFIG)/large_metadata_bad_client_test
server_registered_method_bad_client_test: $(BINDIR)/$(CONFIG)/server_registered_method_bad_client_test
simple_request_bad_client_test: $(BINDIR)/$(CONFIG)/simple_request_bad_client_test
unknown_frame_bad_client_test: $(BINDIR)/$(CONFIG)/unknown_frame_bad_client_test
Expand Down Expand Up @@ -1318,6 +1319,7 @@ buildtests_c: privatelibs_c \
$(BINDIR)/$(CONFIG)/head_of_line_blocking_bad_client_test \
$(BINDIR)/$(CONFIG)/headers_bad_client_test \
$(BINDIR)/$(CONFIG)/initial_settings_frame_bad_client_test \
$(BINDIR)/$(CONFIG)/large_metadata_bad_client_test \
$(BINDIR)/$(CONFIG)/server_registered_method_bad_client_test \
$(BINDIR)/$(CONFIG)/simple_request_bad_client_test \
$(BINDIR)/$(CONFIG)/unknown_frame_bad_client_test \
Expand Down Expand Up @@ -1656,6 +1658,8 @@ test_c: buildtests_c
$(Q) $(BINDIR)/$(CONFIG)/headers_bad_client_test || ( echo test headers_bad_client_test failed ; exit 1 )
$(E) "[RUN] Testing initial_settings_frame_bad_client_test"
$(Q) $(BINDIR)/$(CONFIG)/initial_settings_frame_bad_client_test || ( echo test initial_settings_frame_bad_client_test failed ; exit 1 )
$(E) "[RUN] Testing large_metadata_bad_client_test"
$(Q) $(BINDIR)/$(CONFIG)/large_metadata_bad_client_test || ( echo test large_metadata_bad_client_test failed ; exit 1 )
$(E) "[RUN] Testing server_registered_method_bad_client_test"
$(Q) $(BINDIR)/$(CONFIG)/server_registered_method_bad_client_test || ( echo test server_registered_method_bad_client_test failed ; exit 1 )
$(E) "[RUN] Testing simple_request_bad_client_test"
Expand Down Expand Up @@ -13104,6 +13108,26 @@ ifneq ($(NO_DEPS),true)
endif


LARGE_METADATA_BAD_CLIENT_TEST_SRC = \
test/core/bad_client/tests/large_metadata.c \

LARGE_METADATA_BAD_CLIENT_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(LARGE_METADATA_BAD_CLIENT_TEST_SRC))))


$(BINDIR)/$(CONFIG)/large_metadata_bad_client_test: $(LARGE_METADATA_BAD_CLIENT_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libbad_client_test.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util_unsecure.a $(LIBDIR)/$(CONFIG)/libgrpc_unsecure.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a
$(E) "[LD] Linking $@"
$(Q) mkdir -p `dirname $@`
$(Q) $(LD) $(LDFLAGS) $(LARGE_METADATA_BAD_CLIENT_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libbad_client_test.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util_unsecure.a $(LIBDIR)/$(CONFIG)/libgrpc_unsecure.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) -o $(BINDIR)/$(CONFIG)/large_metadata_bad_client_test

$(OBJDIR)/$(CONFIG)/test/core/bad_client/tests/large_metadata.o: $(LIBDIR)/$(CONFIG)/libbad_client_test.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util_unsecure.a $(LIBDIR)/$(CONFIG)/libgrpc_unsecure.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a

deps_large_metadata_bad_client_test: $(LARGE_METADATA_BAD_CLIENT_TEST_OBJS:.o=.dep)

ifneq ($(NO_DEPS),true)
-include $(LARGE_METADATA_BAD_CLIENT_TEST_OBJS:.o=.dep)
endif


SERVER_REGISTERED_METHOD_BAD_CLIENT_TEST_SRC = \
test/core/bad_client/tests/server_registered_method.c \

Expand Down
2 changes: 2 additions & 0 deletions include/grpc/impl/codegen/grpc_types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ typedef struct {
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.
Expand Down
5 changes: 2 additions & 3 deletions include/grpc/impl/codegen/slice_buffer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@ extern "C" {

#define GRPC_SLICE_BUFFER_INLINE_ELEMENTS 8

/* Represents an expandable array of slices, to be interpreted as a single item
TODO(ctiller): inline some small number of elements into the struct, to
avoid per-call allocations */
/* Represents an expandable array of slices, to be interpreted as a
single item. */
typedef struct {
/* slices in the array */
gpr_slice *slices;
Expand Down
150 changes: 105 additions & 45 deletions src/core/ext/transport/chttp2/transport/chttp2_transport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
#define DEFAULT_CONNECTION_WINDOW_TARGET (1024 * 1024)
#define MAX_WINDOW 0x7fffffffu

#define DEFAULT_MAX_HEADER_LIST_SIZE (16 * 1024)

#define MAX_CLIENT_STREAM_ID 0x7fffffffu

int grpc_http_trace = 0;
Expand All @@ -65,8 +67,8 @@ int grpc_flowctl_trace = 0;
((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) \
Expand Down Expand Up @@ -311,6 +313,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
push_setting(t, GRPC_CHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, 0);
}
push_setting(t, GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, DEFAULT_WINDOW);
push_setting(t, GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE,
DEFAULT_MAX_HEADER_LIST_SIZE);

if (channel_args) {
for (i = 0; i < channel_args->num_args; i++) {
Expand Down Expand Up @@ -378,6 +382,18 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
&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);
}
}
}
}
Expand Down Expand Up @@ -510,7 +526,7 @@ static int init_stream(grpc_exec_ctx *exec_ctx, grpc_transport *gt,
[GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE];
*t->accepting_stream = s;
grpc_chttp2_stream_map_add(&t->parsing_stream_map, s->global.id, s);
s->global.in_stream_map = 1;
s->global.in_stream_map = true;
}

grpc_chttp2_run_with_global_lock(exec_ctx, t, s, finish_init_stream_locked,
Expand Down Expand Up @@ -834,7 +850,7 @@ static void maybe_start_some_streams(
grpc_chttp2_stream_map_add(
&TRANSPORT_FROM_GLOBAL(transport_global)->new_stream_map,
stream_global->id, STREAM_FROM_GLOBAL(stream_global));
stream_global->in_stream_map = 1;
stream_global->in_stream_map = true;
transport_global->concurrent_stream_count++;
grpc_chttp2_become_writable(transport_global, stream_global);
}
Expand Down Expand Up @@ -933,24 +949,38 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx,
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);
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,
"to-be-sent 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 {
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 {
GPR_ASSERT(stream_global->id != 0);
grpc_chttp2_become_writable(transport_global, stream_global);
grpc_chttp2_complete_closure_step(
exec_ctx, stream_global,
&stream_global->send_initial_metadata_finished, 0);
}
} else {
grpc_chttp2_complete_closure_step(
exec_ctx, stream_global,
&stream_global->send_initial_metadata_finished, 0);
}
}

Expand All @@ -974,19 +1004,34 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx,
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,
"to-be-sent 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);
}
}
}

Expand Down Expand Up @@ -1149,6 +1194,16 @@ static void check_read_ops(grpc_exec_ctx *exec_ctx,
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);
Expand Down Expand Up @@ -1178,10 +1233,15 @@ static void check_read_ops(grpc_exec_ctx *exec_ctx,
}
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(
Expand Down Expand Up @@ -1213,7 +1273,7 @@ static void remove_stream(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
s = grpc_chttp2_stream_map_delete(&t->new_stream_map, id);
}
GPR_ASSERT(s);
s->global.in_stream_map = 0;
s->global.in_stream_map = false;
if (t->parsing.incoming_stream == &s->parsing) {
t->parsing.incoming_stream = NULL;
grpc_chttp2_parsing_become_skip_parser(exec_ctx, &t->parsing);
Expand Down Expand Up @@ -1257,7 +1317,7 @@ static void cancel_from_api(grpc_exec_ctx *exec_ctx,
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,
Expand All @@ -1269,7 +1329,7 @@ void grpc_chttp2_fake_status(grpc_exec_ctx *exec_ctx,
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
Expand All @@ -1293,7 +1353,7 @@ void grpc_chttp2_fake_status(grpc_exec_ctx *exec_ctx,
GRPC_MDSTR_GRPC_MESSAGE,
grpc_mdstr_from_slice(gpr_slice_ref(*slice))));
}
stream_global->published_trailing_metadata = 1;
stream_global->published_trailing_metadata = true;
grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
}
if (slice) {
Expand Down Expand Up @@ -1323,13 +1383,13 @@ void grpc_chttp2_mark_stream_closed(
}
grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
if (close_reads && !stream_global->read_closed) {
stream_global->read_closed = 1;
stream_global->published_initial_metadata = 1;
stream_global->published_trailing_metadata = 1;
stream_global->read_closed = true;
stream_global->published_initial_metadata = true;
stream_global->published_trailing_metadata = true;
decrement_active_streams_locked(exec_ctx, transport_global, stream_global);
}
if (close_writes && !stream_global->write_closed) {
stream_global->write_closed = 1;
stream_global->write_closed = true;
if (TRANSPORT_FROM_GLOBAL(transport_global)->executor.writing_active) {
GRPC_CHTTP2_STREAM_REF(stream_global, "finish_writes");
grpc_chttp2_list_add_closed_waiting_for_writing(transport_global,
Expand Down
5 changes: 5 additions & 0 deletions src/core/ext/transport/chttp2/transport/frame_rst_stream.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,20 @@ gpr_slice grpc_chttp2_rst_stream_create(uint32_t id, uint32_t code,
stats->framing_bytes += frame_size;
uint8_t *p = GPR_SLICE_START_PTR(slice);

// Frame size.
*p++ = 0;
*p++ = 0;
*p++ = 4;
// Frame type.
*p++ = GRPC_CHTTP2_FRAME_RST_STREAM;
// Flags.
*p++ = 0;
// Stream ID.
*p++ = (uint8_t)(id >> 24);
*p++ = (uint8_t)(id >> 16);
*p++ = (uint8_t)(id >> 8);
*p++ = (uint8_t)(id);
// Error code.
*p++ = (uint8_t)(code >> 24);
*p++ = (uint8_t)(code >> 16);
*p++ = (uint8_t)(code >> 8);
Expand Down
1 change: 1 addition & 0 deletions src/core/ext/transport/chttp2/transport/incoming_metadata.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ void grpc_chttp2_incoming_metadata_buffer_add(
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(
Expand Down
1 change: 1 addition & 0 deletions src/core/ext/transport/chttp2/transport/incoming_metadata.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ typedef struct {
size_t capacity;
gpr_timespec deadline;
int published;
size_t size; // total size of metadata
} grpc_chttp2_incoming_metadata_buffer;

/** assumes everything initially zeroed */
Expand Down
Loading

0 comments on commit cfbe020

Please sign in to comment.