From 303d3082a07363c29dc747e986658fd6c8dc4053 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Thu, 5 May 2016 18:25:34 -0700 Subject: Fixed compression interop and re-enable for C++. Also added some defense in depth for compression algorithms in the receive path. --- src/core/lib/surface/byte_buffer_reader.c | 19 +++++++++++++------ src/core/lib/surface/call.c | 9 +++++++-- 2 files changed, 20 insertions(+), 8 deletions(-) (limited to 'src/core/lib/surface') diff --git a/src/core/lib/surface/byte_buffer_reader.c b/src/core/lib/surface/byte_buffer_reader.c index 809fd5f1fa..c7f941525d 100644 --- a/src/core/lib/surface/byte_buffer_reader.c +++ b/src/core/lib/surface/byte_buffer_reader.c @@ -62,12 +62,19 @@ void grpc_byte_buffer_reader_init(grpc_byte_buffer_reader *reader, case GRPC_BB_RAW: gpr_slice_buffer_init(&decompressed_slices_buffer); if (is_compressed(reader->buffer_in)) { - grpc_msg_decompress(reader->buffer_in->data.raw.compression, - &reader->buffer_in->data.raw.slice_buffer, - &decompressed_slices_buffer); - reader->buffer_out = - grpc_raw_byte_buffer_create(decompressed_slices_buffer.slices, - decompressed_slices_buffer.count); + if (grpc_msg_decompress(reader->buffer_in->data.raw.compression, + &reader->buffer_in->data.raw.slice_buffer, + &decompressed_slices_buffer) < 0) { + gpr_log(GPR_ERROR, + "Unexpected error decompressing data for algorithm with enum " + "value '%d'. Reading data as if it were uncompressed.", + reader->buffer_in->data.raw.compression); + reader->buffer_out = reader->buffer_in; + } else { /* all fine */ + reader->buffer_out = + grpc_raw_byte_buffer_create(decompressed_slices_buffer.slices, + decompressed_slices_buffer.count); + } gpr_slice_buffer_destroy(&decompressed_slices_buffer); } else { /* not compressed, use the input buffer as output */ reader->buffer_out = reader->buffer_in; diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 9b2b94eedf..778557121d 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -408,6 +408,7 @@ static void set_status_code(grpc_call *call, status_source source, static void set_compression_algorithm(grpc_call *call, grpc_compression_algorithm algo) { + GPR_ASSERT(algo < GRPC_COMPRESS_ALGORITHMS_COUNT); call->compression_algorithm = algo; } @@ -828,12 +829,16 @@ static uint32_t decode_status(grpc_mdelem *md) { return status; } -static uint32_t decode_compression(grpc_mdelem *md) { +static grpc_compression_algorithm decode_compression(grpc_mdelem *md) { grpc_compression_algorithm algorithm = grpc_compression_algorithm_from_mdstr(md->value); if (algorithm == GRPC_COMPRESS_ALGORITHMS_COUNT) { const char *md_c_str = grpc_mdstr_as_c_string(md->value); - gpr_log(GPR_ERROR, "Invalid compression algorithm: '%s'", md_c_str); + gpr_log(GPR_ERROR, + "Invalid incoming compression algorithm: '%s'. Interpreting " + "incoming data as uncompressed.", + md_c_str); + return GRPC_COMPRESS_NONE; } return algorithm; } -- cgit v1.2.3 From 461233770f2aafe3188efefcfff7deecee3e441b Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 9 May 2016 15:28:42 -0700 Subject: grpc-accept-encoding checks --- src/core/lib/channel/compress_filter.c | 6 +++--- src/core/lib/channel/compress_filter.h | 2 +- src/core/lib/surface/call.c | 23 ++++++++++++++++++++++- src/core/lib/surface/init.c | 2 +- 4 files changed, 27 insertions(+), 6 deletions(-) (limited to 'src/core/lib/surface') diff --git a/src/core/lib/channel/compress_filter.c b/src/core/lib/channel/compress_filter.c index 5510c79b18..e18bce7e56 100644 --- a/src/core/lib/channel/compress_filter.c +++ b/src/core/lib/channel/compress_filter.c @@ -47,7 +47,7 @@ #include "src/core/lib/support/string.h" #include "src/core/lib/transport/static_metadata.h" -int grpc_compress_filter_trace = 0; +int grpc_compression_trace = 0; typedef struct call_data { gpr_slice_buffer slices; /**< Buffers up input slices to be compressed */ @@ -171,7 +171,7 @@ static void finish_send_message(grpc_exec_ctx *exec_ctx, did_compress = grpc_msg_compress(calld->compression_algorithm, &calld->slices, &tmp); if (did_compress) { - if (grpc_compress_filter_trace) { + if (grpc_compression_trace) { char *algo_name; const size_t before_size = calld->slices.length; const size_t after_size = tmp.length; @@ -185,7 +185,7 @@ static void finish_send_message(grpc_exec_ctx *exec_ctx, gpr_slice_buffer_swap(&calld->slices, &tmp); calld->send_flags |= GRPC_WRITE_INTERNAL_COMPRESS; } else { - if (grpc_compress_filter_trace) { + if (grpc_compression_trace) { char *algo_name; GPR_ASSERT(grpc_compression_algorithm_name(calld->compression_algorithm, &algo_name)); diff --git a/src/core/lib/channel/compress_filter.h b/src/core/lib/channel/compress_filter.h index cf5879d82e..0ce5d08837 100644 --- a/src/core/lib/channel/compress_filter.h +++ b/src/core/lib/channel/compress_filter.h @@ -38,7 +38,7 @@ #define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "grpc-internal-encoding-request" -extern int grpc_compress_filter_trace; +extern int grpc_compression_trace; /** Compression filter for outgoing data. * diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 9b2b94eedf..296a5f0d51 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -261,6 +261,8 @@ grpc_call *grpc_call_create(grpc_channel *channel, grpc_call *parent_call, call->channel = channel; call->cq = cq; call->parent = parent_call; + /* Always support no compression */ + GPR_BITSET(&call->encodings_accepted_by_peer, GRPC_COMPRESS_NONE); call->is_client = server_transport_data == NULL; if (call->is_client) { GPR_ASSERT(add_initial_metadata_count < MAX_SEND_EXTRA_METADATA_COUNT); @@ -1087,6 +1089,24 @@ static void receiving_initial_metadata_ready(grpc_exec_ctx *exec_ctx, &call->metadata_batch[1 /* is_receiving */][0 /* is_trailing */]; grpc_metadata_batch_filter(md, recv_initial_filter, call); + /* make sure the received grpc-encoding is amongst the ones listed in + * grpc-accept-encoding */ + + GPR_ASSERT(call->encodings_accepted_by_peer != 0); + if (!GPR_BITGET(call->encodings_accepted_by_peer, + call->compression_algorithm)) { + extern int grpc_compression_trace; + if (grpc_compression_trace) { + char *algo_name; + grpc_compression_algorithm_name(call->compression_algorithm, + &algo_name); + gpr_log(GPR_ERROR, + "Compression algorithm (grpc-encoding = '%s') not present in " + "the bitset of accepted encodings (grpc-accept-encodings: " + "'0x%x')", + algo_name, call->encodings_accepted_by_peer); + } + } if (gpr_time_cmp(md->deadline, gpr_inf_future(md->deadline.clock_type)) != 0 && !call->is_client) { @@ -1474,7 +1494,8 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops, grpc_call_error err; GRPC_API_TRACE( - "grpc_call_start_batch(call=%p, ops=%p, nops=%lu, tag=%p, reserved=%p)", + "grpc_call_start_batch(call=%p, ops=%p, nops=%lu, tag=%p, " + "reserved=%p)", 5, (call, ops, (unsigned long)nops, tag, reserved)); if (reserved != NULL) { diff --git a/src/core/lib/surface/init.c b/src/core/lib/surface/init.c index 57c6897626..1c8b709015 100644 --- a/src/core/lib/surface/init.c +++ b/src/core/lib/surface/init.c @@ -164,7 +164,7 @@ void grpc_init(void) { grpc_register_tracer("channel_stack_builder", &grpc_trace_channel_stack_builder); grpc_register_tracer("http1", &grpc_http1_trace); - grpc_register_tracer("compression", &grpc_compress_filter_trace); + grpc_register_tracer("compression", &grpc_compression_trace); grpc_security_pre_init(); grpc_iomgr_init(); grpc_executor_init(); -- cgit v1.2.3 From 0b405d54d496acc331f1511353b26e9f9a0e4598 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 9 May 2016 15:58:22 -0700 Subject: fixed wrong change --- src/core/lib/compression/message_compress.c | 2 +- src/core/lib/surface/byte_buffer_reader.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core/lib/surface') diff --git a/src/core/lib/compression/message_compress.c b/src/core/lib/compression/message_compress.c index 699719a523..cbe0b5a285 100644 --- a/src/core/lib/compression/message_compress.c +++ b/src/core/lib/compression/message_compress.c @@ -194,5 +194,5 @@ int grpc_msg_decompress(grpc_compression_algorithm algorithm, break; } gpr_log(GPR_ERROR, "invalid compression algorithm %d", algorithm); - return -1; /* to distinguish it from GRPC_COMPRESS_NONE */ + return 0; } diff --git a/src/core/lib/surface/byte_buffer_reader.c b/src/core/lib/surface/byte_buffer_reader.c index c7f941525d..c97079f638 100644 --- a/src/core/lib/surface/byte_buffer_reader.c +++ b/src/core/lib/surface/byte_buffer_reader.c @@ -64,7 +64,7 @@ void grpc_byte_buffer_reader_init(grpc_byte_buffer_reader *reader, if (is_compressed(reader->buffer_in)) { if (grpc_msg_decompress(reader->buffer_in->data.raw.compression, &reader->buffer_in->data.raw.slice_buffer, - &decompressed_slices_buffer) < 0) { + &decompressed_slices_buffer) == 0) { gpr_log(GPR_ERROR, "Unexpected error decompressing data for algorithm with enum " "value '%d'. Reading data as if it were uncompressed.", -- cgit v1.2.3