From 49772e00eb9606c3192b88f348d9cbcb22eb93c2 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Aug 2015 08:08:37 -0700 Subject: Outlaw illegal metadata characters --- src/core/channel/compress_filter.h | 2 +- src/core/surface/call.c | 5 +++-- src/core/transport/metadata.c | 26 +++++++++++++++++++++++--- src/core/transport/metadata.h | 1 + 4 files changed, 28 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/core/channel/compress_filter.h b/src/core/channel/compress_filter.h index 0917e81ca4..415459bca6 100644 --- a/src/core/channel/compress_filter.h +++ b/src/core/channel/compress_filter.h @@ -36,7 +36,7 @@ #include "src/core/channel/channel_stack.h" -#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "internal:grpc-encoding-request" +#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "grpc-internal-encoding-request" /** Compression filter for outgoing data. * diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 33f277da46..4426bbbce9 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -1046,10 +1046,11 @@ static int prepare_application_metadata(grpc_call *call, size_t count, (const gpr_uint8 *)md->value, md->value_length, 1); if (!grpc_mdstr_is_legal_header(l->md->key)) { - gpr_log(GPR_ERROR, "attempt to send invalid metadata key"); + gpr_log(GPR_ERROR, "attempt to send invalid metadata key: %s", + grpc_mdstr_as_c_string(l->md->key)); return 0; } else if (!grpc_mdstr_is_bin_suffixed(l->md->key) && - !grpc_mdstr_is_legal_header(l->md->value)) { + !grpc_mdstr_is_legal_nonbin_header(l->md->value)) { gpr_log(GPR_ERROR, "attempt to send invalid metadata value"); return 0; } diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c index f92e87e9dd..d758351feb 100644 --- a/src/core/transport/metadata.c +++ b/src/core/transport/metadata.c @@ -681,16 +681,36 @@ void grpc_mdctx_locked_mdelem_unref(grpc_mdctx *ctx, void grpc_mdctx_unlock(grpc_mdctx *ctx) { unlock(ctx); } -int grpc_mdstr_is_legal_header(grpc_mdstr *s) { - /* TODO(ctiller): consider caching this, or computing it on construction */ +static int conforms_to(grpc_mdstr *s, const gpr_uint8 *legal_bits) { const gpr_uint8 *p = GPR_SLICE_START_PTR(s->slice); const gpr_uint8 *e = GPR_SLICE_END_PTR(s->slice); for (; p != e; p++) { - if (*p < 32 || *p > 126) return 0; + int idx = *p; + int byte = idx / 8; + int bit = idx % 8; + if ((legal_bits[byte] & (1 << bit)) == 0) return 0; } return 1; } +int grpc_mdstr_is_legal_header(grpc_mdstr *s) { + static const gpr_uint8 legal_header_bits[256 / 8] = { + 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xff, 0x03, 0xfe, 0xff, 0xff, + 0x07, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + + /* TODO(ctiller): consider caching this, or computing it on construction */ + return conforms_to(s, legal_header_bits); +} + +int grpc_mdstr_is_legal_nonbin_header(grpc_mdstr *s) { + static const gpr_uint8 legal_header_bits[256 / 8] = { + 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + return conforms_to(s, legal_header_bits); +} + int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s) { /* TODO(ctiller): consider caching this */ return grpc_is_binary_header((const char *)GPR_SLICE_START_PTR(s->slice), diff --git a/src/core/transport/metadata.h b/src/core/transport/metadata.h index a7af49ba55..eb17747be7 100644 --- a/src/core/transport/metadata.h +++ b/src/core/transport/metadata.h @@ -154,6 +154,7 @@ void grpc_mdelem_unref(grpc_mdelem *md); const char *grpc_mdstr_as_c_string(grpc_mdstr *s); 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); /* Batch mode metadata functions. -- cgit v1.2.3 From 80aa28013fb3290f69c0452116ab3dd0b95566cc Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Aug 2015 08:50:51 -0700 Subject: Fix character classes to updated spec --- src/core/transport/metadata.c | 6 ++---- tools/codegen/core/gen_legal_metadata_characters.c | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c index d758351feb..3fd21a2f5d 100644 --- a/src/core/transport/metadata.c +++ b/src/core/transport/metadata.c @@ -695,11 +695,9 @@ static int conforms_to(grpc_mdstr *s, const gpr_uint8 *legal_bits) { int grpc_mdstr_is_legal_header(grpc_mdstr *s) { static const gpr_uint8 legal_header_bits[256 / 8] = { - 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xff, 0x03, 0xfe, 0xff, 0xff, - 0x07, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xff, 0x03, 0x00, 0x00, 0x00, + 0x80, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; - - /* TODO(ctiller): consider caching this, or computing it on construction */ return conforms_to(s, legal_header_bits); } diff --git a/tools/codegen/core/gen_legal_metadata_characters.c b/tools/codegen/core/gen_legal_metadata_characters.c index 5e32d91d50..5c290f2923 100644 --- a/tools/codegen/core/gen_legal_metadata_characters.c +++ b/tools/codegen/core/gen_legal_metadata_characters.c @@ -60,9 +60,9 @@ int main(void) { clear(); for (i = 'a'; i <= 'z'; i++) legal(i); - for (i = 'A'; i <= 'Z'; i++) legal(i); for (i = '0'; i <= '9'; i++) legal(i); legal('-'); + legal('_'); dump(); clear(); -- cgit v1.2.3 From 987263a039f0e707ae31d163fbdb94640efbd588 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Aug 2015 16:14:43 -0700 Subject: Lower-case string --- src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py b/src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py index 9bab930e56..f1bec238cf 100644 --- a/src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py +++ b/src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py @@ -76,7 +76,7 @@ class InsecureServerInsecureClient(unittest.TestCase): CLIENT_METADATA_BIN_VALUE = b'\0'*1000 SERVER_INITIAL_METADATA_KEY = 'init_me_me_me' SERVER_INITIAL_METADATA_VALUE = 'whodawha?' - SERVER_TRAILING_METADATA_KEY = 'California_is_in_a_drought' + SERVER_TRAILING_METADATA_KEY = 'california_is_in_a_drought' SERVER_TRAILING_METADATA_VALUE = 'zomg it is' SERVER_STATUS_CODE = _types.StatusCode.OK SERVER_STATUS_DETAILS = 'our work is never over' -- cgit v1.2.3 From b3e56c473699393698514d7fcd9289befbdee66c Mon Sep 17 00:00:00 2001 From: Nathaniel Manista Date: Tue, 25 Aug 2015 15:41:44 +0000 Subject: Drop whitespace from Python test metadata keys Whitespace is now disallowed in metadata keys. --- .../grpcio_test/grpc_test/_adapter/_links_test.py | 4 ++-- .../grpcio_test/grpc_test/_links/_transmission_test.py | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/python/grpcio_test/grpc_test/_adapter/_links_test.py b/src/python/grpcio_test/grpc_test/_adapter/_links_test.py index c4686b327a..4077b8e350 100644 --- a/src/python/grpcio_test/grpc_test/_adapter/_links_test.py +++ b/src/python/grpcio_test/grpc_test/_adapter/_links_test.py @@ -46,8 +46,8 @@ _TIMEOUT = 32 # TODO(nathaniel): End-to-end metadata testing. def _transform_metadata(unused_metadata): return ( - ('one unused key', 'one unused value'), - ('another unused key', 'another unused value'), + ('one_unused_key', 'one unused value'), + ('another_unused_key', 'another unused value'), ) diff --git a/src/python/grpcio_test/grpc_test/_links/_transmission_test.py b/src/python/grpcio_test/grpc_test/_links/_transmission_test.py index 9cdc9620f0..75580e2250 100644 --- a/src/python/grpcio_test/grpc_test/_links/_transmission_test.py +++ b/src/python/grpcio_test/grpc_test/_links/_transmission_test.py @@ -66,9 +66,9 @@ class TransmissionTest(test_cases.TransmissionTest, unittest.TestCase): def create_invocation_initial_metadata(self): return ( - ('first invocation initial metadata key', 'just a string value'), - ('second invocation initial metadata key', '0123456789'), - ('third invocation initial metadata key-bin', '\x00\x57' * 100), + ('first_invocation_initial_metadata_key', 'just a string value'), + ('second_invocation_initial_metadata_key', '0123456789'), + ('third_invocation_initial_metadata_key-bin', '\x00\x57' * 100), ) def create_invocation_terminal_metadata(self): @@ -76,16 +76,16 @@ class TransmissionTest(test_cases.TransmissionTest, unittest.TestCase): def create_service_initial_metadata(self): return ( - ('first service initial metadata key', 'just another string value'), - ('second service initial metadata key', '9876543210'), - ('third service initial metadata key-bin', '\x00\x59\x02' * 100), + ('first_service_initial_metadata_key', 'just another string value'), + ('second_service_initial_metadata_key', '9876543210'), + ('third_service_initial_metadata_key-bin', '\x00\x59\x02' * 100), ) def create_service_terminal_metadata(self): return ( - ('first service terminal metadata key', 'yet another string value'), - ('second service terminal metadata key', 'abcdefghij'), - ('third service terminal metadata key-bin', '\x00\x37' * 100), + ('first_service_terminal_metadata_key', 'yet another string value'), + ('second_service_terminal_metadata_key', 'abcdefghij'), + ('third_service_terminal_metadata_key-bin', '\x00\x37' * 100), ) def create_invocation_completion(self): -- cgit v1.2.3