diff options
-rw-r--r-- | include/grpc/impl/codegen/grpc_types.h | 3 | ||||
-rw-r--r-- | src/core/ext/filters/client_channel/parse_address.cc | 9 | ||||
-rw-r--r-- | src/core/ext/filters/client_channel/subchannel_index.cc | 7 | ||||
-rw-r--r-- | src/core/ext/filters/client_channel/subchannel_index.h | 5 | ||||
-rw-r--r-- | src/core/ext/filters/http/server/http_server_filter.cc | 17 | ||||
-rw-r--r-- | test/core/client_channel/parse_address_test.cc | 14 | ||||
-rw-r--r-- | test/cpp/util/cli_credentials.cc | 69 | ||||
-rw-r--r-- | test/cpp/util/cli_credentials.h | 3 |
8 files changed, 109 insertions, 18 deletions
diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index b5353c1dea..5f3b96f40b 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -342,6 +342,9 @@ typedef struct { "grpc.disable_client_authority_filter" /** If set to zero, disables use of http proxies. Enabled by default. */ #define GRPC_ARG_ENABLE_HTTP_PROXY "grpc.enable_http_proxy" +/** If set to non zero, surfaces the user agent string to the server. User + agent is surfaced by default. */ +#define GRPC_ARG_SURFACE_USER_AGENT "grpc.surface_user_agent" /** \} */ /** Result of a grpc call. If the caller satisfies the prerequisites of a diff --git a/src/core/ext/filters/client_channel/parse_address.cc b/src/core/ext/filters/client_channel/parse_address.cc index b3900114ad..7e726dd562 100644 --- a/src/core/ext/filters/client_channel/parse_address.cc +++ b/src/core/ext/filters/client_channel/parse_address.cc @@ -125,9 +125,16 @@ bool grpc_parse_ipv6_hostport(const char* hostport, grpc_resolved_address* addr, char* host_end = static_cast<char*>(gpr_memrchr(host, '%', strlen(host))); if (host_end != nullptr) { GPR_ASSERT(host_end >= host); - char host_without_scope[GRPC_INET6_ADDRSTRLEN]; + char host_without_scope[GRPC_INET6_ADDRSTRLEN + 1]; size_t host_without_scope_len = static_cast<size_t>(host_end - host); uint32_t sin6_scope_id = 0; + if (host_without_scope_len > GRPC_INET6_ADDRSTRLEN) { + gpr_log(GPR_ERROR, + "invalid ipv6 address length %zu. Length cannot be greater than " + "GRPC_INET6_ADDRSTRLEN i.e %d)", + host_without_scope_len, GRPC_INET6_ADDRSTRLEN); + goto done; + } strncpy(host_without_scope, host, host_without_scope_len); host_without_scope[host_without_scope_len] = '\0'; if (grpc_inet_pton(GRPC_AF_INET6, host_without_scope, &in6->sin6_addr) == diff --git a/src/core/ext/filters/client_channel/subchannel_index.cc b/src/core/ext/filters/client_channel/subchannel_index.cc index cb02b1a748..f2b6c24e8e 100644 --- a/src/core/ext/filters/client_channel/subchannel_index.cc +++ b/src/core/ext/filters/client_channel/subchannel_index.cc @@ -42,7 +42,7 @@ struct grpc_subchannel_key { grpc_subchannel_args args; }; -static bool g_force_creation = false; +static gpr_atm g_force_creation = false; static grpc_subchannel_key* create_key( const grpc_subchannel_args* args, @@ -73,7 +73,8 @@ static grpc_subchannel_key* subchannel_key_copy(grpc_subchannel_key* k) { int grpc_subchannel_key_compare(const grpc_subchannel_key* a, const grpc_subchannel_key* b) { - if (g_force_creation) return false; + // To pretend the keys are different, return a non-zero value. + if (GPR_UNLIKELY(gpr_atm_no_barrier_load(&g_force_creation))) return 1; int c = GPR_ICMP(a->args.filter_count, b->args.filter_count); if (c != 0) return c; if (a->args.filter_count > 0) { @@ -250,5 +251,5 @@ void grpc_subchannel_index_unregister(grpc_subchannel_key* key, } void grpc_subchannel_index_test_only_set_force_creation(bool force_creation) { - g_force_creation = force_creation; + gpr_atm_no_barrier_store(&g_force_creation, force_creation); } diff --git a/src/core/ext/filters/client_channel/subchannel_index.h b/src/core/ext/filters/client_channel/subchannel_index.h index a7dae9d47d..c135613d26 100644 --- a/src/core/ext/filters/client_channel/subchannel_index.h +++ b/src/core/ext/filters/client_channel/subchannel_index.h @@ -65,13 +65,10 @@ void grpc_subchannel_index_ref(void); void grpc_subchannel_index_unref(void); /** \em TEST ONLY. - * If \a force_creation is true, all key comparisons will be false, resulting in + * If \a force_creation is true, all keys are regarded different, resulting in * new subchannels always being created. Otherwise, the keys will be compared as * usual. * - * This function is *not* threadsafe on purpose: it should *only* be used in - * test code. - * * Tests using this function \em MUST run tests with and without \a * force_creation set. */ void grpc_subchannel_index_test_only_set_force_creation(bool force_creation); diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index 926afeec84..1b3426b120 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -23,6 +23,7 @@ #include <grpc/support/alloc.h> #include <grpc/support/log.h> #include <string.h> +#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gprpp/manual_constructor.h" #include "src/core/lib/profiling/timers.h" #include "src/core/lib/slice/b64.h" @@ -66,6 +67,10 @@ struct call_data { grpc_closure* original_recv_trailing_metadata_ready; }; +struct channel_data { + bool surface_user_agent; +}; + } // namespace static grpc_error* hs_filter_outgoing_metadata(grpc_call_element* elem, @@ -262,6 +267,11 @@ static grpc_error* hs_filter_incoming_metadata(grpc_call_element* elem, GRPC_ERROR_STR_KEY, grpc_slice_from_static_string(":authority"))); } + channel_data* chand = static_cast<channel_data*>(elem->channel_data); + if (!chand->surface_user_agent && b->idx.named.user_agent != nullptr) { + grpc_metadata_batch_remove(b, b->idx.named.user_agent); + } + return error; } @@ -430,7 +440,12 @@ static void hs_destroy_call_elem(grpc_call_element* elem, /* Constructor for channel_data */ static grpc_error* hs_init_channel_elem(grpc_channel_element* elem, grpc_channel_element_args* args) { + channel_data* chand = static_cast<channel_data*>(elem->channel_data); GPR_ASSERT(!args->is_last); + chand->surface_user_agent = grpc_channel_arg_get_bool( + grpc_channel_args_find(args->channel_args, + const_cast<char*>(GRPC_ARG_SURFACE_USER_AGENT)), + true); return GRPC_ERROR_NONE; } @@ -444,7 +459,7 @@ const grpc_channel_filter grpc_http_server_filter = { hs_init_call_elem, grpc_call_stack_ignore_set_pollset_or_pollset_set, hs_destroy_call_elem, - 0, + sizeof(channel_data), hs_init_channel_elem, hs_destroy_channel_elem, grpc_channel_next_get_info, diff --git a/test/core/client_channel/parse_address_test.cc b/test/core/client_channel/parse_address_test.cc index ae157fbb8b..004549fa62 100644 --- a/test/core/client_channel/parse_address_test.cc +++ b/test/core/client_channel/parse_address_test.cc @@ -91,6 +91,15 @@ static void test_grpc_parse_ipv6(const char* uri_text, const char* host, grpc_uri_destroy(uri); } +/* Test parsing invalid ipv6 addresses (valid uri_text but invalid ipv6 addr) */ +static void test_grpc_parse_ipv6_invalid(const char* uri_text) { + grpc_core::ExecCtx exec_ctx; + grpc_uri* uri = grpc_uri_parse(uri_text, 0); + grpc_resolved_address addr; + GPR_ASSERT(!grpc_parse_ipv6(uri, &addr)); + grpc_uri_destroy(uri); +} + int main(int argc, char** argv) { grpc_test_init(argc, argv); grpc_init(); @@ -100,5 +109,10 @@ int main(int argc, char** argv) { test_grpc_parse_ipv6("ipv6:[2001:db8::1]:12345", "2001:db8::1", 12345, 0); test_grpc_parse_ipv6("ipv6:[2001:db8::1%252]:12345", "2001:db8::1", 12345, 2); + /* Address length greater than GRPC_INET6_ADDRSTRLEN */ + test_grpc_parse_ipv6_invalid( + "ipv6:WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW45%" + "v6:45%x$1*"); + grpc_shutdown(); } diff --git a/test/cpp/util/cli_credentials.cc b/test/cpp/util/cli_credentials.cc index acf4ef8ef1..0a922617bb 100644 --- a/test/cpp/util/cli_credentials.cc +++ b/test/cpp/util/cli_credentials.cc @@ -28,7 +28,8 @@ DEFINE_bool(use_auth, false, "--channel_creds_type=gdc."); DEFINE_string( access_token, "", - "The access token that will be sent to the server to authenticate RPCs."); + "The access token that will be sent to the server to authenticate RPCs. " + "Deprecated. Use --call_creds=access_token=<token>."); DEFINE_string( ssl_target, "", "If not empty, treat the server host name as this for ssl/tls certificate " @@ -37,10 +38,34 @@ DEFINE_string( channel_creds_type, "", "The channel creds type: insecure, ssl, gdc (Google Default Credentials) " "or alts."); +DEFINE_string( + call_creds, "", + "Call credentials to use: none (default), or access_token=<token>. If " + "provided, the call creds are composited on top of channel creds."); namespace grpc { namespace testing { +namespace { + +const char ACCESS_TOKEN_PREFIX[] = "access_token="; +constexpr int ACCESS_TOKEN_PREFIX_LEN = + sizeof(ACCESS_TOKEN_PREFIX) / sizeof(*ACCESS_TOKEN_PREFIX) - 1; + +bool IsAccessToken(const grpc::string& auth) { + return auth.length() > ACCESS_TOKEN_PREFIX_LEN && + auth.compare(0, ACCESS_TOKEN_PREFIX_LEN, ACCESS_TOKEN_PREFIX) == 0; +} + +grpc::string AccessToken(const grpc::string& auth) { + if (!IsAccessToken(auth)) { + return ""; + } + return grpc::string(auth, ACCESS_TOKEN_PREFIX_LEN); +} + +} // namespace + grpc::string CliCredentials::GetDefaultChannelCredsType() const { // Compatibility logic for --enable_ssl. if (FLAGS_enable_ssl) { @@ -59,6 +84,16 @@ grpc::string CliCredentials::GetDefaultChannelCredsType() const { return "insecure"; } +grpc::string CliCredentials::GetDefaultCallCreds() const { + if (!FLAGS_access_token.empty()) { + fprintf(stderr, + "warning: --access_token is deprecated. Use " + "--call_creds=access_token=<token>.\n"); + return grpc::string("access_token=") + FLAGS_access_token; + } + return "none"; +} + std::shared_ptr<grpc::ChannelCredentials> CliCredentials::GetChannelCredentials() const { if (FLAGS_channel_creds_type.compare("insecure") == 0) { @@ -80,18 +115,30 @@ CliCredentials::GetChannelCredentials() const { std::shared_ptr<grpc::CallCredentials> CliCredentials::GetCallCredentials() const { - if (!FLAGS_access_token.empty()) { - if (FLAGS_use_auth) { - fprintf(stderr, - "warning: use_auth is ignored when access_token is provided."); - } - return grpc::AccessTokenCredentials(FLAGS_access_token); + if (IsAccessToken(FLAGS_call_creds)) { + return grpc::AccessTokenCredentials(AccessToken(FLAGS_call_creds)); } + if (FLAGS_call_creds.compare("none") != 0) { + // Nothing to do; creds, if any, are baked into the channel. + return std::shared_ptr<grpc::CallCredentials>(); + } + fprintf(stderr, + "--call_creds=%s invalid; must be none " + "or access_token=<token>.\n", + FLAGS_call_creds.c_str()); return std::shared_ptr<grpc::CallCredentials>(); } std::shared_ptr<grpc::ChannelCredentials> CliCredentials::GetCredentials() const { + if (FLAGS_call_creds.empty()) { + FLAGS_call_creds = GetDefaultCallCreds(); + } else if (!FLAGS_access_token.empty() && !IsAccessToken(FLAGS_call_creds)) { + fprintf(stderr, + "warning: ignoring --access_token because --call_creds " + "already set to %s.\n", + FLAGS_call_creds.c_str()); + } if (FLAGS_channel_creds_type.empty()) { FLAGS_channel_creds_type = GetDefaultChannelCredsType(); } else if (FLAGS_enable_ssl && FLAGS_channel_creds_type.compare("ssl") != 0) { @@ -106,7 +153,7 @@ std::shared_ptr<grpc::ChannelCredentials> CliCredentials::GetCredentials() FLAGS_channel_creds_type.c_str()); } // Legacy transport upgrade logic for insecure requests. - if (!FLAGS_access_token.empty() && + if (IsAccessToken(FLAGS_call_creds) && FLAGS_channel_creds_type.compare("insecure") == 0) { fprintf(stderr, "warning: --channel_creds_type=insecure upgraded to ssl because " @@ -126,10 +173,14 @@ const grpc::string CliCredentials::GetCredentialUsage() const { return " --enable_ssl ; Set whether to use ssl (deprecated)\n" " --use_auth ; Set whether to create default google" " credentials\n" + " ; (deprecated)\n" " --access_token ; Set the access token in metadata," " overrides --use_auth\n" + " ; (deprecated)\n" " --ssl_target ; Set server host for ssl validation\n" - " --channel_creds_type ; Set to insecure, ssl, gdc, or alts\n"; + " --channel_creds_type ; Set to insecure, ssl, gdc, or alts\n" + " --call_creds ; Set to none, or" + " access_token=<token>\n"; } const grpc::string CliCredentials::GetSslTargetNameOverride() const { diff --git a/test/cpp/util/cli_credentials.h b/test/cpp/util/cli_credentials.h index 4636d3ca14..472c7abef2 100644 --- a/test/cpp/util/cli_credentials.h +++ b/test/cpp/util/cli_credentials.h @@ -36,6 +36,9 @@ class CliCredentials { // Returns the appropriate channel_creds_type value for the set of legacy // flag arguments. virtual grpc::string GetDefaultChannelCredsType() const; + // Returns the appropriate call_creds value for the set of legacy flag + // arguments. + virtual grpc::string GetDefaultCallCreds() const; // Returns the base transport channel credentials. Child classes can override // to support additional channel_creds_types unknown to this base class. virtual std::shared_ptr<grpc::ChannelCredentials> GetChannelCredentials() |