diff --git a/BUILD b/BUILD index 9c9eaae1d9997a4348d168c54cb744ac24b3a78c..b8f19546970eecc9a3bdf43f34477125613c9b56 100644 --- a/BUILD +++ b/BUILD @@ -384,6 +384,7 @@ cc_library( "src/core/surface/server.c", "src/core/surface/server_chttp2.c", "src/core/surface/server_create.c", + "src/core/surface/validate_metadata.c", "src/core/surface/version.c", "src/core/transport/byte_stream.c", "src/core/transport/chttp2/alpn.c", @@ -655,6 +656,7 @@ cc_library( "src/core/surface/server.c", "src/core/surface/server_chttp2.c", "src/core/surface/server_create.c", + "src/core/surface/validate_metadata.c", "src/core/surface/version.c", "src/core/transport/byte_stream.c", "src/core/transport/chttp2/alpn.c", @@ -1189,6 +1191,7 @@ objc_library( "src/core/surface/server.c", "src/core/surface/server_chttp2.c", "src/core/surface/server_create.c", + "src/core/surface/validate_metadata.c", "src/core/surface/version.c", "src/core/transport/byte_stream.c", "src/core/transport/chttp2/alpn.c", diff --git a/Makefile b/Makefile index 02f1637d1d2bbb3af492199c79199d2394c36c09..07118c20b2ab38361a7511bb85d401e7b40c8d58 100644 --- a/Makefile +++ b/Makefile @@ -6441,6 +6441,7 @@ LIBGRPC_SRC = \ src/core/surface/server.c \ src/core/surface/server_chttp2.c \ src/core/surface/server_create.c \ + src/core/surface/validate_metadata.c \ src/core/surface/version.c \ src/core/transport/byte_stream.c \ src/core/transport/chttp2/alpn.c \ @@ -6724,6 +6725,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/surface/server.c \ src/core/surface/server_chttp2.c \ src/core/surface/server_create.c \ + src/core/surface/validate_metadata.c \ src/core/surface/version.c \ src/core/transport/byte_stream.c \ src/core/transport/chttp2/alpn.c \ diff --git a/binding.gyp b/binding.gyp index 75e2f3c8de5915a6f675cc75cf54e035ba06e051..d18d28e4804108404b17cc51840bded69ee2289b 100644 --- a/binding.gyp +++ b/binding.gyp @@ -269,6 +269,7 @@ 'src/core/surface/server.c', 'src/core/surface/server_chttp2.c', 'src/core/surface/server_create.c', + 'src/core/surface/validate_metadata.c', 'src/core/surface/version.c', 'src/core/transport/byte_stream.c', 'src/core/transport/chttp2/alpn.c', diff --git a/build.yaml b/build.yaml index d4caff2c4ec4f2e59edbc4c7a3b72fb3ce60f388..ebfd96b6e487895a4c5646d7045f06af267dae46 100644 --- a/build.yaml +++ b/build.yaml @@ -316,6 +316,7 @@ filegroups: - src/core/surface/server.c - src/core/surface/server_chttp2.c - src/core/surface/server_create.c + - src/core/surface/validate_metadata.c - src/core/surface/version.c - src/core/transport/byte_stream.c - src/core/transport/chttp2/alpn.c diff --git a/gRPC.podspec b/gRPC.podspec index da29387e83fce61b16681001e141f4c5111bdc30..1e2925d42a35964e27ed3ab80e4f397fa8e16d63 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -395,6 +395,7 @@ Pod::Spec.new do |s| 'src/core/surface/server.c', 'src/core/surface/server_chttp2.c', 'src/core/surface/server_create.c', + 'src/core/surface/validate_metadata.c', 'src/core/surface/version.c', 'src/core/transport/byte_stream.c', 'src/core/transport/chttp2/alpn.c', diff --git a/grpc.gemspec b/grpc.gemspec index ee5c22c24c9d22bc10a997755a950da8ce384d7d..bf4435a9d71f18377e8b750f5a39f1f19b3ff448 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -33,7 +33,7 @@ Gem::Specification.new do |s| s.platform = Gem::Platform::RUBY s.add_dependency 'google-protobuf', '~> 3.0.0alpha.1.1' - s.add_dependency 'googleauth', '~> 0.4' + s.add_dependency 'googleauth', '~> 0.5.1' s.add_development_dependency 'bundler', '~> 1.9' s.add_development_dependency 'logging', '~> 2.0' @@ -42,7 +42,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'rake-compiler', '~> 0.9' s.add_development_dependency 'rspec', '~> 3.2' s.add_development_dependency 'rubocop', '~> 0.30.0' - s.add_development_dependency 'signet', '~>0.6.0' + s.add_development_dependency 'signet', '~>0.7.0' s.extensions = %w(src/ruby/ext/grpc/extconf.rb) @@ -378,6 +378,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/surface/server.c ) s.files += %w( src/core/surface/server_chttp2.c ) s.files += %w( src/core/surface/server_create.c ) + s.files += %w( src/core/surface/validate_metadata.c ) s.files += %w( src/core/surface/version.c ) s.files += %w( src/core/transport/byte_stream.c ) s.files += %w( src/core/transport/chttp2/alpn.c ) diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index d52aab0dd3002823ab16d8ab4e389bf58a79a322..85a2ec4547eac33ad9b55a6515b098c600765534 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -715,6 +715,16 @@ void grpc_server_destroy(grpc_server *server); thread-safety issues raised by it should not be of concern. */ int grpc_tracer_set_enabled(const char *name, int enabled); +/** Check whether a metadata key is legal (will be accepted by core) */ +int grpc_header_key_is_legal(const char *key, size_t length); + +/** Check whether a non-binary metadata value is legal (will be accepted by + core) */ +int grpc_header_nonbin_value_is_legal(const char *value, size_t length); + +/** Check whether a metadata key corresponds to a binary value */ +int grpc_is_binary_header(const char *key, size_t length); + #ifdef __cplusplus } #endif diff --git a/package.json b/package.json index 739195de5da7e43482d56c44f3f8b3d2af91a343..8daf2fa788329e2e1d6cdce9b7559edc66ef4d7c 100644 --- a/package.json +++ b/package.json @@ -31,13 +31,13 @@ "protobufjs": "^4.0.0" }, "devDependencies": { - "async": "^0.9.0", + "async": "^1.5.0", "google-auth-library": "^0.9.2", "istanbul": "^0.3.21", "jsdoc": "^3.3.2", "jshint": "^2.5.0", "minimist": "^1.1.0", - "mocha": "~1.21.0", + "mocha": "^2.3.4", "mocha-jenkins-reporter": "^0.1.9", "mustache": "^2.0.0", "poisson-process": "^0.2.1" @@ -329,6 +329,7 @@ "src/core/surface/server.c", "src/core/surface/server_chttp2.c", "src/core/surface/server_create.c", + "src/core/surface/validate_metadata.c", "src/core/surface/version.c", "src/core/transport/byte_stream.c", "src/core/transport/chttp2/alpn.c", diff --git a/src/core/surface/call.c b/src/core/surface/call.c index a162d99193d1b0ffea6d3dcab8dd599b32898646..2782c1a4ddceed495531cbb47d94069d0953bfe6 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -37,6 +37,7 @@ #include <string.h> #include <grpc/compression.h> +#include <grpc/grpc.h> #include <grpc/support/alloc.h> #include <grpc/support/log.h> #include <grpc/support/string_util.h> @@ -562,12 +563,16 @@ static int prepare_application_metadata(grpc_call *call, int count, GPR_ASSERT(sizeof(grpc_linked_mdelem) == sizeof(md->internal_data)); l->md = grpc_mdelem_from_string_and_buffer( md->key, (const gpr_uint8 *)md->value, md->value_length); - if (!grpc_mdstr_is_legal_header(l->md->key)) { + if (!grpc_header_key_is_legal(grpc_mdstr_as_c_string(l->md->key), + GRPC_MDSTR_LENGTH(l->md->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_nonbin_header(l->md->value)) { + } else if (!grpc_is_binary_header(grpc_mdstr_as_c_string(l->md->key), + GRPC_MDSTR_LENGTH(l->md->key)) && + !grpc_header_nonbin_value_is_legal( + grpc_mdstr_as_c_string(l->md->value), + GRPC_MDSTR_LENGTH(l->md->value))) { gpr_log(GPR_ERROR, "attempt to send invalid metadata value"); return 0; } diff --git a/src/core/surface/validate_metadata.c b/src/core/surface/validate_metadata.c new file mode 100644 index 0000000000000000000000000000000000000000..7e88cc24d2d774404466b3caa670ea242a13f56d --- /dev/null +++ b/src/core/surface/validate_metadata.c @@ -0,0 +1,73 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include <stdlib.h> +#include <string.h> + +#include <grpc/support/port_platform.h> + +static int conforms_to(const char *s, size_t len, const gpr_uint8 *legal_bits) { + const char *p = s; + const char *e = s + len; + for (; p != e; p++) { + int idx = *p; + int byte = idx / 8; + int bit = idx % 8; + if ((legal_bits[byte] & (1 << bit)) == 0) return 0; + } + return 1; +} + +int grpc_header_key_is_legal(const char *key, size_t length) { + static const gpr_uint8 legal_header_bits[256 / 8] = { + 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}; + if (length == 0) { + return 0; + } + return conforms_to(key, length, legal_header_bits); +} + +int grpc_header_nonbin_value_is_legal(const char *value, size_t length) { + 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(value, length, legal_header_bits); +} + +int grpc_is_binary_header(const char *key, size_t length) { + if (length < 5) return 0; + return 0 == memcmp(key + length - 4, "-bin", 4); +} diff --git a/src/core/transport/chttp2/bin_encoder.c b/src/core/transport/chttp2/bin_encoder.c index 9c9070ede4f46a19a3b03dcaac8a06fdea516059..53ea9ac609e620b6a1351579bf64f034d2cb6a0b 100644 --- a/src/core/transport/chttp2/bin_encoder.c +++ b/src/core/transport/chttp2/bin_encoder.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -283,8 +283,3 @@ gpr_slice grpc_chttp2_base64_encode_and_huffman_compress(gpr_slice input) { GPR_ASSERT(in == GPR_SLICE_END_PTR(input)); return output; } - -int grpc_is_binary_header(const char *key, size_t length) { - if (length < 5) return 0; - return 0 == memcmp(key + length - 4, "-bin", 4); -} diff --git a/src/core/transport/chttp2/bin_encoder.h b/src/core/transport/chttp2/bin_encoder.h index d3e5a855ddb05cef8380065deda028de8860d28d..036fddf9981e103297cc01ae5a7485051a5c2225 100644 --- a/src/core/transport/chttp2/bin_encoder.h +++ b/src/core/transport/chttp2/bin_encoder.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -51,6 +51,4 @@ gpr_slice grpc_chttp2_huffman_compress(gpr_slice input); return y; */ gpr_slice grpc_chttp2_base64_encode_and_huffman_compress(gpr_slice input); -int grpc_is_binary_header(const char *key, size_t length); - #endif /* GRPC_INTERNAL_CORE_TRANSPORT_CHTTP2_BIN_ENCODER_H */ diff --git a/src/core/transport/chttp2/hpack_encoder.c b/src/core/transport/chttp2/hpack_encoder.c index 6c558bc1cbfb75929ec68a533789e186cfe5d005..303b8f332a77ddf14ac1951c7414bf56b7028bed 100644 --- a/src/core/transport/chttp2/hpack_encoder.c +++ b/src/core/transport/chttp2/hpack_encoder.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -36,6 +36,11 @@ #include <assert.h> #include <string.h> +/* This is here for grpc_is_binary_header + * TODO(murgatroid99): Remove this + */ +#include <grpc/grpc.h> + #include <grpc/support/alloc.h> #include <grpc/support/log.h> #include <grpc/support/useful.h> diff --git a/src/core/transport/chttp2/hpack_parser.c b/src/core/transport/chttp2/hpack_parser.c index fea000089600100efd0c830962c174836c925d4f..48790c2ef2927c76f3682f22da007ee85c833fda 100644 --- a/src/core/transport/chttp2/hpack_parser.c +++ b/src/core/transport/chttp2/hpack_parser.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -38,6 +38,11 @@ #include <string.h> #include <assert.h> +/* This is here for grpc_is_binary_header + * TODO(murgatroid99): Remove this + */ +#include <grpc/grpc.h> + #include <grpc/support/alloc.h> #include <grpc/support/log.h> #include <grpc/support/port_platform.h> diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c index df05d1a302fb65708dbbfa9b097455ccbeb4ecd3..e645ef9d8c9e692255ff997e9aafdbe91992c28d 100644 --- a/src/core/transport/metadata.c +++ b/src/core/transport/metadata.c @@ -688,37 +688,3 @@ gpr_slice grpc_mdstr_as_base64_encoded_and_huffman_compressed(grpc_mdstr *gs) { gpr_mu_unlock(&shard->mu); return slice; } - -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++) { - 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, 0x00, 0x00, 0x00, - 0x80, 0xfe, 0xff, 0xff, 0x07, 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_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), - GPR_SLICE_LENGTH(s->slice)); -} diff --git a/src/core/transport/metadata.h b/src/core/transport/metadata.h index 3d3efc682d14368346a2615fde2398f785211d4d..829c8a0873a8f6b0f2a124e209fc43c964e010d2 100644 --- a/src/core/transport/metadata.h +++ b/src/core/transport/metadata.h @@ -142,6 +142,8 @@ void grpc_mdelem_unref(grpc_mdelem *md); Does not promise that the returned string has no embedded nulls however. */ const char *grpc_mdstr_as_c_string(grpc_mdstr *s); +#define GRPC_MDSTR_LENGTH(s) (GPR_SLICE_LENGTH(s->slice)) + 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/node/ext/call.cc b/src/node/ext/call.cc index c0e2b0f0e8fac71f8550eca326eec74721a4d21b..84a3e227c89344187b31bccf572f2052a31f9075 100644 --- a/src/node/ext/call.cc +++ b/src/node/ext/call.cc @@ -95,10 +95,6 @@ Local<Value> nanErrorWithCode(const char *msg, grpc_call_error code) { return scope.Escape(err); } -bool EndsWith(const char *str, const char *substr) { - return strcmp(str+strlen(str)-strlen(substr), substr) == 0; -} - bool CreateMetadataArray(Local<Object> metadata, grpc_metadata_array *array, shared_ptr<Resources> resources) { HandleScope scope; @@ -126,7 +122,7 @@ bool CreateMetadataArray(Local<Object> metadata, grpc_metadata_array *array, grpc_metadata *current = &array->metadata[array->count]; current->key = **utf8_key; // Only allow binary headers for "-bin" keys - if (EndsWith(current->key, "-bin")) { + if (grpc_is_binary_header(current->key, strlen(current->key))) { if (::node::Buffer::HasInstance(value)) { current->value = ::node::Buffer::Data(value); current->value_length = ::node::Buffer::Length(value); @@ -180,7 +176,7 @@ Local<Value> ParseMetadata(const grpc_metadata_array *metadata_array) { } else { array = Local<Array>::Cast(maybe_array.ToLocalChecked()); } - if (EndsWith(elem->key, "-bin")) { + if (grpc_is_binary_header(elem->key, strlen(elem->key))) { Nan::Set(array, index_map[elem->key], MakeFastBuffer( Nan::CopyBuffer(elem->value, diff --git a/src/node/ext/node_grpc.cc b/src/node/ext/node_grpc.cc index 5b5f3c1c5b88321a79267ecf82450fa0ea34e852..a2b8e0d22bf12563cfe5c6428c1b54294a34b194 100644 --- a/src/node/ext/node_grpc.cc +++ b/src/node/ext/node_grpc.cc @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -44,6 +44,7 @@ #include "completion_queue_async_worker.h" #include "server_credentials.h" +using v8::FunctionTemplate; using v8::Local; using v8::Value; using v8::Object; @@ -230,6 +231,40 @@ void InitWriteFlags(Local<Object> exports) { Nan::Set(write_flags, Nan::New("NO_COMPRESS").ToLocalChecked(), NO_COMPRESS); } +NAN_METHOD(MetadataKeyIsLegal) { + if (!info[0]->IsString()) { + return Nan::ThrowTypeError( + "headerKeyIsLegal's argument must be a string"); + } + Local<String> key = Nan::To<String>(info[0]).ToLocalChecked(); + char *key_str = *Nan::Utf8String(key); + info.GetReturnValue().Set(static_cast<bool>( + grpc_header_key_is_legal(key_str, static_cast<size_t>(key->Length())))); +} + +NAN_METHOD(MetadataNonbinValueIsLegal) { + if (!info[0]->IsString()) { + return Nan::ThrowTypeError( + "metadataNonbinValueIsLegal's argument must be a string"); + } + Local<String> value = Nan::To<String>(info[0]).ToLocalChecked(); + char *value_str = *Nan::Utf8String(value); + info.GetReturnValue().Set(static_cast<bool>( + grpc_header_nonbin_value_is_legal( + value_str, static_cast<size_t>(value->Length())))); +} + +NAN_METHOD(MetadataKeyIsBinary) { + if (!info[0]->IsString()) { + return Nan::ThrowTypeError( + "metadataKeyIsLegal's argument must be a string"); + } + Local<String> key = Nan::To<String>(info[0]).ToLocalChecked(); + char *key_str = *Nan::Utf8String(key); + info.GetReturnValue().Set(static_cast<bool>( + grpc_is_binary_header(key_str, static_cast<size_t>(key->Length())))); +} + void init(Local<Object> exports) { Nan::HandleScope scope; grpc_init(); @@ -247,6 +282,19 @@ void init(Local<Object> exports) { grpc::node::Server::Init(exports); grpc::node::CompletionQueueAsyncWorker::Init(exports); grpc::node::ServerCredentials::Init(exports); + + // Attach a few utility functions directly to the module + Nan::Set(exports, Nan::New("metadataKeyIsLegal").ToLocalChecked(), + Nan::GetFunction( + Nan::New<FunctionTemplate>(MetadataKeyIsLegal)).ToLocalChecked()); + Nan::Set(exports, Nan::New("metadataNonbinValueIsLegal").ToLocalChecked(), + Nan::GetFunction( + Nan::New<FunctionTemplate>(MetadataNonbinValueIsLegal) + ).ToLocalChecked()); + Nan::Set(exports, Nan::New("metadataKeyIsBinary").ToLocalChecked(), + Nan::GetFunction( + Nan::New<FunctionTemplate>(MetadataKeyIsBinary) + ).ToLocalChecked()); } NODE_MODULE(grpc_node, init) diff --git a/src/node/interop/async_delay_queue.js b/src/node/interop/async_delay_queue.js index 2bd3ca4da3f41e24eb7fc1ccff2cfaffb626834d..5df1e00921d8436407c9fe94743a1266178321d0 100644 --- a/src/node/interop/async_delay_queue.js +++ b/src/node/interop/async_delay_queue.js @@ -36,8 +36,8 @@ var _ = require('lodash'); /** - * This class represents a queue of callbacks that must happen sequentially, each - * with a specific delay after the previous event. + * This class represents a queue of callbacks that must happen sequentially, + * each with a specific delay after the previous event. */ function AsyncDelayQueue() { this.queue = []; diff --git a/src/node/src/metadata.js b/src/node/src/metadata.js index 0a2f1489b6ead9db26642f983d69a2dec7083f6c..fef79f959e49323e6e5fad04f106324f21678c37 100644 --- a/src/node/src/metadata.js +++ b/src/node/src/metadata.js @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -49,6 +49,8 @@ var _ = require('lodash'); +var grpc = require('bindings')('grpc_node'); + /** * Class for storing metadata. Keys are normalized to lowercase ASCII. * @constructor @@ -58,15 +60,16 @@ function Metadata() { } function normalizeKey(key) { - if (!(/^[A-Za-z\d_-]+$/.test(key))) { - throw new Error('Metadata keys must be nonempty strings containing only ' + - 'alphanumeric characters and hyphens'); + key = key.toLowerCase(); + if (grpc.metadataKeyIsLegal(key)) { + return key; + } else { + throw new Error('Metadata key contains illegal characters'); } - return key.toLowerCase(); } function validate(key, value) { - if (_.endsWith(key, '-bin')) { + if (grpc.metadataKeyIsBinary(key)) { if (!(value instanceof Buffer)) { throw new Error('keys that end with \'-bin\' must have Buffer values'); } @@ -75,9 +78,8 @@ function validate(key, value) { throw new Error( 'keys that don\'t end with \'-bin\' must have String values'); } - if (!(/^[\x20-\x7E]*$/.test(value))) { - throw new Error('Metadata string values can only contain printable ' + - 'ASCII characters and space'); + if (!grpc.metadataNonbinValueIsLegal(value)) { + throw new Error('Metadata string value contains illegal characters'); } } } diff --git a/src/ruby/ext/grpc/rb_call.c b/src/ruby/ext/grpc/rb_call.c index 1647d9b484f8f0d5a1b85a6a954f8773e9c1c63a..43adafb73f9a9a0d7b81e2a99fc73c726c4424f7 100644 --- a/src/ruby/ext/grpc/rb_call.c +++ b/src/ruby/ext/grpc/rb_call.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -310,33 +310,61 @@ static int grpc_rb_md_ary_fill_hash_cb(VALUE key, VALUE val, VALUE md_ary_obj) { grpc_metadata_array *md_ary = NULL; long array_length; long i; + char *key_str; + size_t key_len; + char *value_str; + size_t value_len; + + if (TYPE(key) == T_SYMBOL) { + key_str = (char *)rb_id2name(SYM2ID(key)); + key_len = strlen(key_str); + } else { /* StringValueCStr does all other type exclusions for us */ + key_str = StringValueCStr(key); + key_len = RSTRING_LEN(key); + } + + if (!grpc_header_key_is_legal(key_str, key_len)) { + rb_raise(rb_eArgError, + "'%s' is an invalid header key, must match [a-z0-9-_.]+", + key_str); + return ST_STOP; + } /* Construct a metadata object from key and value and add it */ TypedData_Get_Struct(md_ary_obj, grpc_metadata_array, &grpc_rb_md_ary_data_type, md_ary); if (TYPE(val) == T_ARRAY) { - /* If the value is an array, add capacity for each value in the array */ array_length = RARRAY_LEN(val); + /* If the value is an array, add capacity for each value in the array */ for (i = 0; i < array_length; i++) { - if (TYPE(key) == T_SYMBOL) { - md_ary->metadata[md_ary->count].key = (char *)rb_id2name(SYM2ID(key)); - } else { /* StringValueCStr does all other type exclusions for us */ - md_ary->metadata[md_ary->count].key = StringValueCStr(key); + value_str = RSTRING_PTR(rb_ary_entry(val, i)); + value_len = RSTRING_LEN(rb_ary_entry(val, i)); + if (!grpc_is_binary_header(key_str, key_len) && + !grpc_header_nonbin_value_is_legal(value_str, value_len)) { + // The value has invalid characters + rb_raise(rb_eArgError, + "Header value '%s' has invalid characters", value_str); + return ST_STOP; } - md_ary->metadata[md_ary->count].value = RSTRING_PTR(rb_ary_entry(val, i)); - md_ary->metadata[md_ary->count].value_length = - RSTRING_LEN(rb_ary_entry(val, i)); + md_ary->metadata[md_ary->count].key = key_str; + md_ary->metadata[md_ary->count].value = value_str; + md_ary->metadata[md_ary->count].value_length = value_len; md_ary->count += 1; } } else { - if (TYPE(key) == T_SYMBOL) { - md_ary->metadata[md_ary->count].key = (char *)rb_id2name(SYM2ID(key)); - } else { /* StringValueCStr does all other type exclusions for us */ - md_ary->metadata[md_ary->count].key = StringValueCStr(key); + value_str = RSTRING_PTR(val); + value_len = RSTRING_LEN(val); + if (!grpc_is_binary_header(key_str, key_len) && + !grpc_header_nonbin_value_is_legal(value_str, value_len)) { + // The value has invalid characters + rb_raise(rb_eArgError, + "Header value '%s' has invalid characters", value_str); + return ST_STOP; } - md_ary->metadata[md_ary->count].value = RSTRING_PTR(val); - md_ary->metadata[md_ary->count].value_length = RSTRING_LEN(val); + md_ary->metadata[md_ary->count].key = key_str; + md_ary->metadata[md_ary->count].value = value_str; + md_ary->metadata[md_ary->count].value_length = value_len; md_ary->count += 1; } diff --git a/src/ruby/pb/test/client.rb b/src/ruby/pb/test/client.rb index 4e77492ba9833d46c2894a59998077f05aa0d978..684ee807715a17a6280841e95ea3777d542e31eb 100755 --- a/src/ruby/pb/test/client.rb +++ b/src/ruby/pb/test/client.rb @@ -56,8 +56,6 @@ require 'test/proto/empty' require 'test/proto/messages' require 'test/proto/test_services' -require 'signet/ssl_config' - AUTH_ENV = Google::Auth::CredentialsLoader::ENV_VAR # RubyLogger defines a logger for gRPC based on the standard ruby logger. @@ -268,11 +266,6 @@ class NamedTests auth_creds = Google::Auth.get_application_default(@args.oauth_scope) update_metadata = proc do |md| kw = auth_creds.updater_proc.call({}) - - # TODO(mlumish): downcase the metadata keys here to make sure - # they are not rejected by C core. This is a hotfix that should - # be addressed by introducing auto-downcasing logic. - Hash[ kw.each_pair.map { |k, v| [k.downcase, v] }] end call_creds = GRPC::Core::CallCredentials.new(update_metadata) diff --git a/src/ruby/spec/pb/health/checker_spec.rb b/src/ruby/spec/pb/health/checker_spec.rb index 794c5922fa061fb43605d5909840691c80aa7308..10d3a0705ac18438f954f405fd1aff02d93efb7a 100644 --- a/src/ruby/spec/pb/health/checker_spec.rb +++ b/src/ruby/spec/pb/health/checker_spec.rb @@ -47,13 +47,12 @@ describe 'Health protobuf code generation' do end it 'should have the same content as created by code generation' do - root_dir = File.dirname( - File.dirname(File.dirname(File.dirname(__FILE__)))) - pb_dir = File.join(root_dir, 'pb') + root_dir = File.join(File.dirname(__FILE__), '..', '..', '..', '..') + pb_dir = File.join(root_dir, 'proto') # Get the current content - service_path = File.join(pb_dir, 'grpc', 'health', 'v1alpha', - 'health_services.rb') + service_path = File.join(root_dir, 'ruby', 'pb', 'grpc', + 'health', 'v1alpha', 'health_services.rb') want = nil File.open(service_path) { |f| want = f.read } diff --git a/templates/grpc.gemspec.template b/templates/grpc.gemspec.template index 5c220052e774cb2315c413336243630eff4ea6bc..fdf87ee13fd14090fd1c1b56b4f251285e36465a 100644 --- a/templates/grpc.gemspec.template +++ b/templates/grpc.gemspec.template @@ -35,7 +35,7 @@ s.platform = Gem::Platform::RUBY s.add_dependency 'google-protobuf', '~> 3.0.0alpha.1.1' - s.add_dependency 'googleauth', '~> 0.4' + s.add_dependency 'googleauth', '~> 0.5.1' s.add_development_dependency 'bundler', '~> 1.9' s.add_development_dependency 'logging', '~> 2.0' @@ -44,7 +44,7 @@ s.add_development_dependency 'rake-compiler', '~> 0.9' s.add_development_dependency 'rspec', '~> 3.2' s.add_development_dependency 'rubocop', '~> 0.30.0' - s.add_development_dependency 'signet', '~>0.6.0' + s.add_development_dependency 'signet', '~>0.7.0' s.extensions = %w(src/ruby/ext/grpc/extconf.rb) diff --git a/templates/package.json.template b/templates/package.json.template index 0525a525e7a3a3bcc0ea6167b99a155f2454d1f7..d50b1e2eebdbcce51469fc53549ede7ea8e51288 100644 --- a/templates/package.json.template +++ b/templates/package.json.template @@ -33,13 +33,13 @@ "protobufjs": "^4.0.0" }, "devDependencies": { - "async": "^0.9.0", + "async": "^1.5.0", "google-auth-library": "^0.9.2", "istanbul": "^0.3.21", "jsdoc": "^3.3.2", "jshint": "^2.5.0", "minimist": "^1.1.0", - "mocha": "~1.21.0", + "mocha": "^2.3.4", "mocha-jenkins-reporter": "^0.1.9", "mustache": "^2.0.0", "poisson-process": "^0.2.1" diff --git a/test/core/transport/chttp2/bin_encoder_test.c b/test/core/transport/chttp2/bin_encoder_test.c index 1ffd8ed3cbf0cb9ab161597f728548c0fa1852f1..d1838075be01fe9f2dcdf4ef957a815b3817d8dd 100644 --- a/test/core/transport/chttp2/bin_encoder_test.c +++ b/test/core/transport/chttp2/bin_encoder_test.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -35,6 +35,10 @@ #include <string.h> +/* This is here for grpc_is_binary_header + * TODO(murgatroid99): Remove this + */ +#include <grpc/grpc.h> #include "src/core/support/string.h" #include <grpc/support/alloc.h> #include <grpc/support/log.h> diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index f84a35ce65a65940af0bd796e23208d183b43923..8581bf39c86398751d9631dbcbeba30508f48aef 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1013,6 +1013,7 @@ src/core/surface/metadata_array.c \ src/core/surface/server.c \ src/core/surface/server_chttp2.c \ src/core/surface/server_create.c \ +src/core/surface/validate_metadata.c \ src/core/surface/version.c \ src/core/transport/byte_stream.c \ src/core/transport/chttp2/alpn.c \ diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index e69e9877c5c7f272fd1f4e9fd689cfe92b32506f..f8798e1c4d2764207f2910834486c6f565c0d1b9 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -555,7 +555,7 @@ def aggregate_http2_results(stdout): match = re.search(r'\{"cases[^\]]*\]\}', stdout) if not match: return None - + results = json.loads(match.group(0)) skipped = 0 passed = 0 @@ -748,7 +748,7 @@ try: for test_case in _HTTP2_TEST_CASES: if server_name == "go": # TODO(carl-mastrangelo): Reenable after https://github.com/grpc/grpc-go/issues/434 - continue + continue test_job = cloud_to_cloud_jobspec(http2Interop, test_case, server_name, @@ -777,7 +777,7 @@ try: job[0].http2results = aggregate_http2_results(job[0].message) report_utils.render_interop_html_report( - set([str(l) for l in languages]), servers, _TEST_CASES, _AUTH_TEST_CASES, + set([str(l) for l in languages]), servers, _TEST_CASES, _AUTH_TEST_CASES, _HTTP2_TEST_CASES, resultset, num_failures, args.cloud_to_prod_auth or args.cloud_to_prod, args.http2_interop) diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 2ea8715c803b8e5facd9a8136879577c7a403246..63ead1f1b17adf9f62c8d223a04ad5bb93a226ff 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -17842,6 +17842,7 @@ "src/core/surface/server_chttp2.c", "src/core/surface/server_create.c", "src/core/surface/surface_trace.h", + "src/core/surface/validate_metadata.c", "src/core/surface/version.c", "src/core/transport/byte_stream.c", "src/core/transport/byte_stream.h", @@ -18312,6 +18313,7 @@ "src/core/surface/server_chttp2.c", "src/core/surface/server_create.c", "src/core/surface/surface_trace.h", + "src/core/surface/validate_metadata.c", "src/core/surface/version.c", "src/core/transport/byte_stream.c", "src/core/transport/byte_stream.h", diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index 164d47c217f6e647c8c3d6dcfed7969b31e1e593..0d25f440133d80fbda3c6d832ada4e7a0e975aee 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -621,6 +621,8 @@ </ClCompile> <ClCompile Include="..\..\..\src\core\surface\server_create.c"> </ClCompile> + <ClCompile Include="..\..\..\src\core\surface\validate_metadata.c"> + </ClCompile> <ClCompile Include="..\..\..\src\core\surface\version.c"> </ClCompile> <ClCompile Include="..\..\..\src\core\transport\byte_stream.c"> diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 200319c55790dfe07859ff830ca5d3fe8f88b0e1..98958e4d0325c21659c7438a81ec4f58bc93059e 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -352,6 +352,9 @@ <ClCompile Include="..\..\..\src\core\surface\server_create.c"> <Filter>src\core\surface</Filter> </ClCompile> + <ClCompile Include="..\..\..\src\core\surface\validate_metadata.c"> + <Filter>src\core\surface</Filter> + </ClCompile> <ClCompile Include="..\..\..\src\core\surface\version.c"> <Filter>src\core\surface</Filter> </ClCompile> diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index 4e8f238ad974afb961565c6db59fef98734b7470..22d7f1b083c5db11bc383ae1bb7b0190fa5184d7 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -560,6 +560,8 @@ </ClCompile> <ClCompile Include="..\..\..\src\core\surface\server_create.c"> </ClCompile> + <ClCompile Include="..\..\..\src\core\surface\validate_metadata.c"> + </ClCompile> <ClCompile Include="..\..\..\src\core\surface\version.c"> </ClCompile> <ClCompile Include="..\..\..\src\core\transport\byte_stream.c"> diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index 460f6d431dc6e2e71d2870519026332dee20dee1..d537789bbfeab5e79ed2735160a4a61dd1bbc174 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -292,6 +292,9 @@ <ClCompile Include="..\..\..\src\core\surface\server_create.c"> <Filter>src\core\surface</Filter> </ClCompile> + <ClCompile Include="..\..\..\src\core\surface\validate_metadata.c"> + <Filter>src\core\surface</Filter> + </ClCompile> <ClCompile Include="..\..\..\src\core\surface\version.c"> <Filter>src\core\surface</Filter> </ClCompile>