diff --git a/BUILD b/BUILD index d2fce9d6fea..89e4fc0f58b 100644 --- a/BUILD +++ b/BUILD @@ -1010,8 +1010,10 @@ grpc_cc_library( "madler_zlib", "absl/container:inlined_vector", "absl/status", + "absl/status:statusor", "absl/strings", "absl/types:optional", + "absl/container:flat_hash_map", ], language = "c++", public_hdrs = GRPC_PUBLIC_HDRS, diff --git a/BUILD.gn b/BUILD.gn index 5c9947b645c..b43ac2edee9 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1221,10 +1221,12 @@ config("grpc_config") { ":upb", ":absl/types:optional", ":absl/strings:strings", + ":absl/status:statusor", ":absl/status:status", ":absl/functional:bind_front", ":absl/container:inlined_vector", ":absl/container:flat_hash_set", + ":absl/container:flat_hash_map", "//third_party/cares", ":address_sorting", ] diff --git a/CMakeLists.txt b/CMakeLists.txt index 72e072c179b..cc94cf0b3ed 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -122,6 +122,7 @@ set(gRPC_ABSL_USED_TARGETS absl_errno_saver absl_exponential_biased absl_fixed_array + absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_graphcycles_internal @@ -140,12 +141,14 @@ set(gRPC_ABSL_USED_TARGETS absl_malloc_internal absl_memory absl_optional + absl_raw_hash_map absl_raw_hash_set absl_raw_logging_internal absl_span absl_spinlock_wait absl_stacktrace absl_status + absl_statusor absl_str_format absl_str_format_internal absl_strings @@ -2009,10 +2012,12 @@ target_link_libraries(grpc upb absl::optional absl::strings + absl::statusor absl::status absl::bind_front absl::inlined_vector absl::flat_hash_set + absl::flat_hash_map ) if(_gRPC_PLATFORM_IOS OR _gRPC_PLATFORM_MAC) target_link_libraries(grpc "-framework CoreFoundation") @@ -2562,8 +2567,10 @@ target_link_libraries(grpc_unsecure upb absl::optional absl::strings + absl::statusor absl::status absl::inlined_vector + absl::flat_hash_map ) if(_gRPC_PLATFORM_IOS OR _gRPC_PLATFORM_MAC) target_link_libraries(grpc_unsecure "-framework CoreFoundation") @@ -16286,7 +16293,7 @@ generate_pkgconfig( "high performance general RPC framework" "${gRPC_CORE_VERSION}" "gpr openssl" - "-lgrpc -laddress_sorting -lre2 -lupb -lcares -lz -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_bad_variant_access -labsl_city -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity" + "-lgrpc -laddress_sorting -lre2 -lupb -lcares -lz -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_city -labsl_statusor -labsl_bad_variant_access -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity" "" "grpc.pc") @@ -16296,7 +16303,7 @@ generate_pkgconfig( "high performance general RPC framework without SSL" "${gRPC_CORE_VERSION}" "gpr" - "-lgrpc_unsecure -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity" + "-lgrpc_unsecure -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_city -labsl_statusor -labsl_bad_variant_access -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity" "" "grpc_unsecure.pc") @@ -16306,7 +16313,7 @@ generate_pkgconfig( "C++ wrapper for gRPC" "${gRPC_CPP_VERSION}" "grpc" - "-lgrpc++ -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_bad_variant_access -labsl_city -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity" + "-lgrpc++ -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_city -labsl_statusor -labsl_bad_variant_access -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity" "" "grpc++.pc") @@ -16316,6 +16323,6 @@ generate_pkgconfig( "C++ wrapper for gRPC without SSL" "${gRPC_CPP_VERSION}" "grpc_unsecure" - "-lgrpc++_unsecure -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity" + "-lgrpc++_unsecure -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_city -labsl_statusor -labsl_bad_variant_access -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity" "" "grpc++_unsecure.pc") diff --git a/Makefile b/Makefile index eed15f1c9d4..67dcf0a2794 100644 --- a/Makefile +++ b/Makefile @@ -4343,6 +4343,7 @@ LIBGRPC_ABSEIL_SRC = \ third_party/abseil-cpp/absl/numeric/int128.cc \ third_party/abseil-cpp/absl/status/status.cc \ third_party/abseil-cpp/absl/status/status_payload_printer.cc \ + third_party/abseil-cpp/absl/status/statusor.cc \ third_party/abseil-cpp/absl/strings/ascii.cc \ third_party/abseil-cpp/absl/strings/charconv.cc \ third_party/abseil-cpp/absl/strings/cord.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 16ae58c7c46..c4d06626e64 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -1395,10 +1395,12 @@ libs: - upb - absl/types:optional - absl/strings:strings + - absl/status:statusor - absl/status:status - absl/functional:bind_front - absl/container:inlined_vector - absl/container:flat_hash_set + - absl/container:flat_hash_map baselib: true deps_linkage: static dll: true @@ -2079,8 +2081,10 @@ libs: - upb - absl/types:optional - absl/strings:strings + - absl/status:statusor - absl/status:status - absl/container:inlined_vector + - absl/container:flat_hash_map baselib: true deps_linkage: static dll: true diff --git a/config.m4 b/config.m4 index a397d3ac8c3..564013dc9c1 100644 --- a/config.m4 +++ b/config.m4 @@ -644,6 +644,7 @@ if test "$PHP_GRPC" != "no"; then third_party/abseil-cpp/absl/numeric/int128.cc \ third_party/abseil-cpp/absl/status/status.cc \ third_party/abseil-cpp/absl/status/status_payload_printer.cc \ + third_party/abseil-cpp/absl/status/statusor.cc \ third_party/abseil-cpp/absl/strings/ascii.cc \ third_party/abseil-cpp/absl/strings/charconv.cc \ third_party/abseil-cpp/absl/strings/cord.cc \ diff --git a/config.w32 b/config.w32 index 46809387632..824d7fd69d1 100644 --- a/config.w32 +++ b/config.w32 @@ -611,6 +611,7 @@ if (PHP_GRPC != "no") { "third_party\\abseil-cpp\\absl\\numeric\\int128.cc " + "third_party\\abseil-cpp\\absl\\status\\status.cc " + "third_party\\abseil-cpp\\absl\\status\\status_payload_printer.cc " + + "third_party\\abseil-cpp\\absl\\status\\statusor.cc " + "third_party\\abseil-cpp\\absl\\strings\\ascii.cc " + "third_party\\abseil-cpp\\absl\\strings\\charconv.cc " + "third_party\\abseil-cpp\\absl\\strings\\cord.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index f07745cde53..af3cfcedc93 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -186,11 +186,13 @@ Pod::Spec.new do |s| ss.dependency 'gRPC-Core', version abseil_version = '1.20200923.2' ss.dependency 'abseil/base/base', abseil_version + ss.dependency 'abseil/container/flat_hash_map', abseil_version ss.dependency 'abseil/container/flat_hash_set', abseil_version ss.dependency 'abseil/container/inlined_vector', abseil_version ss.dependency 'abseil/functional/bind_front', abseil_version ss.dependency 'abseil/memory/memory', abseil_version ss.dependency 'abseil/status/status', abseil_version + ss.dependency 'abseil/status/statusor', abseil_version ss.dependency 'abseil/strings/str_format', abseil_version ss.dependency 'abseil/strings/strings', abseil_version ss.dependency 'abseil/synchronization/synchronization', abseil_version diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index f5a666e15f0..71c5a3be867 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -175,11 +175,13 @@ Pod::Spec.new do |s| ss.dependency "#{s.name}/Interface", version ss.dependency 'BoringSSL-GRPC', '0.0.14' ss.dependency 'abseil/base/base', abseil_version + ss.dependency 'abseil/container/flat_hash_map', abseil_version ss.dependency 'abseil/container/flat_hash_set', abseil_version ss.dependency 'abseil/container/inlined_vector', abseil_version ss.dependency 'abseil/functional/bind_front', abseil_version ss.dependency 'abseil/memory/memory', abseil_version ss.dependency 'abseil/status/status', abseil_version + ss.dependency 'abseil/status/statusor', abseil_version ss.dependency 'abseil/strings/str_format', abseil_version ss.dependency 'abseil/strings/strings', abseil_version ss.dependency 'abseil/synchronization/synchronization', abseil_version diff --git a/grpc.gemspec b/grpc.gemspec index 24d607a0dbe..6930bae1f87 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1242,6 +1242,7 @@ Gem::Specification.new do |s| s.files += %w( third_party/abseil-cpp/absl/base/port.h ) s.files += %w( third_party/abseil-cpp/absl/base/thread_annotations.h ) s.files += %w( third_party/abseil-cpp/absl/container/fixed_array.h ) + s.files += %w( third_party/abseil-cpp/absl/container/flat_hash_map.h ) s.files += %w( third_party/abseil-cpp/absl/container/flat_hash_set.h ) s.files += %w( third_party/abseil-cpp/absl/container/inlined_vector.h ) s.files += %w( third_party/abseil-cpp/absl/container/internal/common.h ) @@ -1256,6 +1257,7 @@ Gem::Specification.new do |s| s.files += %w( third_party/abseil-cpp/absl/container/internal/have_sse.h ) s.files += %w( third_party/abseil-cpp/absl/container/internal/inlined_vector.h ) s.files += %w( third_party/abseil-cpp/absl/container/internal/layout.h ) + s.files += %w( third_party/abseil-cpp/absl/container/internal/raw_hash_map.h ) s.files += %w( third_party/abseil-cpp/absl/container/internal/raw_hash_set.cc ) s.files += %w( third_party/abseil-cpp/absl/container/internal/raw_hash_set.h ) s.files += %w( third_party/abseil-cpp/absl/debugging/internal/address_is_readable.cc ) @@ -1299,10 +1301,13 @@ Gem::Specification.new do |s| s.files += %w( third_party/abseil-cpp/absl/numeric/int128_have_intrinsic.inc ) s.files += %w( third_party/abseil-cpp/absl/numeric/int128_no_intrinsic.inc ) s.files += %w( third_party/abseil-cpp/absl/status/internal/status_internal.h ) + s.files += %w( third_party/abseil-cpp/absl/status/internal/statusor_internal.h ) s.files += %w( third_party/abseil-cpp/absl/status/status.cc ) s.files += %w( third_party/abseil-cpp/absl/status/status.h ) s.files += %w( third_party/abseil-cpp/absl/status/status_payload_printer.cc ) s.files += %w( third_party/abseil-cpp/absl/status/status_payload_printer.h ) + s.files += %w( third_party/abseil-cpp/absl/status/statusor.cc ) + s.files += %w( third_party/abseil-cpp/absl/status/statusor.h ) s.files += %w( third_party/abseil-cpp/absl/strings/ascii.cc ) s.files += %w( third_party/abseil-cpp/absl/strings/ascii.h ) s.files += %w( third_party/abseil-cpp/absl/strings/charconv.cc ) diff --git a/grpc.gyp b/grpc.gyp index 965a66d14ed..a9cb7b9d473 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -444,10 +444,12 @@ 'upb', 'absl/types:optional', 'absl/strings:strings', + 'absl/status:statusor', 'absl/status:status', 'absl/functional:bind_front', 'absl/container:inlined_vector', 'absl/container:flat_hash_set', + 'absl/container:flat_hash_map', ], 'sources': [ 'src/core/ext/filters/census/grpc_context.cc', @@ -1075,8 +1077,10 @@ 'upb', 'absl/types:optional', 'absl/strings:strings', + 'absl/status:statusor', 'absl/status:status', 'absl/container:inlined_vector', + 'absl/container:flat_hash_map', ], 'sources': [ 'src/core/ext/filters/census/grpc_context.cc', diff --git a/package.xml b/package.xml index 2ec60d6e5df..86a65539804 100644 --- a/package.xml +++ b/package.xml @@ -1244,6 +1244,7 @@ + @@ -1258,6 +1259,7 @@ + @@ -1301,10 +1303,13 @@ + + + diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 77b806cd050..d4b9a08724a 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -314,7 +314,7 @@ class ChannelData { ClientChannelFactory* client_channel_factory_; const grpc_channel_args* channel_args_; RefCountedPtr default_service_config_; - UniquePtr server_name_; + std::string server_name_; UniquePtr target_uri_; channelz::ChannelNode* channelz_node_; @@ -1607,12 +1607,10 @@ ChannelData::ChannelData(grpc_channel_element_args* args, grpc_error** error) default_service_config_.reset(); return; } - grpc_uri* uri = grpc_uri_parse(server_uri, true); - if (uri != nullptr && uri->path[0] != '\0') { - server_name_.reset( - gpr_strdup(uri->path[0] == '/' ? uri->path + 1 : uri->path)); + absl::StatusOr uri = URI::Parse(server_uri); + if (uri.ok() && !uri->path().empty()) { + server_name_ = std::string(absl::StripPrefix(uri->path(), "/")); } - grpc_uri_destroy(uri); char* proxy_name = nullptr; grpc_channel_args* new_args = nullptr; ProxyMapperRegistry::MapName(server_uri, args->channel_args, &proxy_name, @@ -1981,7 +1979,7 @@ void ChannelData::UpdateServiceConfigInDataPlaneLocked() { RefCountedPtr retry_throttle_data; if (retry_throttle_config.has_value()) { retry_throttle_data = internal::ServerRetryThrottleMap::GetDataForServer( - server_name_.get(), retry_throttle_config.value().max_milli_tokens, + server_name_, retry_throttle_config.value().max_milli_tokens, retry_throttle_config.value().milli_token_ratio); } // Grab ref to service config. diff --git a/src/core/ext/filters/client_channel/http_proxy.cc b/src/core/ext/filters/client_channel/http_proxy.cc index 8425a7afccd..f061c424c05 100644 --- a/src/core/ext/filters/client_channel/http_proxy.cc +++ b/src/core/ext/filters/client_channel/http_proxy.cc @@ -24,6 +24,7 @@ #include #include "absl/strings/str_cat.h" +#include "absl/strings/strip.h" #include #include @@ -47,9 +48,10 @@ namespace { * credentials if present in the 'http_proxy' env var, otherwise leaves it * unchanged. It is caller's responsibility to gpr_free user_cred. */ +// TODO(hork): change this to return std::string char* GetHttpProxyServer(const grpc_channel_args* args, char** user_cred) { GPR_ASSERT(user_cred != nullptr); - grpc_uri* uri = nullptr; + absl::StatusOr uri; char* proxy_name = nullptr; char** authority_strs = nullptr; size_t authority_nstrs; @@ -69,17 +71,20 @@ char* GetHttpProxyServer(const grpc_channel_args* args, char** user_cred) { if (uri_str == nullptr) return nullptr; // an emtpy value means "don't use proxy" if (uri_str[0] == '\0') goto done; - uri = grpc_uri_parse(uri_str, false /* suppress_errors */); - if (uri == nullptr || uri->authority == nullptr) { - gpr_log(GPR_ERROR, "cannot parse value of 'http_proxy' env var"); + uri = URI::Parse(uri_str); + if (!uri.ok() || uri->authority().empty()) { + gpr_log(GPR_ERROR, "cannot parse value of 'http_proxy' env var. Error: %s", + uri.status().ToString().c_str()); goto done; } - if (strcmp(uri->scheme, "http") != 0) { - gpr_log(GPR_ERROR, "'%s' scheme not supported in proxy URI", uri->scheme); + if (uri->scheme() != "http") { + gpr_log(GPR_ERROR, "'%s' scheme not supported in proxy URI", + uri->scheme().c_str()); goto done; } /* Split on '@' to separate user credentials from host */ - gpr_string_split(uri->authority, "@", &authority_strs, &authority_nstrs); + gpr_string_split(uri->authority().c_str(), "@", &authority_strs, + &authority_nstrs); GPR_ASSERT(authority_nstrs != 0); /* should have at least 1 string */ if (authority_nstrs == 1) { /* User cred not present in authority */ @@ -99,7 +104,6 @@ char* GetHttpProxyServer(const grpc_channel_args* args, char** user_cred) { gpr_free(authority_strs); done: gpr_free(uri_str); - grpc_uri_destroy(uri); return proxy_name; } @@ -114,15 +118,15 @@ class HttpProxyMapper : public ProxyMapperInterface { *name_to_resolve = GetHttpProxyServer(args, &user_cred); if (*name_to_resolve == nullptr) return false; char* no_proxy_str = nullptr; - grpc_uri* uri = grpc_uri_parse(server_uri, false /* suppress_errors */); - if (uri == nullptr || uri->path[0] == '\0') { + absl::StatusOr uri = URI::Parse(server_uri); + if (!uri.ok() || uri->path().empty()) { gpr_log(GPR_ERROR, "'http_proxy' environment variable set, but cannot " - "parse server URI '%s' -- not using proxy", - server_uri); + "parse server URI '%s' -- not using proxy. Error: %s", + server_uri, uri.status().ToString().c_str()); goto no_use_proxy; } - if (strcmp(uri->scheme, "unix") == 0) { + if (uri->scheme() == "unix") { gpr_log(GPR_INFO, "not using proxy for Unix domain socket '%s'", server_uri); goto no_use_proxy; @@ -135,9 +139,8 @@ class HttpProxyMapper : public ProxyMapperInterface { bool use_proxy = true; std::string server_host; std::string server_port; - if (!grpc_core::SplitHostPort( - uri->path[0] == '/' ? uri->path + 1 : uri->path, &server_host, - &server_port)) { + if (!SplitHostPort(absl::StripPrefix(uri->path(), "/"), &server_host, + &server_port)) { gpr_log(GPR_INFO, "unable to split host and port, not checking no_proxy list for " "host '%s'", @@ -173,7 +176,7 @@ class HttpProxyMapper : public ProxyMapperInterface { grpc_arg args_to_add[2]; args_to_add[0] = grpc_channel_arg_string_create( const_cast(GRPC_ARG_HTTP_CONNECT_SERVER), - uri->path[0] == '/' ? uri->path + 1 : uri->path); + const_cast(absl::StripPrefix(uri->path(), "/").data())); if (user_cred != nullptr) { /* Use base64 encoding for user credentials as stated in RFC 7617 */ char* encoded_user_cred = @@ -188,11 +191,9 @@ class HttpProxyMapper : public ProxyMapperInterface { } else { *new_args = grpc_channel_args_copy_and_add(args, args_to_add, 1); } - grpc_uri_destroy(uri); gpr_free(user_cred); return true; no_use_proxy: - if (uri != nullptr) grpc_uri_destroy(uri); gpr_free(*name_to_resolve); *name_to_resolve = nullptr; gpr_free(user_cred); diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index edb098bc865..6a46a0bbc30 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -68,6 +68,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" +#include "absl/strings/strip.h" #include "upb/upb.hpp" @@ -426,7 +427,7 @@ class GrpcLb : public LoadBalancingPolicy { void CreateOrUpdateChildPolicyLocked(); // Who the client is trying to communicate with. - const char* server_name_ = nullptr; + std::string server_name_; // Configurations for the policy. RefCountedPtr config_; @@ -756,8 +757,7 @@ GrpcLb::BalancerCallState::BalancerCallState( // Init the LB call. Note that the LB call will progress every time there's // activity in grpclb_policy_->interested_parties(), which is comprised of // the polling entities from client_channel. - GPR_ASSERT(grpclb_policy()->server_name_ != nullptr); - GPR_ASSERT(grpclb_policy()->server_name_[0] != '\0'); + GPR_ASSERT(!grpclb_policy()->server_name_.empty()); // Closure Initialization GRPC_CLOSURE_INIT(&lb_on_initial_request_sent_, OnInitialRequestSent, this, grpc_schedule_on_exec_ctx); @@ -780,7 +780,7 @@ GrpcLb::BalancerCallState::BalancerCallState( upb::Arena arena; grpc_slice request_payload_slice = GrpcLbRequestCreate( grpclb_policy()->config_->service_name().empty() - ? grpclb_policy()->server_name_ + ? grpclb_policy()->server_name_.c_str() : grpclb_policy()->config_->service_name().c_str(), arena.ptr()); send_message_payload_ = @@ -1340,15 +1340,14 @@ GrpcLb::GrpcLb(Args args) const grpc_arg* arg = grpc_channel_args_find(args.args, GRPC_ARG_SERVER_URI); const char* server_uri = grpc_channel_arg_get_string(arg); GPR_ASSERT(server_uri != nullptr); - grpc_uri* uri = grpc_uri_parse(server_uri, true); - GPR_ASSERT(uri->path[0] != '\0'); - server_name_ = gpr_strdup(uri->path[0] == '/' ? uri->path + 1 : uri->path); + absl::StatusOr uri = URI::Parse(server_uri); + GPR_ASSERT(uri.ok() && !uri->path().empty()); + server_name_ = std::string(absl::StripPrefix(uri->path(), "/")); if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_glb_trace)) { gpr_log(GPR_INFO, "[grpclb %p] Will use '%s' as the server name for LB request.", - this, server_name_); + this, server_name_.c_str()); } - grpc_uri_destroy(uri); // Record LB call timeout. arg = grpc_channel_args_find(args.args, GRPC_ARG_GRPCLB_CALL_TIMEOUT_MS); lb_call_timeout_ms_ = grpc_channel_arg_get_integer(arg, {0, 0, INT_MAX}); @@ -1358,10 +1357,7 @@ GrpcLb::GrpcLb(Args args) arg, {GRPC_GRPCLB_DEFAULT_FALLBACK_TIMEOUT_MS, 0, INT_MAX}); } -GrpcLb::~GrpcLb() { - gpr_free(const_cast(server_name_)); - grpc_channel_args_destroy(args_); -} +GrpcLb::~GrpcLb() { grpc_channel_args_destroy(args_); } void GrpcLb::ShutdownLocked() { shutting_down_ = true; diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc index 22d7531f53c..c92ee1b3c16 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc @@ -315,15 +315,14 @@ EdsLb::EdsLb(RefCountedPtr xds_client, Args args) const char* server_uri = grpc_channel_args_find_string(args.args, GRPC_ARG_SERVER_URI); GPR_ASSERT(server_uri != nullptr); - grpc_uri* uri = grpc_uri_parse(server_uri, true); - GPR_ASSERT(uri->path[0] != '\0'); - server_name_ = uri->path[0] == '/' ? uri->path + 1 : uri->path; - is_xds_uri_ = strcmp(uri->scheme, "xds") == 0; + absl::StatusOr uri = URI::Parse(server_uri); + GPR_ASSERT(uri.ok() && !uri->path().empty()); + server_name_ = std::string(absl::StripPrefix(uri->path(), "/")); + is_xds_uri_ = uri->scheme() == "xds"; if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_eds_trace)) { gpr_log(GPR_INFO, "[edslb %p] server name from channel (is_xds_uri=%d): %s", this, is_xds_uri_, server_name_.c_str()); } - grpc_uri_destroy(uri); // EDS-only flow. if (!is_xds_uri_) { // Setup channelz linkage. diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index f0bf0cda2fe..0060ad98820 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -86,9 +86,9 @@ class AresDnsResolver : public Resolver { void OnResolvedLocked(grpc_error* error); /// DNS server to use (if not system default) - char* dns_server_; + std::string dns_server_; /// name to resolve (usually the same as target_name) - char* name_to_resolve_; + std::string name_to_resolve_; /// channel args grpc_channel_args* channel_args_; /// whether to request the service config @@ -139,14 +139,9 @@ AresDnsResolver::AresDnsResolver(ResolverArgs args) grpc_schedule_on_exec_ctx); GRPC_CLOSURE_INIT(&on_resolved_, OnResolved, this, grpc_schedule_on_exec_ctx); // Get name to resolve from URI path. - const char* path = args.uri->path; - if (path[0] == '/') ++path; - name_to_resolve_ = gpr_strdup(path); + name_to_resolve_ = std::string(absl::StripPrefix(args.uri.path(), "/")); // Get DNS server from URI authority. - dns_server_ = nullptr; - if (0 != strcmp(args.uri->authority, "")) { - dns_server_ = gpr_strdup(args.uri->authority); - } + dns_server_ = args.uri.authority(); channel_args_ = grpc_channel_args_copy(args.args); // Disable service config option const grpc_arg* arg = grpc_channel_args_find( @@ -175,8 +170,6 @@ AresDnsResolver::AresDnsResolver(ResolverArgs args) AresDnsResolver::~AresDnsResolver() { GRPC_CARES_TRACE_LOG("resolver:%p destroying AresDnsResolver", this); grpc_pollset_set_destroy(interested_parties_); - gpr_free(dns_server_); - gpr_free(name_to_resolve_); grpc_channel_args_destroy(channel_args_); } @@ -439,8 +432,8 @@ void AresDnsResolver::StartResolvingLocked() { resolving_ = true; service_config_json_ = nullptr; pending_request_ = grpc_dns_lookup_ares_locked( - dns_server_, name_to_resolve_, kDefaultPort, interested_parties_, - &on_resolved_, &addresses_, + dns_server_.c_str(), name_to_resolve_.c_str(), kDefaultPort, + interested_parties_, &on_resolved_, &addresses_, enable_srv_queries_ ? &balancer_addresses_ : nullptr, request_service_config_ ? &service_config_json_ : nullptr, query_timeout_ms_, work_serializer()); @@ -455,7 +448,7 @@ void AresDnsResolver::StartResolvingLocked() { class AresDnsResolverFactory : public ResolverFactory { public: - bool IsValidUri(const grpc_uri* /*uri*/) const override { return true; } + bool IsValidUri(const URI& /*uri*/) const override { return true; } OrphanablePtr CreateResolver(ResolverArgs args) const override { return MakeOrphanable(std::move(args)); diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc index 2fd902d0338..7bf8bff1bfb 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc @@ -425,7 +425,7 @@ void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( if (error != GRPC_ERROR_NONE) goto error_cleanup; channel = grpc_ares_ev_driver_get_channel_locked(r->ev_driver); // If dns_server is specified, use it. - if (dns_server != nullptr) { + if (dns_server != nullptr && dns_server[0] != '\0') { GRPC_CARES_TRACE_LOG("request:%p Using DNS server %s", r, dns_server); grpc_resolved_address addr; if (grpc_parse_ipv4_hostport(dns_server, &addr, false /* log_errors */)) { diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index e17d84f414d..3e072aa3b11 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -74,7 +74,7 @@ class NativeDnsResolver : public Resolver { void OnResolvedLocked(grpc_error* error); /// name to resolve - char* name_to_resolve_ = nullptr; + std::string name_to_resolve_; /// channel args grpc_channel_args* channel_args_ = nullptr; /// pollset_set to drive the name resolution process @@ -107,9 +107,7 @@ NativeDnsResolver::NativeDnsResolver(ResolverArgs args) .set_multiplier(GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER) .set_jitter(GRPC_DNS_RECONNECT_JITTER) .set_max_backoff(GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000)) { - char* path = args.uri->path; - if (path[0] == '/') ++path; - name_to_resolve_ = gpr_strdup(path); + name_to_resolve_ = std::string(absl::StripPrefix(args.uri.path(), "/")); channel_args_ = grpc_channel_args_copy(args.args); const grpc_arg* arg = grpc_channel_args_find( args.args, GRPC_ARG_DNS_MIN_TIME_BETWEEN_RESOLUTIONS_MS); @@ -124,7 +122,6 @@ NativeDnsResolver::NativeDnsResolver(ResolverArgs args) NativeDnsResolver::~NativeDnsResolver() { grpc_channel_args_destroy(channel_args_); grpc_pollset_set_destroy(interested_parties_); - gpr_free(name_to_resolve_); } void NativeDnsResolver::StartLocked() { MaybeStartResolvingLocked(); } @@ -269,8 +266,8 @@ void NativeDnsResolver::StartResolvingLocked() { addresses_ = nullptr; GRPC_CLOSURE_INIT(&on_resolved_, NativeDnsResolver::OnResolved, this, grpc_schedule_on_exec_ctx); - grpc_resolve_address(name_to_resolve_, kDefaultPort, interested_parties_, - &on_resolved_, &addresses_); + grpc_resolve_address(name_to_resolve_.c_str(), kDefaultPort, + interested_parties_, &on_resolved_, &addresses_); last_resolution_timestamp_ = grpc_core::ExecCtx::Get()->Now(); } @@ -280,8 +277,8 @@ void NativeDnsResolver::StartResolvingLocked() { class NativeDnsResolverFactory : public ResolverFactory { public: - bool IsValidUri(const grpc_uri* uri) const override { - if (GPR_UNLIKELY(0 != strcmp(uri->authority, ""))) { + bool IsValidUri(const URI& uri) const override { + if (GPR_UNLIKELY(!uri.authority().empty())) { gpr_log(GPR_ERROR, "authority based dns uri's not supported"); return false; } diff --git a/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc b/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc index 8b405f717e0..a779cd47dfc 100644 --- a/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc @@ -362,7 +362,7 @@ namespace { class FakeResolverFactory : public ResolverFactory { public: - bool IsValidUri(const grpc_uri* /*uri*/) const override { return true; } + bool IsValidUri(const URI& /*uri*/) const override { return true; } OrphanablePtr CreateResolver(ResolverArgs args) const override { return MakeOrphanable(std::move(args)); diff --git a/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc b/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc index 4b1f0a1833d..dfa2dd9b972 100644 --- a/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc @@ -23,6 +23,8 @@ #include #include +#include "absl/strings/str_split.h" + #include #include @@ -78,30 +80,20 @@ void SockaddrResolver::StartLocked() { // Factory // -void DoNothing(void* /*ignored*/) {} - -bool ParseUri(const grpc_uri* uri, - bool parse(const grpc_uri* uri, grpc_resolved_address* dst), +bool ParseUri(const URI& uri, + bool parse(const URI& uri, grpc_resolved_address* dst), ServerAddressList* addresses) { - if (0 != strcmp(uri->authority, "")) { + if (!uri.authority().empty()) { gpr_log(GPR_ERROR, "authority-based URIs not supported by the %s scheme", - uri->scheme); + uri.scheme().c_str()); return false; } // Construct addresses. - grpc_slice path_slice = - grpc_slice_new(uri->path, strlen(uri->path), DoNothing); - grpc_slice_buffer path_parts; - grpc_slice_buffer_init(&path_parts); - grpc_slice_split(path_slice, ",", &path_parts); bool errors_found = false; - for (size_t i = 0; i < path_parts.count; i++) { - grpc_uri ith_uri = *uri; - grpc_core::UniquePtr part_str( - grpc_slice_to_c_string(path_parts.slices[i])); - ith_uri.path = part_str.get(); + for (absl::string_view ith_path : absl::StrSplit(uri.path(), ',')) { + URI ith_uri(uri.scheme(), "", std::string(ith_path), {}, ""); grpc_resolved_address addr; - if (!parse(&ith_uri, &addr)) { + if (!parse(ith_uri, &addr)) { errors_found = true; break; } @@ -109,14 +101,11 @@ bool ParseUri(const grpc_uri* uri, addresses->emplace_back(addr, nullptr /* args */); } } - grpc_slice_buffer_destroy_internal(&path_parts); - grpc_slice_unref_internal(path_slice); return !errors_found; } OrphanablePtr CreateSockaddrResolver( - ResolverArgs args, - bool parse(const grpc_uri* uri, grpc_resolved_address* dst)) { + ResolverArgs args, bool parse(const URI& uri, grpc_resolved_address* dst)) { ServerAddressList addresses; if (!ParseUri(args.uri, parse, &addresses)) return nullptr; // Instantiate resolver. @@ -126,7 +115,7 @@ OrphanablePtr CreateSockaddrResolver( class IPv4ResolverFactory : public ResolverFactory { public: - bool IsValidUri(const grpc_uri* uri) const override { + bool IsValidUri(const URI& uri) const override { return ParseUri(uri, grpc_parse_ipv4, nullptr); } @@ -139,7 +128,7 @@ class IPv4ResolverFactory : public ResolverFactory { class IPv6ResolverFactory : public ResolverFactory { public: - bool IsValidUri(const grpc_uri* uri) const override { + bool IsValidUri(const URI& uri) const override { return ParseUri(uri, grpc_parse_ipv6, nullptr); } @@ -153,7 +142,7 @@ class IPv6ResolverFactory : public ResolverFactory { #ifdef GRPC_HAVE_UNIX_SOCKET class UnixResolverFactory : public ResolverFactory { public: - bool IsValidUri(const grpc_uri* uri) const override { + bool IsValidUri(const URI& uri) const override { return ParseUri(uri, grpc_parse_unix, nullptr); } @@ -161,9 +150,8 @@ class UnixResolverFactory : public ResolverFactory { return CreateSockaddrResolver(std::move(args), grpc_parse_unix); } - grpc_core::UniquePtr GetDefaultAuthority( - grpc_uri* /*uri*/) const override { - return grpc_core::UniquePtr(gpr_strdup("localhost")); + std::string GetDefaultAuthority(const URI& uri) const override { + return "localhost"; } const char* scheme() const override { return "unix"; } @@ -171,7 +159,7 @@ class UnixResolverFactory : public ResolverFactory { class UnixAbstractResolverFactory : public ResolverFactory { public: - bool IsValidUri(const grpc_uri* uri) const override { + bool IsValidUri(const URI& uri) const override { return ParseUri(uri, grpc_parse_unix_abstract, nullptr); } @@ -179,9 +167,8 @@ class UnixAbstractResolverFactory : public ResolverFactory { return CreateSockaddrResolver(std::move(args), grpc_parse_unix_abstract); } - grpc_core::UniquePtr GetDefaultAuthority( - grpc_uri* /*uri*/) const override { - return grpc_core::UniquePtr(gpr_strdup("localhost")); + std::string GetDefaultAuthority(const URI& /*uri*/) const override { + return "localhost"; } const char* scheme() const override { return "unix-abstract"; } diff --git a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc index af957f74993..e9028099a6f 100644 --- a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc @@ -48,11 +48,9 @@ class XdsResolver : public Resolver { explicit XdsResolver(ResolverArgs args) : Resolver(std::move(args.work_serializer), std::move(args.result_handler)), + server_name_(absl::StripPrefix(args.uri.path(), "/")), args_(grpc_channel_args_copy(args.args)), interested_parties_(args.pollset_set) { - char* path = args.uri->path; - if (path[0] == '/') ++path; - server_name_ = path; if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_resolver_trace)) { gpr_log(GPR_INFO, "[xds_resolver %p] created for server name %s", this, server_name_.c_str()); @@ -762,8 +760,8 @@ void XdsResolver::MaybeRemoveUnusedClusters() { class XdsResolverFactory : public ResolverFactory { public: - bool IsValidUri(const grpc_uri* uri) const override { - if (GPR_UNLIKELY(0 != strcmp(uri->authority, ""))) { + bool IsValidUri(const URI& uri) const override { + if (GPR_UNLIKELY(!uri.authority().empty())) { gpr_log(GPR_ERROR, "URI authority not supported"); return false; } diff --git a/src/core/ext/filters/client_channel/resolver_factory.h b/src/core/ext/filters/client_channel/resolver_factory.h index d8dd801ae09..7b967461525 100644 --- a/src/core/ext/filters/client_channel/resolver_factory.h +++ b/src/core/ext/filters/client_channel/resolver_factory.h @@ -21,6 +21,8 @@ #include +#include "absl/strings/strip.h" + #include #include "src/core/ext/filters/client_channel/resolver.h" @@ -33,7 +35,7 @@ namespace grpc_core { struct ResolverArgs { /// The parsed URI to resolve. - grpc_uri* uri = nullptr; + URI uri; /// Channel args to be included in resolver results. const grpc_channel_args* args = nullptr; /// Used to drive I/O in the name resolution process. @@ -48,17 +50,15 @@ class ResolverFactory { public: /// Returns a bool indicating whether the input uri is valid to create a /// resolver. - virtual bool IsValidUri(const grpc_uri* uri) const = 0; + virtual bool IsValidUri(const URI& uri) const = 0; /// Returns a new resolver instance. virtual OrphanablePtr CreateResolver(ResolverArgs args) const = 0; /// Returns a string representing the default authority to use for this /// scheme. - virtual grpc_core::UniquePtr GetDefaultAuthority(grpc_uri* uri) const { - const char* path = uri->path; - if (path[0] == '/') ++path; - return grpc_core::UniquePtr(gpr_strdup(path)); + virtual std::string GetDefaultAuthority(const URI& uri) const { + return std::string(absl::StripPrefix(uri.path(), "/")); } /// Returns the URI scheme that this factory implements. diff --git a/src/core/ext/filters/client_channel/resolver_registry.cc b/src/core/ext/filters/client_channel/resolver_registry.cc index c8b7d8dcc02..42b69bbb2ee 100644 --- a/src/core/ext/filters/client_channel/resolver_registry.cc +++ b/src/core/ext/filters/client_channel/resolver_registry.cc @@ -24,6 +24,7 @@ #include "absl/container/inlined_vector.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" #include #include @@ -50,9 +51,9 @@ class RegistryState { factories_.push_back(std::move(factory)); } - ResolverFactory* LookupResolverFactory(const char* scheme) const { + ResolverFactory* LookupResolverFactory(absl::string_view scheme) const { for (size_t i = 0; i < factories_.size(); ++i) { - if (strcmp(scheme, factories_[i]->scheme()) == 0) { + if (scheme == factories_[i]->scheme()) { return factories_[i].get(); } } @@ -65,26 +66,35 @@ class RegistryState { // point to the parsed URI. // If \a default_prefix_ needs to be prepended, sets \a canonical_target // to the canonical target string. - ResolverFactory* FindResolverFactory(const char* target, grpc_uri** uri, + ResolverFactory* FindResolverFactory(absl::string_view target, URI* uri, std::string* canonical_target) const { GPR_ASSERT(uri != nullptr); - *uri = grpc_uri_parse(target, true); + absl::StatusOr tmp_uri = URI::Parse(target); ResolverFactory* factory = - *uri == nullptr ? nullptr : LookupResolverFactory((*uri)->scheme); - if (factory == nullptr) { - grpc_uri_destroy(*uri); - *canonical_target = absl::StrCat(default_prefix_.get(), target); - *uri = grpc_uri_parse(canonical_target->c_str(), true); - factory = - *uri == nullptr ? nullptr : LookupResolverFactory((*uri)->scheme); - if (factory == nullptr) { - grpc_uri_destroy(grpc_uri_parse(target, false)); - grpc_uri_destroy(grpc_uri_parse(canonical_target->c_str(), false)); - gpr_log(GPR_ERROR, "don't know how to resolve '%s' or '%s'", target, - canonical_target->c_str()); - } + tmp_uri.ok() ? LookupResolverFactory(tmp_uri->scheme()) : nullptr; + if (factory != nullptr) { + *uri = *tmp_uri; + return factory; + } + *canonical_target = absl::StrCat(default_prefix_.get(), target); + absl::StatusOr tmp_uri2 = URI::Parse(*canonical_target); + factory = + tmp_uri2.ok() ? LookupResolverFactory(tmp_uri2->scheme()) : nullptr; + if (factory != nullptr) { + *uri = *tmp_uri2; + return factory; } - return factory; + if (!tmp_uri.ok() || !tmp_uri2.ok()) { + gpr_log(GPR_ERROR, "%s", + absl::StrFormat("Error parsing URI(s). '%s':%s; '%s':%s", target, + tmp_uri.status().ToString(), *canonical_target, + tmp_uri2.status().ToString()) + .c_str()); + return nullptr; + } + gpr_log(GPR_ERROR, "Don't know how to resolve '%s' or '%s'.", + std::string(target).c_str(), canonical_target->c_str()); + return nullptr; } private: @@ -134,14 +144,12 @@ ResolverFactory* ResolverRegistry::LookupResolverFactory(const char* scheme) { return g_state->LookupResolverFactory(scheme); } -bool ResolverRegistry::IsValidTarget(const char* target) { - grpc_uri* uri = nullptr; +bool ResolverRegistry::IsValidTarget(absl::string_view target) { + URI uri; std::string canonical_target; ResolverFactory* factory = g_state->FindResolverFactory(target, &uri, &canonical_target); - bool result = factory == nullptr ? false : factory->IsValidUri(uri); - grpc_uri_destroy(uri); - return result; + return factory == nullptr ? false : factory->IsValidUri(uri); } OrphanablePtr ResolverRegistry::CreateResolver( @@ -150,12 +158,10 @@ OrphanablePtr ResolverRegistry::CreateResolver( std::shared_ptr work_serializer, std::unique_ptr result_handler) { GPR_ASSERT(g_state != nullptr); - grpc_uri* uri = nullptr; std::string canonical_target; - ResolverFactory* factory = - g_state->FindResolverFactory(target, &uri, &canonical_target); ResolverArgs resolver_args; - resolver_args.uri = uri; + ResolverFactory* factory = g_state->FindResolverFactory( + target, &resolver_args.uri, &canonical_target); resolver_args.args = args; resolver_args.pollset_set = pollset_set; resolver_args.work_serializer = std::move(work_serializer); @@ -163,30 +169,26 @@ OrphanablePtr ResolverRegistry::CreateResolver( OrphanablePtr resolver = factory == nullptr ? nullptr : factory->CreateResolver(std::move(resolver_args)); - grpc_uri_destroy(uri); return resolver; } -grpc_core::UniquePtr ResolverRegistry::GetDefaultAuthority( - const char* target) { +std::string ResolverRegistry::GetDefaultAuthority(absl::string_view target) { GPR_ASSERT(g_state != nullptr); - grpc_uri* uri = nullptr; + URI uri; std::string canonical_target; ResolverFactory* factory = g_state->FindResolverFactory(target, &uri, &canonical_target); - grpc_core::UniquePtr authority = - factory == nullptr ? nullptr : factory->GetDefaultAuthority(uri); - grpc_uri_destroy(uri); + std::string authority = + factory == nullptr ? "" : factory->GetDefaultAuthority(uri); return authority; } grpc_core::UniquePtr ResolverRegistry::AddDefaultPrefixIfNeeded( const char* target) { GPR_ASSERT(g_state != nullptr); - grpc_uri* uri = nullptr; + URI uri; std::string canonical_target; g_state->FindResolverFactory(target, &uri, &canonical_target); - grpc_uri_destroy(uri); return grpc_core::UniquePtr(canonical_target.empty() ? gpr_strdup(target) : gpr_strdup(canonical_target.c_str())); diff --git a/src/core/ext/filters/client_channel/resolver_registry.h b/src/core/ext/filters/client_channel/resolver_registry.h index bf34216b2cf..f2e5b9c195f 100644 --- a/src/core/ext/filters/client_channel/resolver_registry.h +++ b/src/core/ext/filters/client_channel/resolver_registry.h @@ -51,7 +51,7 @@ class ResolverRegistry { }; /// Checks whether the user input \a target is valid to create a resolver. - static bool IsValidTarget(const char* target); + static bool IsValidTarget(absl::string_view target); /// Creates a resolver given \a target. /// First tries to parse \a target as a URI. If this succeeds, tries @@ -73,7 +73,7 @@ class ResolverRegistry { std::unique_ptr result_handler); /// Returns the default authority to pass from a client for \a target. - static grpc_core::UniquePtr GetDefaultAuthority(const char* target); + static std::string GetDefaultAuthority(absl::string_view target); /// Returns \a target with the default prefix prepended, if needed. static grpc_core::UniquePtr AddDefaultPrefixIfNeeded( diff --git a/src/core/ext/filters/client_channel/retry_throttle.cc b/src/core/ext/filters/client_channel/retry_throttle.cc index 65df8034d71..03c7a673efd 100644 --- a/src/core/ext/filters/client_channel/retry_throttle.cc +++ b/src/core/ext/filters/client_channel/retry_throttle.cc @@ -23,6 +23,8 @@ #include #include +#include + #include #include #include @@ -164,20 +166,20 @@ void ServerRetryThrottleMap::Shutdown() { } RefCountedPtr ServerRetryThrottleMap::GetDataForServer( - const char* server_name, intptr_t max_milli_tokens, + const std::string& server_name, intptr_t max_milli_tokens, intptr_t milli_token_ratio) { RefCountedPtr result; gpr_mu_lock(&g_mu); ServerRetryThrottleData* throttle_data = static_cast( - grpc_avl_get(g_avl, const_cast(server_name), nullptr)); + grpc_avl_get(g_avl, const_cast(server_name.c_str()), nullptr)); if (throttle_data == nullptr || throttle_data->max_milli_tokens() != max_milli_tokens || throttle_data->milli_token_ratio() != milli_token_ratio) { // Entry not found, or found with old parameters. Create a new one. result = MakeRefCounted( max_milli_tokens, milli_token_ratio, throttle_data); - g_avl = grpc_avl_add(g_avl, gpr_strdup(server_name), + g_avl = grpc_avl_add(g_avl, gpr_strdup(server_name.c_str()), result->Ref().release(), nullptr); } else { // Entry found. Return a new ref to it. diff --git a/src/core/ext/filters/client_channel/retry_throttle.h b/src/core/ext/filters/client_channel/retry_throttle.h index e40363fd3d1..583def35c93 100644 --- a/src/core/ext/filters/client_channel/retry_throttle.h +++ b/src/core/ext/filters/client_channel/retry_throttle.h @@ -67,7 +67,7 @@ class ServerRetryThrottleMap { /// Returns the failure data for \a server_name, creating a new entry if /// needed. static RefCountedPtr GetDataForServer( - const char* server_name, intptr_t max_milli_tokens, + const std::string& server_name, intptr_t max_milli_tokens, intptr_t milli_token_ratio); }; diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 6834ec65224..9885f4cdcff 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -929,10 +929,12 @@ const char* Subchannel::GetUriFromSubchannelAddressArg( namespace { void UriToSockaddr(const char* uri_str, grpc_resolved_address* addr) { - grpc_uri* uri = grpc_uri_parse(uri_str, false /* suppress_errors */); - GPR_ASSERT(uri != nullptr); - if (!grpc_parse_uri(uri, addr)) memset(addr, 0, sizeof(*addr)); - grpc_uri_destroy(uri); + absl::StatusOr uri = URI::Parse(uri_str); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } + if (!grpc_parse_uri(*uri, addr)) memset(addr, 0, sizeof(*addr)); } } // namespace diff --git a/src/core/ext/filters/load_reporting/server_load_reporting_filter.cc b/src/core/ext/filters/load_reporting/server_load_reporting_filter.cc index b3f99281054..5d578307a9e 100644 --- a/src/core/ext/filters/load_reporting/server_load_reporting_filter.cc +++ b/src/core/ext/filters/load_reporting/server_load_reporting_filter.cc @@ -134,18 +134,18 @@ std::string ServerLoadReportingCallData::GetCensusSafeClientIpString() { "metadata."); return ""; } - // Parse the client URI string into grpc_uri. - grpc_uri* client_uri = grpc_uri_parse(client_uri_str, true); - if (client_uri == nullptr) { + absl::StatusOr client_uri = + grpc_core::URI::Parse(client_uri_str); + if (!client_uri.ok()) { gpr_log(GPR_ERROR, "Unable to parse the client URI string (peer string) to a client " - "URI."); + "URI. Error: %s", + client_uri.status().ToString().c_str()); return ""; } // Parse the client URI into grpc_resolved_address. grpc_resolved_address resolved_address; - bool success = grpc_parse_uri(client_uri, &resolved_address); - grpc_uri_destroy(client_uri); + bool success = grpc_parse_uri(*client_uri, &resolved_address); if (!success) { gpr_log(GPR_ERROR, "Unable to parse client URI into a grpc_resolved_address."); diff --git a/src/core/ext/transport/chttp2/client/authority.cc b/src/core/ext/transport/chttp2/client/authority.cc index bad3153b013..8e4f8c1bbdf 100644 --- a/src/core/ext/transport/chttp2/client/authority.cc +++ b/src/core/ext/transport/chttp2/client/authority.cc @@ -26,7 +26,7 @@ grpc_channel_args* grpc_default_authority_add_if_not_present( grpc_channel_args_find(args, GRPC_ARG_DEFAULT_AUTHORITY) != nullptr; grpc_arg new_args[1]; size_t num_new_args = 0; - grpc_core::UniquePtr default_authority; + std::string default_authority; if (!has_default_authority) { const grpc_arg* server_uri_arg = grpc_channel_args_find(args, GRPC_ARG_SERVER_URI); @@ -34,9 +34,9 @@ grpc_channel_args* grpc_default_authority_add_if_not_present( GPR_ASSERT(server_uri_str != nullptr); default_authority = grpc_core::ResolverRegistry::GetDefaultAuthority(server_uri_str); - GPR_ASSERT(default_authority != nullptr); new_args[num_new_args++] = grpc_channel_arg_string_create( - const_cast(GRPC_ARG_DEFAULT_AUTHORITY), default_authority.get()); + const_cast(GRPC_ARG_DEFAULT_AUTHORITY), + const_cast(default_authority.c_str())); } return grpc_channel_args_copy_and_add(args, new_args, num_new_args); } diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc index 0b22c7de367..7edf6aa34ed 100644 --- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc +++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc @@ -78,9 +78,8 @@ class Chttp2SecureClientChannelFactory : public ClientChannelFactory { // First, check the authority override channel arg. // Otherwise, get it from the server name used to construct the // channel. - grpc_core::UniquePtr authority( - gpr_strdup(FindAuthorityOverrideInArgs(args))); - if (authority == nullptr) { + std::string authority(FindAuthorityOverrideInArgs(args)); + if (authority.empty()) { const char* server_uri_str = grpc_channel_args_find_string(args, GRPC_ARG_SERVER_URI); GPR_ASSERT(server_uri_str != nullptr); @@ -92,7 +91,8 @@ class Chttp2SecureClientChannelFactory : public ClientChannelFactory { // If the channel args don't already contain GRPC_ARG_DEFAULT_AUTHORITY, // add the arg, setting it to the value just obtained. args_to_add[num_args_to_add++] = grpc_channel_arg_string_create( - const_cast(GRPC_ARG_DEFAULT_AUTHORITY), authority.get()); + const_cast(GRPC_ARG_DEFAULT_AUTHORITY), + const_cast(authority.c_str())); } grpc_channel_args* args_with_authority = grpc_channel_args_copy_and_add(args, args_to_add, num_args_to_add); @@ -101,12 +101,12 @@ class Chttp2SecureClientChannelFactory : public ClientChannelFactory { RefCountedPtr subchannel_security_connector = channel_credentials->create_security_connector( - /*call_creds=*/nullptr, authority.get(), args_with_authority, + /*call_creds=*/nullptr, authority.c_str(), args_with_authority, &new_args_from_connector); if (subchannel_security_connector == nullptr) { gpr_log(GPR_ERROR, "Failed to create secure subchannel for secure name '%s'", - authority.get()); + authority.c_str()); grpc_channel_args_destroy(args_with_authority); return nullptr; } diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index c0dbfd05f48..188203cbbbe 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -20,6 +20,8 @@ #include "src/core/lib/channel/channelz.h" +#include "absl/strings/strip.h" + #include #include #include @@ -344,14 +346,12 @@ void PopulateSocketAddressJson(Json::Object* json, const char* name, const char* addr_str) { if (addr_str == nullptr) return; Json::Object data; - grpc_uri* uri = grpc_uri_parse(addr_str, true); - if ((uri != nullptr) && ((strcmp(uri->scheme, "ipv4") == 0) || - (strcmp(uri->scheme, "ipv6") == 0))) { - const char* host_port = uri->path; - if (*host_port == '/') ++host_port; + absl::StatusOr uri = URI::Parse(addr_str); + if (uri.ok() && (uri->scheme() == "ipv4" || uri->scheme() == "ipv6")) { std::string host; std::string port; - GPR_ASSERT(SplitHostPort(host_port, &host, &port)); + GPR_ASSERT( + SplitHostPort(absl::StripPrefix(uri->path(), "/"), &host, &port)); int port_num = -1; if (!port.empty()) { port_num = atoi(port.data()); @@ -362,16 +362,15 @@ void PopulateSocketAddressJson(Json::Object* json, const char* name, {"ip_address", b64_host}, }; gpr_free(b64_host); - } else if (uri != nullptr && strcmp(uri->scheme, "unix") == 0) { + } else if (uri.ok() && uri->scheme() == "unix") { data["uds_address"] = Json::Object{ - {"filename", uri->path}, + {"filename", uri->path()}, }; } else { data["other_address"] = Json::Object{ {"name", addr_str}, }; } - grpc_uri_destroy(uri); (*json)[name] = std::move(data); } diff --git a/src/core/lib/iomgr/parse_address.cc b/src/core/lib/iomgr/parse_address.cc index 9e8f7859e00..9afac74fd41 100644 --- a/src/core/lib/iomgr/parse_address.cc +++ b/src/core/lib/iomgr/parse_address.cc @@ -18,18 +18,20 @@ #include -#include "absl/strings/str_cat.h" - -#include "src/core/lib/iomgr/grpc_if_nametoindex.h" #include "src/core/lib/iomgr/parse_address.h" -#include "src/core/lib/iomgr/sockaddr.h" -#include "src/core/lib/iomgr/socket_utils.h" #include #include #ifdef GRPC_HAVE_UNIX_SOCKET #include #endif +#ifdef GRPC_POSIX_SOCKET +#include +#include +#endif + +#include "absl/strings/str_cat.h" +#include "absl/strings/strip.h" #include #include @@ -37,21 +39,21 @@ #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/host_port.h" - -#ifdef GRPC_POSIX_SOCKET -#include -#include -#endif +#include "src/core/lib/iomgr/grpc_if_nametoindex.h" +#include "src/core/lib/iomgr/sockaddr.h" +#include "src/core/lib/iomgr/socket_utils.h" #ifdef GRPC_HAVE_UNIX_SOCKET -bool grpc_parse_unix(const grpc_uri* uri, +bool grpc_parse_unix(const grpc_core::URI& uri, grpc_resolved_address* resolved_addr) { - if (strcmp("unix", uri->scheme) != 0) { - gpr_log(GPR_ERROR, "Expected 'unix' scheme, got '%s'", uri->scheme); + if (uri.scheme() != "unix") { + gpr_log(GPR_ERROR, "Expected 'unix' scheme, got '%s'", + uri.scheme().c_str()); return false; } - grpc_error* error = grpc_core::UnixSockaddrPopulate(uri->path, resolved_addr); + grpc_error* error = + grpc_core::UnixSockaddrPopulate(uri.path(), resolved_addr); if (error != GRPC_ERROR_NONE) { gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); GRPC_ERROR_UNREF(error); @@ -60,15 +62,15 @@ bool grpc_parse_unix(const grpc_uri* uri, return true; } -bool grpc_parse_unix_abstract(const grpc_uri* uri, +bool grpc_parse_unix_abstract(const grpc_core::URI& uri, grpc_resolved_address* resolved_addr) { - if (strcmp("unix-abstract", uri->scheme) != 0) { + if (uri.scheme() != "unix-abstract") { gpr_log(GPR_ERROR, "Expected 'unix-abstract' scheme, got '%s'", - uri->scheme); + uri.scheme().c_str()); return false; } grpc_error* error = - grpc_core::UnixAbstractSockaddrPopulate(uri->path, resolved_addr); + grpc_core::UnixAbstractSockaddrPopulate(uri.path(), resolved_addr); if (error != GRPC_ERROR_NONE) { gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); GRPC_ERROR_UNREF(error); @@ -120,12 +122,12 @@ grpc_error* UnixAbstractSockaddrPopulate(absl::string_view path, #else /* GRPC_HAVE_UNIX_SOCKET */ -bool grpc_parse_unix(const grpc_uri* uri, +bool grpc_parse_unix(const grpc_core::URI& uri, grpc_resolved_address* resolved_addr) { abort(); } -bool grpc_parse_unix_abstract(const grpc_uri* uri, +bool grpc_parse_unix_abstract(const grpc_core::URI& uri, grpc_resolved_address* resolved_addr) { abort(); } @@ -145,15 +147,16 @@ grpc_error* UnixAbstractSockaddrPopulate(absl::string_view path, } // namespace grpc_core #endif /* GRPC_HAVE_UNIX_SOCKET */ -bool grpc_parse_ipv4_hostport(const char* hostport, grpc_resolved_address* addr, - bool log_errors) { +bool grpc_parse_ipv4_hostport(absl::string_view hostport, + grpc_resolved_address* addr, bool log_errors) { bool success = false; // Split host and port. std::string host; std::string port; if (!grpc_core::SplitHostPort(hostport, &host, &port)) { if (log_errors) { - gpr_log(GPR_ERROR, "Failed gpr_split_host_port(%s, ...)", hostport); + gpr_log(GPR_ERROR, "Failed gpr_split_host_port(%s, ...)", + std::string(hostport).c_str()); } return false; } @@ -185,27 +188,27 @@ done: return success; } -bool grpc_parse_ipv4(const grpc_uri* uri, +bool grpc_parse_ipv4(const grpc_core::URI& uri, grpc_resolved_address* resolved_addr) { - if (strcmp("ipv4", uri->scheme) != 0) { - gpr_log(GPR_ERROR, "Expected 'ipv4' scheme, got '%s'", uri->scheme); + if (uri.scheme() != "ipv4") { + gpr_log(GPR_ERROR, "Expected 'ipv4' scheme, got '%s'", + uri.scheme().c_str()); return false; } - const char* host_port = uri->path; - if (*host_port == '/') ++host_port; - return grpc_parse_ipv4_hostport(host_port, resolved_addr, - true /* log_errors */); + return grpc_parse_ipv4_hostport(absl::StripPrefix(uri.path(), "/"), + resolved_addr, true /* log_errors */); } -bool grpc_parse_ipv6_hostport(const char* hostport, grpc_resolved_address* addr, - bool log_errors) { +bool grpc_parse_ipv6_hostport(absl::string_view hostport, + grpc_resolved_address* addr, bool log_errors) { bool success = false; // Split host and port. std::string host; std::string port; if (!grpc_core::SplitHostPort(hostport, &host, &port)) { if (log_errors) { - gpr_log(GPR_ERROR, "Failed gpr_split_host_port(%s, ...)", hostport); + gpr_log(GPR_ERROR, "Failed gpr_split_host_port(%s, ...)", + std::string(hostport).c_str()); } return false; } @@ -280,29 +283,32 @@ done: return success; } -bool grpc_parse_ipv6(const grpc_uri* uri, +bool grpc_parse_ipv6(const grpc_core::URI& uri, grpc_resolved_address* resolved_addr) { - if (strcmp("ipv6", uri->scheme) != 0) { - gpr_log(GPR_ERROR, "Expected 'ipv6' scheme, got '%s'", uri->scheme); + if (uri.scheme() != "ipv6") { + gpr_log(GPR_ERROR, "Expected 'ipv6' scheme, got '%s'", + uri.scheme().c_str()); return false; } - const char* host_port = uri->path; - if (*host_port == '/') ++host_port; - return grpc_parse_ipv6_hostport(host_port, resolved_addr, - true /* log_errors */); + return grpc_parse_ipv6_hostport(absl::StripPrefix(uri.path(), "/"), + resolved_addr, true /* log_errors */); } -bool grpc_parse_uri(const grpc_uri* uri, grpc_resolved_address* resolved_addr) { - if (strcmp("unix", uri->scheme) == 0) { +bool grpc_parse_uri(const grpc_core::URI& uri, + grpc_resolved_address* resolved_addr) { + if (uri.scheme() == "unix") { return grpc_parse_unix(uri, resolved_addr); - } else if (strcmp("unix-abstract", uri->scheme) == 0) { + } + if (uri.scheme() == "unix-abstract") { return grpc_parse_unix_abstract(uri, resolved_addr); - } else if (strcmp("ipv4", uri->scheme) == 0) { + } + if (uri.scheme() == "ipv4") { return grpc_parse_ipv4(uri, resolved_addr); - } else if (strcmp("ipv6", uri->scheme) == 0) { + } + if (uri.scheme() == "ipv6") { return grpc_parse_ipv6(uri, resolved_addr); } - gpr_log(GPR_ERROR, "Can't parse scheme '%s'", uri->scheme); + gpr_log(GPR_ERROR, "Can't parse scheme '%s'", uri.scheme().c_str()); return false; } diff --git a/src/core/lib/iomgr/parse_address.h b/src/core/lib/iomgr/parse_address.h index 29c8321a0cf..870afefa4eb 100644 --- a/src/core/lib/iomgr/parse_address.h +++ b/src/core/lib/iomgr/parse_address.h @@ -30,29 +30,33 @@ /** Populate \a resolved_addr from \a uri, whose path is expected to contain a * unix socket path. Returns true upon success. */ -bool grpc_parse_unix(const grpc_uri* uri, grpc_resolved_address* resolved_addr); +bool grpc_parse_unix(const grpc_core::URI& uri, + grpc_resolved_address* resolved_addr); /** Populate \a resolved_addr from \a uri, whose path is expected to contain a * unix socket path in the abstract namespace. Returns true upon success. */ -bool grpc_parse_unix_abstract(const grpc_uri* uri, +bool grpc_parse_unix_abstract(const grpc_core::URI& uri, grpc_resolved_address* resolved_addr); /** Populate \a resolved_addr from \a uri, whose path is expected to contain an * IPv4 host:port pair. Returns true upon success. */ -bool grpc_parse_ipv4(const grpc_uri* uri, grpc_resolved_address* resolved_addr); +bool grpc_parse_ipv4(const grpc_core::URI& uri, + grpc_resolved_address* resolved_addr); /** Populate \a resolved_addr from \a uri, whose path is expected to contain an * IPv6 host:port pair. Returns true upon success. */ -bool grpc_parse_ipv6(const grpc_uri* uri, grpc_resolved_address* resolved_addr); +bool grpc_parse_ipv6(const grpc_core::URI& uri, + grpc_resolved_address* resolved_addr); /** Populate \a resolved_addr from \a uri. Returns true upon success. */ -bool grpc_parse_uri(const grpc_uri* uri, grpc_resolved_address* resolved_addr); +bool grpc_parse_uri(const grpc_core::URI& uri, + grpc_resolved_address* resolved_addr); /** Parse bare IPv4 or IPv6 "IP:port" strings. */ -bool grpc_parse_ipv4_hostport(const char* hostport, grpc_resolved_address* addr, - bool log_errors); -bool grpc_parse_ipv6_hostport(const char* hostport, grpc_resolved_address* addr, - bool log_errors); +bool grpc_parse_ipv4_hostport(absl::string_view hostport, + grpc_resolved_address* addr, bool log_errors); +bool grpc_parse_ipv6_hostport(absl::string_view hostport, + grpc_resolved_address* addr, bool log_errors); /* Converts named or numeric port to a uint16 suitable for use in a sockaddr. */ uint16_t grpc_strhtons(const char* port); diff --git a/src/core/lib/security/authorization/evaluate_args.cc b/src/core/lib/security/authorization/evaluate_args.cc index 68f6cb803d3..6eaf10670da 100644 --- a/src/core/lib/security/authorization/evaluate_args.cc +++ b/src/core/lib/security/authorization/evaluate_args.cc @@ -87,14 +87,12 @@ int EvaluateArgs::GetLocalPort() const { if (endpoint_ == nullptr) { return 0; } - grpc_uri* uri = grpc_uri_parse( - std::string(grpc_endpoint_get_local_address(endpoint_)).c_str(), true); + absl::StatusOr uri = + URI::Parse(grpc_endpoint_get_local_address(endpoint_)); grpc_resolved_address resolved_addr; - if (uri == nullptr || !grpc_parse_uri(uri, &resolved_addr)) { - grpc_uri_destroy(uri); + if (!uri.ok() || !grpc_parse_uri(*uri, &resolved_addr)) { return 0; } - grpc_uri_destroy(uri); return grpc_sockaddr_get_port(&resolved_addr); } @@ -113,14 +111,11 @@ int EvaluateArgs::GetPeerPort() const { if (endpoint_ == nullptr) { return 0; } - grpc_uri* uri = grpc_uri_parse( - std::string(grpc_endpoint_get_peer(endpoint_)).c_str(), true); + absl::StatusOr uri = URI::Parse(grpc_endpoint_get_peer(endpoint_)); grpc_resolved_address resolved_addr; - if (uri == nullptr || !grpc_parse_uri(uri, &resolved_addr)) { - grpc_uri_destroy(uri); + if (!uri.ok() || !grpc_parse_uri(*uri, &resolved_addr)) { return 0; } - grpc_uri_destroy(uri); return grpc_sockaddr_get_port(&resolved_addr); } diff --git a/src/core/lib/security/credentials/external/aws_external_account_credentials.cc b/src/core/lib/security/credentials/external/aws_external_account_credentials.cc index 12428c8b2a6..01c7b005ce5 100644 --- a/src/core/lib/security/credentials/external/aws_external_account_credentials.cc +++ b/src/core/lib/security/credentials/external/aws_external_account_credentials.cc @@ -149,21 +149,20 @@ void AwsExternalAccountCredentials::RetrieveRegion() { } return; } - grpc_uri* uri = grpc_uri_parse(region_url_, false); - if (uri == nullptr) { - FinishRetrieveSubjectToken( - "", - GRPC_ERROR_CREATE_FROM_COPIED_STRING( - absl::StrFormat("Invalid region url: %s.", region_url_).c_str())); + absl::StatusOr uri = URI::Parse(region_url_); + if (!uri.ok()) { + FinishRetrieveSubjectToken("", GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrFormat("Invalid region url. %s", + uri.status().ToString()) + .c_str())); return; } grpc_httpcli_request request; memset(&request, 0, sizeof(grpc_httpcli_request)); - request.host = const_cast(uri->authority); - request.http.path = gpr_strdup(uri->path); - request.handshaker = (strcmp(uri->scheme, "https") == 0) - ? &grpc_httpcli_ssl - : &grpc_httpcli_plaintext; + request.host = const_cast(uri->authority().c_str()); + request.http.path = gpr_strdup(uri->path().c_str()); + request.handshaker = + uri->scheme() == "https" ? &grpc_httpcli_ssl : &grpc_httpcli_plaintext; grpc_resource_quota* resource_quota = grpc_resource_quota_create("external_account_credentials"); grpc_http_response_destroy(&ctx_->response); @@ -173,7 +172,6 @@ void AwsExternalAccountCredentials::RetrieveRegion() { &request, ctx_->deadline, &ctx_->closure, &ctx_->response); grpc_resource_quota_unref_internal(resource_quota); grpc_http_request_destroy(&request.http); - grpc_uri_destroy(uri); } void AwsExternalAccountCredentials::OnRetrieveRegion(void* arg, @@ -201,20 +199,20 @@ void AwsExternalAccountCredentials::OnRetrieveRegionInternal( } void AwsExternalAccountCredentials::RetrieveRoleName() { - grpc_uri* uri = grpc_uri_parse(url_, false); - if (uri == nullptr) { + absl::StatusOr uri = URI::Parse(url_); + if (!uri.ok()) { FinishRetrieveSubjectToken( "", GRPC_ERROR_CREATE_FROM_COPIED_STRING( - absl::StrFormat("Invalid url: %s.", url_).c_str())); + absl::StrFormat("Invalid url: %s.", uri.status().ToString()) + .c_str())); return; } grpc_httpcli_request request; memset(&request, 0, sizeof(grpc_httpcli_request)); - request.host = const_cast(uri->authority); - request.http.path = gpr_strdup(uri->path); - request.handshaker = (strcmp(uri->scheme, "https") == 0) - ? &grpc_httpcli_ssl - : &grpc_httpcli_plaintext; + request.host = const_cast(uri->authority().c_str()); + request.http.path = gpr_strdup(uri->path().c_str()); + request.handshaker = + uri->scheme() == "https" ? &grpc_httpcli_ssl : &grpc_httpcli_plaintext; grpc_resource_quota* resource_quota = grpc_resource_quota_create("external_account_credentials"); grpc_http_response_destroy(&ctx_->response); @@ -224,7 +222,6 @@ void AwsExternalAccountCredentials::RetrieveRoleName() { &request, ctx_->deadline, &ctx_->closure, &ctx_->response); grpc_resource_quota_unref_internal(resource_quota); grpc_http_request_destroy(&request.http); - grpc_uri_destroy(uri); } void AwsExternalAccountCredentials::OnRetrieveRoleName(void* arg, @@ -264,22 +261,21 @@ void AwsExternalAccountCredentials::RetrieveSigningKeys() { return; } std::string url_with_role_name = absl::StrCat(url_, "/", role_name_); - grpc_uri* uri = grpc_uri_parse(url_with_role_name, false); - if (uri == nullptr) { + absl::StatusOr uri = URI::Parse(url_with_role_name); + if (!uri.ok()) { FinishRetrieveSubjectToken( "", GRPC_ERROR_CREATE_FROM_COPIED_STRING( absl::StrFormat("Invalid url with role name: %s.", - url_with_role_name) + uri.status().ToString()) .c_str())); return; } grpc_httpcli_request request; memset(&request, 0, sizeof(grpc_httpcli_request)); - request.host = const_cast(uri->authority); - request.http.path = gpr_strdup(uri->path); - request.handshaker = (strcmp(uri->scheme, "https") == 0) - ? &grpc_httpcli_ssl - : &grpc_httpcli_plaintext; + request.host = const_cast(uri->authority().c_str()); + request.http.path = gpr_strdup(uri->path().c_str()); + request.handshaker = + uri->scheme() == "https" ? &grpc_httpcli_ssl : &grpc_httpcli_plaintext; grpc_resource_quota* resource_quota = grpc_resource_quota_create("external_account_credentials"); grpc_http_response_destroy(&ctx_->response); @@ -289,7 +285,6 @@ void AwsExternalAccountCredentials::RetrieveSigningKeys() { &request, ctx_->deadline, &ctx_->closure, &ctx_->response); grpc_resource_quota_unref_internal(resource_quota); grpc_http_request_destroy(&request.http); - grpc_uri_destroy(uri); } void AwsExternalAccountCredentials::OnRetrieveSigningKeys(void* arg, diff --git a/src/core/lib/security/credentials/external/aws_request_signer.cc b/src/core/lib/security/credentials/external/aws_request_signer.cc index b51f595df36..af8531ee02a 100644 --- a/src/core/lib/security/credentials/external/aws_request_signer.cc +++ b/src/core/lib/security/credentials/external/aws_request_signer.cc @@ -95,15 +95,14 @@ AwsRequestSigner::AwsRequestSigner( static_request_date_ = absl::FormatTime(kXAmzDateFormat, request_date, absl::UTCTimeZone()); } - url_ = grpc_uri_parse(url, false); - if (url_ == nullptr) { + absl::StatusOr tmp_url = URI::Parse(url); + if (!tmp_url.ok()) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Invalid Aws request url."); return; } + url_ = tmp_url.value(); } -AwsRequestSigner::~AwsRequestSigner() { grpc_uri_destroy(url_); } - std::map AwsRequestSigner::GetSignedRequestHeaders() { std::string request_date_full; if (!static_request_date_.empty()) { @@ -124,15 +123,20 @@ std::map AwsRequestSigner::GetSignedRequestHeaders() { canonical_request_vector.emplace_back(method_); canonical_request_vector.emplace_back("\n"); // 2. CanonicalURI - - canonical_request_vector.emplace_back(*url_->path == '\0' ? "/" : url_->path); + canonical_request_vector.emplace_back( + url_.path().empty() ? "/" : absl::string_view(url_.path())); canonical_request_vector.emplace_back("\n"); // 3. CanonicalQueryString - canonical_request_vector.emplace_back(url_->query); + std::vector query_vector; + for (const URI::QueryParam& query_kv : url_.query_parameter_pairs()) { + query_vector.emplace_back(absl::StrCat(query_kv.key, "=", query_kv.value)); + } + std::string query = absl::StrJoin(query_vector, "&"); + canonical_request_vector.emplace_back(query); canonical_request_vector.emplace_back("\n"); // 4. CanonicalHeaders if (request_headers_.empty()) { - request_headers_.insert({"host", url_->authority}); + request_headers_.insert({"host", url_.authority()}); if (!token_.empty()) { request_headers_.insert({"x-amz-security-token", token_}); } @@ -177,7 +181,7 @@ std::map AwsRequestSigner::GetSignedRequestHeaders() { string_to_sign_vector.emplace_back("\n"); // 3. CredentialScope std::pair host_parts = - absl::StrSplit(url_->authority, absl::MaxSplits('.', 1)); + absl::StrSplit(url_.authority(), absl::MaxSplits('.', 1)); std::string service_name(host_parts.first); std::string credential_scope = absl::StrFormat( "%s/%s/%s/aws4_request", request_date_short, region_, service_name); diff --git a/src/core/lib/security/credentials/external/aws_request_signer.h b/src/core/lib/security/credentials/external/aws_request_signer.h index a4823e079ae..62e112cf0f4 100644 --- a/src/core/lib/security/credentials/external/aws_request_signer.h +++ b/src/core/lib/security/credentials/external/aws_request_signer.h @@ -46,7 +46,6 @@ class AwsRequestSigner { std::string region, std::string request_payload, std::map additional_headers, grpc_error** error); - ~AwsRequestSigner(); // This method triggers the signing process then returns the headers of the // signed request as a map. In case there is an error, the input `error` @@ -59,7 +58,7 @@ class AwsRequestSigner { std::string secret_access_key_; std::string token_; std::string method_; - grpc_uri* url_ = nullptr; + URI url_; std::string region_; std::string request_payload_; std::map additional_headers_; diff --git a/src/core/lib/security/credentials/external/external_account_credentials.cc b/src/core/lib/security/credentials/external/external_account_credentials.cc index 599b718655f..eff0890a95b 100644 --- a/src/core/lib/security/credentials/external/external_account_credentials.cc +++ b/src/core/lib/security/credentials/external/external_account_credentials.cc @@ -89,16 +89,18 @@ void ExternalAccountCredentials::OnRetrieveSubjectTokenInternal( void ExternalAccountCredentials::ExchangeToken( absl::string_view subject_token) { - grpc_uri* uri = grpc_uri_parse(options_.token_url, false); - if (uri == nullptr) { + absl::StatusOr uri = URI::Parse(options_.token_url); + if (!uri.ok()) { FinishTokenFetch(GRPC_ERROR_CREATE_FROM_COPIED_STRING( - absl::StrFormat("Invalid token url: %s.", options_.token_url).c_str())); + absl::StrFormat("Invalid token url: %s. Error: %s", options_.token_url, + uri.status().ToString()) + .c_str())); return; } grpc_httpcli_request request; memset(&request, 0, sizeof(grpc_httpcli_request)); - request.host = const_cast(uri->authority); - request.http.path = gpr_strdup(uri->path); + request.host = const_cast(uri->authority().c_str()); + request.http.path = gpr_strdup(uri->path().c_str()); grpc_http_header* headers = nullptr; if (!options_.client_id.empty() && !options_.client_secret.empty()) { request.http.hdr_count = 2; @@ -122,9 +124,8 @@ void ExternalAccountCredentials::ExchangeToken( headers[0].value = gpr_strdup("application/x-www-form-urlencoded"); } request.http.hdrs = headers; - request.handshaker = (strcmp(uri->scheme, "https") == 0) - ? &grpc_httpcli_ssl - : &grpc_httpcli_plaintext; + request.handshaker = + uri->scheme() == "https" ? &grpc_httpcli_ssl : &grpc_httpcli_plaintext; std::vector body_parts; body_parts.push_back(absl::StrFormat("%s=%s", "audience", options_.audience)); body_parts.push_back(absl::StrFormat( @@ -152,7 +153,6 @@ void ExternalAccountCredentials::ExchangeToken( &ctx_->closure, &ctx_->response); grpc_resource_quota_unref_internal(resource_quota); grpc_http_request_destroy(&request.http); - grpc_uri_destroy(uri); } void ExternalAccountCredentials::OnExchangeToken(void* arg, grpc_error* error) { @@ -195,19 +195,20 @@ void ExternalAccountCredentials::ImpersenateServiceAccount() { return; } std::string access_token = it->second.string_value(); - grpc_uri* uri = - grpc_uri_parse(options_.service_account_impersonation_url, false); - if (uri == nullptr) { + absl::StatusOr uri = + URI::Parse(options_.service_account_impersonation_url); + if (!uri.ok()) { FinishTokenFetch(GRPC_ERROR_CREATE_FROM_COPIED_STRING( - absl::StrFormat("Invalid service account impersonation url: %s.", - options_.service_account_impersonation_url) + absl::StrFormat( + "Invalid service account impersonation url: %s. Error: %s", + options_.service_account_impersonation_url, uri.status().ToString()) .c_str())); return; } grpc_httpcli_request request; memset(&request, 0, sizeof(grpc_httpcli_request)); - request.host = const_cast(uri->authority); - request.http.path = gpr_strdup(uri->path); + request.host = const_cast(uri->authority().c_str()); + request.http.path = gpr_strdup(uri->path().c_str()); request.http.hdr_count = 2; grpc_http_header* headers = static_cast( gpr_malloc(sizeof(grpc_http_header) * request.http.hdr_count)); @@ -217,9 +218,8 @@ void ExternalAccountCredentials::ImpersenateServiceAccount() { headers[1].key = gpr_strdup("Authorization"); headers[1].value = gpr_strdup(str.c_str()); request.http.hdrs = headers; - request.handshaker = (strcmp(uri->scheme, "https") == 0) - ? &grpc_httpcli_ssl - : &grpc_httpcli_plaintext; + request.handshaker = + uri->scheme() == "https" ? &grpc_httpcli_ssl : &grpc_httpcli_plaintext; std::string scope = absl::StrJoin(scopes_, " "); std::string body = absl::StrFormat("%s=%s", "scope", scope); grpc_resource_quota* resource_quota = @@ -232,7 +232,6 @@ void ExternalAccountCredentials::ImpersenateServiceAccount() { &ctx_->closure, &ctx_->response); grpc_resource_quota_unref_internal(resource_quota); grpc_http_request_destroy(&request.http); - grpc_uri_destroy(uri); } void ExternalAccountCredentials::OnImpersenateServiceAccount( diff --git a/src/core/lib/security/credentials/external/url_external_account_credentials.cc b/src/core/lib/security/credentials/external/url_external_account_credentials.cc index 43029e0deb4..af02a17ed9d 100644 --- a/src/core/lib/security/credentials/external/url_external_account_credentials.cc +++ b/src/core/lib/security/credentials/external/url_external_account_credentials.cc @@ -48,13 +48,15 @@ UrlExternalAccountCredentials::UrlExternalAccountCredentials( GRPC_ERROR_CREATE_FROM_STATIC_STRING("url field must be a string."); return; } - grpc_uri* url = grpc_uri_parse(it->second.string_value(), false); - if (url == nullptr) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Invalid credential source url."); + absl::StatusOr tmp_url = URI::Parse(it->second.string_value()); + if (!tmp_url.ok()) { + *error = GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrFormat("Invalid credential source url. Error: %s", + tmp_url.status().ToString()) + .c_str()); return; } - url_ = url; + url_ = *tmp_url; it = options.credential_source.object_value().find("headers"); if (it != options.credential_source.object_value().end()) { if (it->second.type() != Json::Type::OBJECT) { @@ -104,10 +106,6 @@ UrlExternalAccountCredentials::UrlExternalAccountCredentials( } } -UrlExternalAccountCredentials::~UrlExternalAccountCredentials() { - grpc_uri_destroy(url_); -} - void UrlExternalAccountCredentials::RetrieveSubjectToken( HTTPRequestContext* ctx, const ExternalAccountCredentialsOptions& options, std::function cb) { @@ -122,8 +120,8 @@ void UrlExternalAccountCredentials::RetrieveSubjectToken( cb_ = cb; grpc_httpcli_request request; memset(&request, 0, sizeof(grpc_httpcli_request)); - request.host = const_cast(url_->authority); - request.http.path = gpr_strdup(url_->path); + request.host = const_cast(url_.authority().c_str()); + request.http.path = gpr_strdup(url_.path().c_str()); grpc_http_header* headers = nullptr; request.http.hdr_count = headers_.size(); headers = static_cast( @@ -135,9 +133,8 @@ void UrlExternalAccountCredentials::RetrieveSubjectToken( ++i; } request.http.hdrs = headers; - request.handshaker = (strcmp(url_->scheme, "https") == 0) - ? &grpc_httpcli_ssl - : &grpc_httpcli_plaintext; + request.handshaker = + url_.scheme() == "https" ? &grpc_httpcli_ssl : &grpc_httpcli_plaintext; grpc_resource_quota* resource_quota = grpc_resource_quota_create("external_account_credentials"); grpc_http_response_destroy(&ctx_->response); diff --git a/src/core/lib/security/credentials/external/url_external_account_credentials.h b/src/core/lib/security/credentials/external/url_external_account_credentials.h index 8383ab523a7..16f6f36c035 100644 --- a/src/core/lib/security/credentials/external/url_external_account_credentials.h +++ b/src/core/lib/security/credentials/external/url_external_account_credentials.h @@ -32,7 +32,6 @@ class UrlExternalAccountCredentials final : public ExternalAccountCredentials { UrlExternalAccountCredentials(ExternalAccountCredentialsOptions options, std::vector scopes, grpc_error** error); - ~UrlExternalAccountCredentials() override; private: void RetrieveSubjectToken( @@ -45,7 +44,7 @@ class UrlExternalAccountCredentials final : public ExternalAccountCredentials { void FinishRetrieveSubjectToken(std::string subject_token, grpc_error* error); // Fields of credential source - grpc_uri* url_ = nullptr; + URI url_; std::map headers_; std::string format_type_; std::string format_subject_token_field_name_; diff --git a/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc b/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc index 1ed839a57c9..bbd2cb5bae3 100644 --- a/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc +++ b/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc @@ -538,9 +538,9 @@ grpc_error* LoadTokenFile(const char* path, gpr_slice* token) { class StsTokenFetcherCredentials : public grpc_oauth2_token_fetcher_credentials { public: - StsTokenFetcherCredentials(grpc_uri* sts_url, // Ownership transferred. + StsTokenFetcherCredentials(URI sts_url, const grpc_sts_credentials_options* options) - : sts_url_(sts_url), + : sts_url_(std::move(sts_url)), resource_(gpr_strdup(options->resource)), audience_(gpr_strdup(options->audience)), scope_(gpr_strdup(options->scope)), @@ -550,12 +550,10 @@ class StsTokenFetcherCredentials actor_token_path_(gpr_strdup(options->actor_token_path)), actor_token_type_(gpr_strdup(options->actor_token_type)) {} - ~StsTokenFetcherCredentials() override { grpc_uri_destroy(sts_url_); } - std::string debug_string() override { return absl::StrFormat( - "StsTokenFetcherCredentials{Path:%s,Authority:%s,%s}", sts_url_->path, - sts_url_->authority, + "StsTokenFetcherCredentials{Path:%s,Authority:%s,%s}", sts_url_.path(), + sts_url_.authority(), grpc_oauth2_token_fetcher_credentials::debug_string()); } @@ -578,11 +576,11 @@ class StsTokenFetcherCredentials const_cast("application/x-www-form-urlencoded")}; grpc_httpcli_request request; memset(&request, 0, sizeof(grpc_httpcli_request)); - request.host = sts_url_->authority; - request.http.path = sts_url_->path; + request.host = const_cast(sts_url_.authority().c_str()); + request.http.path = const_cast(sts_url_.path().c_str()); request.http.hdr_count = 1; request.http.hdrs = &header; - request.handshaker = (strcmp(sts_url_->scheme, "https") == 0) + request.handshaker = (sts_url_.scheme() == "https") ? &grpc_httpcli_ssl : &grpc_httpcli_plaintext; /* TODO(ctiller): Carry the resource_quota in ctx and share it with the host @@ -642,7 +640,7 @@ class StsTokenFetcherCredentials return cleanup(); } - grpc_uri* sts_url_; + URI sts_url_; grpc_closure http_post_cb_closure_; grpc_core::UniquePtr resource_; grpc_core::UniquePtr audience_; @@ -656,26 +654,21 @@ class StsTokenFetcherCredentials } // namespace -grpc_error* ValidateStsCredentialsOptions( - const grpc_sts_credentials_options* options, grpc_uri** sts_url_out) { - struct GrpcUriDeleter { - void operator()(grpc_uri* uri) { grpc_uri_destroy(uri); } - }; - *sts_url_out = nullptr; +absl::StatusOr ValidateStsCredentialsOptions( + const grpc_sts_credentials_options* options) { absl::InlinedVector error_list; - std::unique_ptr sts_url( - options->token_exchange_service_uri != nullptr - ? grpc_uri_parse(options->token_exchange_service_uri, false) - : nullptr); - if (sts_url == nullptr) { + absl::StatusOr sts_url = + URI::Parse(options->token_exchange_service_uri == nullptr + ? "" + : options->token_exchange_service_uri); + if (!sts_url.ok()) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrFormat("Invalid or missing STS endpoint URL. Error: %s", + sts_url.status().ToString()) + .c_str())); + } else if (sts_url->scheme() != "https" && sts_url->scheme() != "http") { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Invalid or missing STS endpoint URL")); - } else { - if (strcmp(sts_url->scheme, "https") != 0 && - strcmp(sts_url->scheme, "http") != 0) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Invalid URI scheme, must be https to http.")); - } + "Invalid URI scheme, must be https to http.")); } if (options->subject_token_path == nullptr || strlen(options->subject_token_path) == 0) { @@ -688,12 +681,13 @@ grpc_error* ValidateStsCredentialsOptions( "subject_token_type needs to be specified")); } if (error_list.empty()) { - *sts_url_out = sts_url.release(); - return GRPC_ERROR_NONE; - } else { - return GRPC_ERROR_CREATE_FROM_VECTOR("Invalid STS Credentials Options", - &error_list); + return sts_url; } + auto grpc_error_vec = GRPC_ERROR_CREATE_FROM_VECTOR( + "Invalid STS Credentials Options", &error_list); + auto retval = absl::InvalidArgumentError(grpc_error_string(grpc_error_vec)); + GRPC_ERROR_UNREF(grpc_error_vec); + return retval; } } // namespace grpc_core @@ -701,17 +695,15 @@ grpc_error* ValidateStsCredentialsOptions( grpc_call_credentials* grpc_sts_credentials_create( const grpc_sts_credentials_options* options, void* reserved) { GPR_ASSERT(reserved == nullptr); - grpc_uri* sts_url; - grpc_error* error = - grpc_core::ValidateStsCredentialsOptions(options, &sts_url); - if (error != GRPC_ERROR_NONE) { + absl::StatusOr sts_url = + grpc_core::ValidateStsCredentialsOptions(options); + if (!sts_url.ok()) { gpr_log(GPR_ERROR, "STS Credentials creation failed. Error: %s.", - grpc_error_string(error)); - GRPC_ERROR_UNREF(error); + sts_url.status().ToString().c_str()); return nullptr; } return grpc_core::MakeRefCounted( - sts_url, options) + std::move(*sts_url), options) .release(); } diff --git a/src/core/lib/security/credentials/oauth2/oauth2_credentials.h b/src/core/lib/security/credentials/oauth2/oauth2_credentials.h index 6111d9adcaa..264291965eb 100644 --- a/src/core/lib/security/credentials/oauth2/oauth2_credentials.h +++ b/src/core/lib/security/credentials/oauth2/oauth2_credentials.h @@ -165,8 +165,8 @@ namespace grpc_core { // Exposed for testing only. This function validates the options, ensuring that // the required fields are set, and outputs the parsed URL of the STS token // exchanged service. -grpc_error* ValidateStsCredentialsOptions( - const grpc_sts_credentials_options* options, grpc_uri** sts_url); +absl::StatusOr ValidateStsCredentialsOptions( + const grpc_sts_credentials_options* options); } // namespace grpc_core #endif /* GRPC_CORE_LIB_SECURITY_CREDENTIALS_OAUTH2_OAUTH2_CREDENTIALS_H */ diff --git a/src/core/lib/transport/authority_override.cc b/src/core/lib/transport/authority_override.cc index 8c13320b73b..668507250fe 100644 --- a/src/core/lib/transport/authority_override.cc +++ b/src/core/lib/transport/authority_override.cc @@ -16,6 +16,8 @@ #include +#include "absl/strings/string_view.h" + #include "src/core/lib/channel/channel_args.h" // Channel arg key for the authority override. @@ -23,16 +25,16 @@ namespace grpc_core { -/// Returns a channel argument containing \a authority. grpc_arg CreateAuthorityOverrideChannelArg(const char* authority) { return grpc_channel_arg_string_create( const_cast(GRPC_ARG_AUTHORITY_OVERRIDE), const_cast(authority)); } -/// Returns the authority override from \a args or nullptr. -const char* FindAuthorityOverrideInArgs(const grpc_channel_args* args) { - return grpc_channel_args_find_string(args, GRPC_ARG_AUTHORITY_OVERRIDE); +absl::string_view FindAuthorityOverrideInArgs(const grpc_channel_args* args) { + const char* found = + grpc_channel_args_find_string(args, GRPC_ARG_AUTHORITY_OVERRIDE); + return found == nullptr ? "" : found; } } // namespace grpc_core diff --git a/src/core/lib/transport/authority_override.h b/src/core/lib/transport/authority_override.h index 0cbf27e316b..7af6526e0a3 100644 --- a/src/core/lib/transport/authority_override.h +++ b/src/core/lib/transport/authority_override.h @@ -19,6 +19,8 @@ #include +#include "absl/strings/string_view.h" + #include namespace grpc_core { @@ -26,8 +28,9 @@ namespace grpc_core { /// Returns a channel argument containing \a authority. grpc_arg CreateAuthorityOverrideChannelArg(const char* authority); -/// Returns the authority override from \a args or nullptr. -const char* FindAuthorityOverrideInArgs(const grpc_channel_args* args); +/// Returns the authority override from \a args or the empty string. The return +/// value is a string_view into the `args` data structure. +absl::string_view FindAuthorityOverrideInArgs(const grpc_channel_args* args); } // namespace grpc_core diff --git a/src/core/lib/uri/uri_parser.cc b/src/core/lib/uri/uri_parser.cc index 28a76d9c094..e47daeb5a4b 100644 --- a/src/core/lib/uri/uri_parser.cc +++ b/src/core/lib/uri/uri_parser.cc @@ -22,288 +22,168 @@ #include +#include #include +#include "absl/strings/escaping.h" #include "absl/strings/str_format.h" +#include "absl/strings/str_split.h" -#include -#include #include #include "src/core/lib/gpr/string.h" -#include "src/core/lib/slice/percent_encoding.h" -#include "src/core/lib/slice/slice_internal.h" -#include "src/core/lib/slice/slice_string_helpers.h" -/** a size_t default value... maps to all 1's */ -#define NOT_SET (~(size_t)0) +namespace grpc_core { +namespace { -static grpc_uri* bad_uri(absl::string_view uri_text, size_t pos, - const char* section, bool suppress_errors) { - if (!suppress_errors) { - std::string line_prefix = absl::StrFormat("bad uri.%s: '", section); - gpr_log(GPR_ERROR, "%s%s'", line_prefix.c_str(), - std::string(uri_text).c_str()); - size_t pfx_len = line_prefix.size() + pos; - gpr_log(GPR_ERROR, "%s^ here", std::string(pfx_len, ' ').c_str()); +// Similar to `grpc_permissive_percent_decode_slice`, this %-decodes all valid +// triplets, and passes through the rest verbatim. +std::string PercentDecode(absl::string_view str) { + if (str.empty() || !absl::StrContains(str, "%")) { + return std::string(str); + } + std::string out; + std::string unescaped; + out.reserve(str.size()); + for (size_t i = 0; i < str.length(); i++) { + if (str[i] != '%') { + out += str[i]; + continue; + } + if (i + 3 >= str.length() || + !absl::CUnescape(absl::StrCat("\\x", str.substr(i + 1, 2)), + &unescaped)) { + out += str[i]; + } else { + out += unescaped[0]; + i += 2; + } } - return nullptr; -} - -/** Returns a copy of percent decoded \a src[begin, end) */ -static char* decode_and_copy_component(absl::string_view src, size_t begin, - size_t end) { - grpc_slice component = - (begin == NOT_SET || end == NOT_SET) - ? grpc_empty_slice() - : grpc_slice_from_copied_buffer(src.data() + begin, end - begin); - grpc_slice decoded_component = - grpc_permissive_percent_decode_slice(component); - char* out = grpc_dump_slice(decoded_component, GPR_DUMP_ASCII); - grpc_slice_unref_internal(component); - grpc_slice_unref_internal(decoded_component); return out; } -static bool valid_hex(char c) { - return ((c >= 'a') && (c <= 'f')) || ((c >= 'A') && (c <= 'F')) || - ((c >= '0') && (c <= '9')); +// Checks if this string is made up of pchars, '/', '?', and '%' exclusively. +// See https://tools.ietf.org/html/rfc3986#section-3.4 +bool IsPCharString(absl::string_view str) { + return (str.find_first_not_of("ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789" + "?/:@\\-._~!$&'()*+,;=%") == + absl::string_view::npos); } -/** Returns how many chars to advance if \a uri_text[i] begins a valid \a pchar - * production. If \a uri_text[i] introduces an invalid \a pchar (such as percent - * sign not followed by two hex digits), NOT_SET is returned. */ -static size_t parse_pchar(absl::string_view uri_text, size_t i) { - /* pchar = unreserved / pct-encoded / sub-delims / ":" / "@" - * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" - * pct-encoded = "%" HEXDIG HEXDIG - * sub-delims = "!" / "$" / "&" / "'" / "(" / ")" - / "*" / "+" / "," / ";" / "=" */ - char c = uri_text[i]; - switch (c) { - default: - if (((c >= 'a') && (c <= 'z')) || ((c >= 'A') && (c <= 'Z')) || - ((c >= '0') && (c <= '9'))) { - return 1; - } - break; - case ':': - case '@': - case '-': - case '.': - case '_': - case '~': - case '!': - case '$': - case '&': - case '\'': - case '(': - case ')': - case '*': - case '+': - case ',': - case ';': - case '=': - return 1; - case '%': /* pct-encoded */ - if (uri_text.size() > i + 2 && valid_hex(uri_text[i + 1]) && - valid_hex(uri_text[i + 2])) { - return 2; - } - return NOT_SET; - } - return 0; +absl::Status MakeInvalidURIStatus(absl::string_view part_name, + absl::string_view uri, + absl::string_view extra) { + return absl::InvalidArgumentError(absl::StrFormat( + "Could not parse '%s' from uri '%s'. %s", part_name, uri, extra)); } - -/* *( pchar / "?" / "/" ) */ -static int parse_fragment_or_query(absl::string_view uri_text, size_t* i) { - while (uri_text.size() > *i) { - const size_t advance = parse_pchar(uri_text, *i); /* pchar */ - switch (advance) { - case 0: /* uri_text[i] isn't in pchar */ - /* maybe it's ? or / */ - if (uri_text[*i] == '?' || uri_text[*i] == '/') { - (*i)++; - break; - } else { - return 1; - } - GPR_UNREACHABLE_CODE(return 0); - default: - (*i) += advance; - break; - case NOT_SET: /* uri_text[i] introduces an invalid URI */ - return 0; - } +} // namespace + +absl::StatusOr URI::Parse(absl::string_view uri_text) { + absl::StatusOr decoded; + absl::string_view remaining = uri_text; + // parse scheme + size_t idx = remaining.find(':'); + if (idx == remaining.npos || idx == 0) { + return MakeInvalidURIStatus("scheme", uri_text, "Scheme not found."); } - /* *i is the first uri_text position past the \a query production, maybe \0 */ - return 1; -} - -static void parse_query_parts(grpc_uri* uri) { - static const char* QUERY_PARTS_SEPARATOR = "&"; - static const char* QUERY_PARTS_VALUE_SEPARATOR = "="; - GPR_ASSERT(uri->query != nullptr); - if (uri->query[0] == '\0') { - uri->query_parts = nullptr; - uri->query_parts_values = nullptr; - uri->num_query_parts = 0; - return; + std::string scheme(remaining.substr(0, idx)); + if (scheme.find_first_not_of("ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789+-.") != std::string::npos) { + return MakeInvalidURIStatus("scheme", uri_text, + "Scheme contains invalid characters."); } - - gpr_string_split(uri->query, QUERY_PARTS_SEPARATOR, &uri->query_parts, - &uri->num_query_parts); - uri->query_parts_values = - static_cast(gpr_malloc(uri->num_query_parts * sizeof(char**))); - for (size_t i = 0; i < uri->num_query_parts; i++) { - char** query_param_parts; - size_t num_query_param_parts; - char* full = uri->query_parts[i]; - gpr_string_split(full, QUERY_PARTS_VALUE_SEPARATOR, &query_param_parts, - &num_query_param_parts); - GPR_ASSERT(num_query_param_parts > 0); - uri->query_parts[i] = query_param_parts[0]; - if (num_query_param_parts > 1) { - /* TODO(dgq): only the first value after the separator is considered. - * Perhaps all chars after the first separator for the query part should - * be included, even if they include the separator. */ - uri->query_parts_values[i] = query_param_parts[1]; - } else { - uri->query_parts_values[i] = nullptr; - } - for (size_t j = 2; j < num_query_param_parts; j++) { - gpr_free(query_param_parts[j]); - } - gpr_free(query_param_parts); - gpr_free(full); + if (!isalpha(scheme[0])) { + return MakeInvalidURIStatus( + "scheme", uri_text, + "Scheme must begin with an alpha character [A-Za-z]."); } -} - -grpc_uri* grpc_uri_parse(absl::string_view uri_text, bool suppress_errors) { - grpc_uri* uri; - size_t scheme_begin = 0; - size_t scheme_end = NOT_SET; - size_t authority_begin = NOT_SET; - size_t authority_end = NOT_SET; - size_t path_begin = NOT_SET; - size_t path_end = NOT_SET; - size_t query_begin = NOT_SET; - size_t query_end = NOT_SET; - size_t fragment_begin = NOT_SET; - size_t fragment_end = NOT_SET; - size_t i; - - for (i = scheme_begin; i < uri_text.size(); ++i) { - if (uri_text[i] == ':') { - scheme_end = i; - break; - } - if (uri_text[i] >= 'a' && uri_text[i] <= 'z') continue; - if (uri_text[i] >= 'A' && uri_text[i] <= 'Z') continue; - if (i != scheme_begin) { - if (uri_text[i] >= '0' && uri_text[i] <= '9') continue; - if (uri_text[i] == '+') continue; - if (uri_text[i] == '-') continue; - if (uri_text[i] == '.') continue; - } - break; + remaining.remove_prefix(scheme.length() + 1); + // parse authority + std::string authority; + if (absl::StartsWith(remaining, "//")) { + remaining.remove_prefix(2); + authority = + PercentDecode(remaining.substr(0, remaining.find_first_of("/?#"))); + remaining.remove_prefix(authority.length()); } - if (scheme_end == NOT_SET) { - return bad_uri(uri_text, i, "scheme", suppress_errors); + // parse path + std::string path; + if (!remaining.empty()) { + path = PercentDecode(remaining.substr(0, remaining.find_first_of("?#"))); + remaining.remove_prefix(path.length()); } - - if (uri_text.size() > scheme_end + 2 && uri_text[scheme_end + 1] == '/' && - uri_text[scheme_end + 2] == '/') { - authority_begin = scheme_end + 3; - for (i = authority_begin; uri_text.size() > i && authority_end == NOT_SET; - i++) { - if (uri_text[i] == '/' || uri_text[i] == '?' || uri_text[i] == '#') { - authority_end = i; - } + // parse query + std::vector query_param_pairs; + if (!remaining.empty() && remaining[0] == '?') { + remaining.remove_prefix(1); + absl::string_view tmp_query = remaining.substr(0, remaining.find('#')); + if (tmp_query.empty()) { + return MakeInvalidURIStatus("query", uri_text, "Invalid query string."); } - if (authority_end == NOT_SET && uri_text.size() == i) { - authority_end = i; + if (!IsPCharString(tmp_query)) { + return MakeInvalidURIStatus("query string", uri_text, + "Query string contains invalid characters."); } - if (authority_end == NOT_SET) { - return bad_uri(uri_text, i, "authority", suppress_errors); + for (absl::string_view query_param : absl::StrSplit(tmp_query, '&')) { + const std::pair possible_kv = + absl::StrSplit(query_param, absl::MaxSplits('=', 1)); + if (possible_kv.first.empty()) continue; + query_param_pairs.push_back({PercentDecode(possible_kv.first), + PercentDecode(possible_kv.second)}); } - /* TODO(ctiller): parse the authority correctly */ - path_begin = authority_end; - } else { - path_begin = scheme_end + 1; + remaining.remove_prefix(tmp_query.length()); } - - for (i = path_begin; i < uri_text.size(); ++i) { - if (uri_text[i] == '?' || uri_text[i] == '#') { - path_end = i; - break; + std::string fragment; + if (!remaining.empty() && remaining[0] == '#') { + remaining.remove_prefix(1); + if (!IsPCharString(remaining)) { + return MakeInvalidURIStatus("fragment", uri_text, + "Fragment contains invalid characters."); } + fragment = PercentDecode(remaining); } - if (path_end == NOT_SET && uri_text.size() == i) { - path_end = i; - } - if (path_end == NOT_SET) { - return bad_uri(uri_text, i, "path", suppress_errors); - } + return URI(std::move(scheme), std::move(authority), std::move(path), + std::move(query_param_pairs), std::move(fragment)); +} - if (uri_text.size() > i && uri_text[i] == '?') { - query_begin = ++i; - if (!parse_fragment_or_query(uri_text, &i)) { - return bad_uri(uri_text, i, "query", suppress_errors); - } else if (uri_text.size() > i && uri_text[i] != '#') { - /* We must be at the end or at the beginning of a fragment */ - return bad_uri(uri_text, i, "query", suppress_errors); - } - query_end = i; - } - if (uri_text.size() > i && uri_text[i] == '#') { - fragment_begin = ++i; - if (!parse_fragment_or_query(uri_text, &i)) { - return bad_uri(uri_text, i - fragment_end, "fragment", suppress_errors); - } else if (uri_text.size() > i) { - /* We must be at the end */ - return bad_uri(uri_text, i, "fragment", suppress_errors); - } - fragment_end = i; +URI::URI(std::string scheme, std::string authority, std::string path, + std::vector query_parameter_pairs, std::string fragment) + : scheme_(std::move(scheme)), + authority_(std::move(authority)), + path_(std::move(path)), + query_parameter_pairs_(std::move(query_parameter_pairs)), + fragment_(std::move(fragment)) { + for (const auto& kv : query_parameter_pairs_) { + query_parameter_map_[kv.key] = kv.value; } - - uri = static_cast(gpr_zalloc(sizeof(*uri))); - uri->scheme = decode_and_copy_component(uri_text, scheme_begin, scheme_end); - uri->authority = - decode_and_copy_component(uri_text, authority_begin, authority_end); - uri->path = decode_and_copy_component(uri_text, path_begin, path_end); - uri->query = decode_and_copy_component(uri_text, query_begin, query_end); - uri->fragment = - decode_and_copy_component(uri_text, fragment_begin, fragment_end); - parse_query_parts(uri); - - return uri; } -const char* grpc_uri_get_query_arg(const grpc_uri* uri, const char* key) { - GPR_ASSERT(key != nullptr); - if (key[0] == '\0') return nullptr; - - for (size_t i = 0; i < uri->num_query_parts; ++i) { - if (0 == strcmp(key, uri->query_parts[i])) { - return uri->query_parts_values[i]; - } +URI::URI(const URI& other) + : scheme_(other.scheme_), + authority_(other.authority_), + path_(other.path_), + query_parameter_pairs_(other.query_parameter_pairs_), + fragment_(other.fragment_) { + for (const auto& kv : query_parameter_pairs_) { + query_parameter_map_[kv.key] = kv.value; } - return nullptr; } -void grpc_uri_destroy(grpc_uri* uri) { - if (!uri) return; - gpr_free(uri->scheme); - gpr_free(uri->authority); - gpr_free(uri->path); - gpr_free(uri->query); - for (size_t i = 0; i < uri->num_query_parts; ++i) { - gpr_free(uri->query_parts[i]); - gpr_free(uri->query_parts_values[i]); +URI& URI::operator=(const URI& other) { + if (this == &other) { + return *this; + } + scheme_ = other.scheme_; + authority_ = other.authority_; + path_ = other.path_; + query_parameter_pairs_ = other.query_parameter_pairs_; + fragment_ = other.fragment_; + for (const auto& kv : query_parameter_pairs_) { + query_parameter_map_[kv.key] = kv.value; } - gpr_free(uri->query_parts); - gpr_free(uri->query_parts_values); - gpr_free(uri->fragment); - gpr_free(uri); + return *this; } +} // namespace grpc_core diff --git a/src/core/lib/uri/uri_parser.h b/src/core/lib/uri/uri_parser.h index e000afc0578..dcd7e12dd2f 100644 --- a/src/core/lib/uri/uri_parser.h +++ b/src/core/lib/uri/uri_parser.h @@ -21,31 +21,67 @@ #include +#include + +#include +#include +#include + +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" -#include +namespace grpc_core { -struct grpc_uri { - char* scheme; - char* authority; - char* path; - char* query; - /** Query substrings separated by '&' */ - char** query_parts; - /** Number of elements in \a query_parts and \a query_parts_values */ - size_t num_query_parts; - /** Split each query part by '='. NULL if not present. */ - char** query_parts_values; - char* fragment; -}; -/** parse a uri, return NULL on failure */ -grpc_uri* grpc_uri_parse(absl::string_view uri_text, bool suppress_errors); +class URI { + public: + struct QueryParam { + std::string key; + std::string value; + bool operator==(const QueryParam& other) const { + return key == other.key && value == other.value; + } + }; + + // Creates an instance of GrpcURI by parsing an rfc3986 URI string. Returns + // an IllegalArgumentError on failure. + static absl::StatusOr Parse(absl::string_view uri_text); + // Explicit construction by individual URI components + URI(std::string scheme, std::string authority, std::string path, + std::vector query_parameter_pairs, std::string fragment_); + URI() = default; + // Copy construction and assignment + URI(const URI& other); + URI& operator=(const URI& other); + // Move construction and assignment + URI(URI&&) = default; + URI& operator=(URI&&) = default; -/** return the part of a query string after the '=' in "?key=xxx&...", or NULL - * if key is not present */ -const char* grpc_uri_get_query_arg(const grpc_uri* uri, const char* key); + const std::string& scheme() const { return scheme_; } + const std::string& authority() const { return authority_; } + const std::string& path() const { return path_; } + // Stores the *last* value appearing for each repeated key in the query + // string. If you need to capture repeated query parameters, use + // `query_parameter_pairs`. + const std::map& query_parameter_map() + const { + return query_parameter_map_; + } + // A vector of key:value query parameter pairs, kept in order of appearance + // within the URI search string. Repeated keys are represented as separate + // key:value elements. + const std::vector& query_parameter_pairs() const { + return query_parameter_pairs_; + } + const std::string& fragment() const { return fragment_; } -/** destroy a uri */ -void grpc_uri_destroy(grpc_uri* uri); + private: + std::string scheme_; + std::string authority_; + std::string path_; + std::map query_parameter_map_; + std::vector query_parameter_pairs_; + std::string fragment_; +}; +} // namespace grpc_core #endif /* GRPC_CORE_LIB_URI_URI_PARSER_H */ diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index d9ec88e4df2..c5e0e1b567b 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -610,6 +610,7 @@ CORE_SOURCE_FILES = [ 'third_party/abseil-cpp/absl/numeric/int128.cc', 'third_party/abseil-cpp/absl/status/status.cc', 'third_party/abseil-cpp/absl/status/status_payload_printer.cc', + 'third_party/abseil-cpp/absl/status/statusor.cc', 'third_party/abseil-cpp/absl/strings/ascii.cc', 'third_party/abseil-cpp/absl/strings/charconv.cc', 'third_party/abseil-cpp/absl/strings/cord.cc', diff --git a/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc b/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc index 163a4f42776..08bc8d5a9a8 100644 --- a/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc @@ -95,15 +95,17 @@ static grpc_core::OrphanablePtr create_resolver( std::unique_ptr result_handler) { grpc_core::ResolverFactory* factory = grpc_core::ResolverRegistry::LookupResolverFactory("dns"); - grpc_uri* uri = grpc_uri_parse(name, false); - GPR_ASSERT(uri); + absl::StatusOr uri = grpc_core::URI::Parse(name); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_core::ResolverArgs args; - args.uri = uri; + args.uri = std::move(*uri); args.work_serializer = *g_work_serializer; args.result_handler = std::move(result_handler); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); - grpc_uri_destroy(uri); return resolver; } diff --git a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc index d10468a2092..9be8df129a1 100644 --- a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc @@ -280,12 +280,16 @@ static void start_test_under_work_serializer(void* arg) { res_cb_arg->result_handler = new ResultHandler(); grpc_core::ResolverFactory* factory = grpc_core::ResolverRegistry::LookupResolverFactory("dns"); - grpc_uri* uri = grpc_uri_parse(res_cb_arg->uri_str, false); + absl::StatusOr uri = + grpc_core::URI::Parse(res_cb_arg->uri_str); gpr_log(GPR_DEBUG, "test: '%s' should be valid for '%s'", res_cb_arg->uri_str, factory->scheme()); - GPR_ASSERT(uri != nullptr); + if (!uri.ok()) { + gpr_log(GPR_ERROR, uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_core::ResolverArgs args; - args.uri = uri; + args.uri = std::move(*uri); args.work_serializer = *g_work_serializer; args.result_handler = std::unique_ptr( res_cb_arg->result_handler); @@ -298,7 +302,6 @@ static void start_test_under_work_serializer(void* arg) { args.args = &cooldown_args; res_cb_arg->resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(res_cb_arg->resolver != nullptr); - grpc_uri_destroy(uri); // First resolution, would incur in system-level resolution. res_cb_arg->result_handler->SetCallback(on_first_resolution, res_cb_arg); res_cb_arg->resolver->StartLocked(); diff --git a/test/core/client_channel/resolvers/dns_resolver_test.cc b/test/core/client_channel/resolvers/dns_resolver_test.cc index 3b91dc1cbe3..eac3286339f 100644 --- a/test/core/client_channel/resolvers/dns_resolver_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_test.cc @@ -40,16 +40,18 @@ static void test_succeeds(grpc_core::ResolverFactory* factory, gpr_log(GPR_DEBUG, "test: '%s' should be valid for '%s'", string, factory->scheme()); grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(string, false); - GPR_ASSERT(uri); + absl::StatusOr uri = grpc_core::URI::Parse(string); + if (!uri.ok()) { + gpr_log(GPR_ERROR, uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_core::ResolverArgs args; - args.uri = uri; + args.uri = std::move(*uri); args.work_serializer = *g_work_serializer; args.result_handler = absl::make_unique(); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(resolver != nullptr); - grpc_uri_destroy(uri); } static void test_fails(grpc_core::ResolverFactory* factory, @@ -57,16 +59,18 @@ static void test_fails(grpc_core::ResolverFactory* factory, gpr_log(GPR_DEBUG, "test: '%s' should be invalid for '%s'", string, factory->scheme()); grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(string, false); - GPR_ASSERT(uri); + absl::StatusOr uri = grpc_core::URI::Parse(string); + if (!uri.ok()) { + gpr_log(GPR_ERROR, uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_core::ResolverArgs args; - args.uri = uri; + args.uri = std::move(*uri); args.work_serializer = *g_work_serializer; args.result_handler = absl::make_unique(); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(resolver == nullptr); - grpc_uri_destroy(uri); } int main(int argc, char** argv) { diff --git a/test/core/client_channel/resolvers/fake_resolver_test.cc b/test/core/client_channel/resolvers/fake_resolver_test.cc index c5cdf4b63e5..5dde6bea07b 100644 --- a/test/core/client_channel/resolvers/fake_resolver_test.cc +++ b/test/core/client_channel/resolvers/fake_resolver_test.cc @@ -93,14 +93,14 @@ static grpc_core::Resolver::Result create_new_resolver_result() { for (size_t i = 0; i < num_addresses; ++i) { std::string uri_string = absl::StrFormat("ipv4:127.0.0.1:100%" PRIuPTR, test_counter * num_addresses + i); - grpc_uri* uri = grpc_uri_parse(uri_string.c_str(), true); + absl::StatusOr uri = grpc_core::URI::Parse(uri_string); + GPR_ASSERT(uri.ok()); grpc_resolved_address address; - GPR_ASSERT(grpc_parse_uri(uri, &address)); + GPR_ASSERT(grpc_parse_uri(*uri, &address)); absl::InlinedVector args_to_add; result.addresses.emplace_back( address.addr, address.len, grpc_channel_args_copy_and_add(nullptr, nullptr, 0)); - grpc_uri_destroy(uri); } ++test_counter; return result; diff --git a/test/core/client_channel/resolvers/sockaddr_resolver_test.cc b/test/core/client_channel/resolvers/sockaddr_resolver_test.cc index ff56963ec84..9dfd883ef77 100644 --- a/test/core/client_channel/resolvers/sockaddr_resolver_test.cc +++ b/test/core/client_channel/resolvers/sockaddr_resolver_test.cc @@ -42,16 +42,18 @@ static void test_succeeds(grpc_core::ResolverFactory* factory, gpr_log(GPR_DEBUG, "test: '%s' should be valid for '%s'", string, factory->scheme()); grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(string, false); - GPR_ASSERT(uri); + absl::StatusOr uri = grpc_core::URI::Parse(string); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_core::ResolverArgs args; - args.uri = uri; + args.uri = std::move(*uri); args.work_serializer = *g_work_serializer; args.result_handler = absl::make_unique(); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(resolver != nullptr); - grpc_uri_destroy(uri); resolver->StartLocked(); /* Flush ExecCtx to avoid stack-use-after-scope on on_res_arg which is * accessed in the closure on_resolution_cb */ @@ -63,16 +65,18 @@ static void test_fails(grpc_core::ResolverFactory* factory, gpr_log(GPR_DEBUG, "test: '%s' should be invalid for '%s'", string, factory->scheme()); grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(string, false); - GPR_ASSERT(uri); + absl::StatusOr uri = grpc_core::URI::Parse(string); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_core::ResolverArgs args; - args.uri = uri; + args.uri = std::move(*uri); args.work_serializer = *g_work_serializer; args.result_handler = absl::make_unique(); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(resolver == nullptr); - grpc_uri_destroy(uri); } int main(int argc, char** argv) { diff --git a/test/core/client_channel/retry_throttle_test.cc b/test/core/client_channel/retry_throttle_test.cc index 793e86dc065..1ae86843689 100644 --- a/test/core/client_channel/retry_throttle_test.cc +++ b/test/core/client_channel/retry_throttle_test.cc @@ -98,7 +98,7 @@ TEST(ServerRetryThrottleData, Replacement) { TEST(ServerRetryThrottleMap, Replacement) { ServerRetryThrottleMap::Init(); - const char kServerName[] = "server_name"; + const std::string kServerName = "server_name"; // Create old throttle data. // Max token count is 4, so threshold for retrying is 2. // Token count starts at 4. diff --git a/test/core/iomgr/parse_address_test.cc b/test/core/iomgr/parse_address_test.cc index 1f9db4b6b1f..d61ff5d5984 100644 --- a/test/core/iomgr/parse_address_test.cc +++ b/test/core/iomgr/parse_address_test.cc @@ -36,32 +36,36 @@ static void test_grpc_parse_unix(const char* uri_text, const char* pathname) { grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(uri_text, false); + absl::StatusOr uri = grpc_core::URI::Parse(uri_text); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_resolved_address addr; - GPR_ASSERT(1 == grpc_parse_uri(uri, &addr)); + GPR_ASSERT(1 == grpc_parse_uri(*uri, &addr)); struct sockaddr_un* addr_un = reinterpret_cast(addr.addr); GPR_ASSERT(AF_UNIX == addr_un->sun_family); GPR_ASSERT(0 == strcmp(addr_un->sun_path, pathname)); - - grpc_uri_destroy(uri); } static void test_grpc_parse_unix_abstract(const char* uri_text, const char* pathname) { grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(uri_text, false); + absl::StatusOr uri = grpc_core::URI::Parse(uri_text); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_resolved_address addr; - GPR_ASSERT(1 == grpc_parse_uri(uri, &addr)); + GPR_ASSERT(1 == grpc_parse_uri(*uri, &addr)); struct sockaddr_un* addr_un = reinterpret_cast(addr.addr); GPR_ASSERT(AF_UNIX == addr_un->sun_family); GPR_ASSERT('\0' == addr_un->sun_path[0]); GPR_ASSERT(0 == strncmp(addr_un->sun_path + 1, pathname, strlen(pathname))); - - grpc_uri_destroy(uri); } #else /* GRPC_HAVE_UNIX_SOCKET */ @@ -75,29 +79,34 @@ static void test_grpc_parse_unix_abstract(const char* uri_text, static void test_grpc_parse_ipv4(const char* uri_text, const char* host, unsigned short port) { grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(uri_text, false); + absl::StatusOr uri = grpc_core::URI::Parse(uri_text); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_resolved_address addr; char ntop_buf[GRPC_INET_ADDRSTRLEN]; - GPR_ASSERT(1 == grpc_parse_ipv4(uri, &addr)); + GPR_ASSERT(1 == grpc_parse_ipv4(*uri, &addr)); grpc_sockaddr_in* addr_in = reinterpret_cast(addr.addr); GPR_ASSERT(GRPC_AF_INET == addr_in->sin_family); GPR_ASSERT(nullptr != grpc_inet_ntop(GRPC_AF_INET, &addr_in->sin_addr, ntop_buf, sizeof(ntop_buf))); GPR_ASSERT(0 == strcmp(ntop_buf, host)); GPR_ASSERT(grpc_ntohs(addr_in->sin_port) == port); - - grpc_uri_destroy(uri); } static void test_grpc_parse_ipv6(const char* uri_text, const char* host, unsigned short port, uint32_t scope_id) { grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(uri_text, false); + absl::StatusOr uri = grpc_core::URI::Parse(uri_text); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_resolved_address addr; char ntop_buf[GRPC_INET6_ADDRSTRLEN]; - - GPR_ASSERT(1 == grpc_parse_ipv6(uri, &addr)); + GPR_ASSERT(1 == grpc_parse_ipv6(*uri, &addr)); grpc_sockaddr_in6* addr_in6 = reinterpret_cast(addr.addr); GPR_ASSERT(GRPC_AF_INET6 == addr_in6->sin6_family); GPR_ASSERT(nullptr != grpc_inet_ntop(GRPC_AF_INET6, &addr_in6->sin6_addr, @@ -105,17 +114,18 @@ static void test_grpc_parse_ipv6(const char* uri_text, const char* host, GPR_ASSERT(0 == strcmp(ntop_buf, host)); GPR_ASSERT(grpc_ntohs(addr_in6->sin6_port) == port); GPR_ASSERT(addr_in6->sin6_scope_id == scope_id); - - 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, false); + absl::StatusOr uri = grpc_core::URI::Parse(uri_text); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_resolved_address addr; - GPR_ASSERT(!grpc_parse_ipv6(uri, &addr)); - grpc_uri_destroy(uri); + GPR_ASSERT(!grpc_parse_ipv6(*uri, &addr)); } int main(int argc, char** argv) { @@ -131,7 +141,7 @@ int main(int argc, char** argv) { /* Address length greater than GRPC_INET6_ADDRSTRLEN */ test_grpc_parse_ipv6_invalid( "ipv6:WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW45%" - "v6:45%x$1*"); + "25v6:45%25x$1*"); grpc_shutdown(); } diff --git a/test/core/iomgr/parse_address_with_named_scope_id_test.cc b/test/core/iomgr/parse_address_with_named_scope_id_test.cc index 1fcc60b1e98..e7b987845e9 100644 --- a/test/core/iomgr/parse_address_with_named_scope_id_test.cc +++ b/test/core/iomgr/parse_address_with_named_scope_id_test.cc @@ -41,9 +41,13 @@ static void test_grpc_parse_ipv6_parity_with_getaddrinfo( const char* target, const struct sockaddr_in6 result_from_getaddrinfo) { // Get the sockaddr that gRPC's ipv6 resolver resolves this too. grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(target, false); + absl::StatusOr uri = grpc_core::URI::Parse(target); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } grpc_resolved_address addr; - GPR_ASSERT(1 == grpc_parse_ipv6(uri, &addr)); + GPR_ASSERT(1 == grpc_parse_ipv6(*uri, &addr)); grpc_sockaddr_in6* result_from_grpc_parser = reinterpret_cast(addr.addr); // Compare the sockaddr returned from gRPC's ipv6 resolver with that returned @@ -57,14 +61,17 @@ static void test_grpc_parse_ipv6_parity_with_getaddrinfo( GPR_ASSERT(result_from_grpc_parser->sin6_scope_id != 0); // TODO(unknown): compare sin6_flow_info fields? parse_ipv6 zero's this field // as is. Cleanup - grpc_uri_destroy(uri); } struct sockaddr_in6 resolve_with_gettaddrinfo(const char* uri_text) { - grpc_uri* uri = grpc_uri_parse(uri_text, false); + absl::StatusOr uri = grpc_core::URI::Parse(uri_text); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } std::string host; std::string port; - grpc_core::SplitHostPort(uri->path, &host, &port); + grpc_core::SplitHostPort(uri->path(), &host, &port); struct addrinfo hints; memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_INET6; @@ -88,7 +95,6 @@ struct sockaddr_in6 resolve_with_gettaddrinfo(const char* uri_text) { *reinterpret_cast(result->ai_addr); // Cleanup freeaddrinfo(result); - grpc_uri_destroy(uri); return out; } diff --git a/test/core/iomgr/stranded_event_test.cc b/test/core/iomgr/stranded_event_test.cc index 85dd39e0a08..c8bb6a80376 100644 --- a/test/core/iomgr/stranded_event_test.cc +++ b/test/core/iomgr/stranded_event_test.cc @@ -296,15 +296,15 @@ grpc_core::Resolver::Result BuildResolverResponse( const std::vector& addresses) { grpc_core::Resolver::Result result; for (const auto& address_str : addresses) { - grpc_uri* uri = grpc_uri_parse(address_str.c_str(), true); - if (uri == nullptr) { - gpr_log(GPR_ERROR, "Failed to parse uri:%s", address_str.c_str()); - GPR_ASSERT(0); + absl::StatusOr uri = grpc_core::URI::Parse(address_str); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "Failed to parse. Error: %s", + uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); } grpc_resolved_address address; - GPR_ASSERT(grpc_parse_uri(uri, &address)); + GPR_ASSERT(grpc_parse_uri(*uri, &address)); result.addresses.emplace_back(address.addr, address.len, nullptr); - grpc_uri_destroy(uri); } return result; } diff --git a/test/core/security/credentials_test.cc b/test/core/security/credentials_test.cc index 35f9a7db472..04a118a3c02 100644 --- a/test/core/security/credentials_test.cc +++ b/test/core/security/credentials_test.cc @@ -26,6 +26,7 @@ #include +#include "absl/strings/match.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/str_replace.h" @@ -878,17 +879,14 @@ static void test_valid_sts_creds_options(void) { nullptr, // actor_token_path nullptr // actor_token_type }; - grpc_uri* sts_url; - grpc_error* error = - grpc_core::ValidateStsCredentialsOptions(&valid_options, &sts_url); - GPR_ASSERT(error == GRPC_ERROR_NONE); - GPR_ASSERT(sts_url != nullptr); + absl::StatusOr sts_url = + grpc_core::ValidateStsCredentialsOptions(&valid_options); + GPR_ASSERT(sts_url.ok()); absl::string_view host; absl::string_view port; - GPR_ASSERT(grpc_core::SplitHostPort(sts_url->authority, &host, &port)); + GPR_ASSERT(grpc_core::SplitHostPort(sts_url->authority(), &host, &port)); GPR_ASSERT(host == "foo.com"); GPR_ASSERT(port == "5555"); - grpc_uri_destroy(sts_url); } static void test_invalid_sts_creds_options(void) { @@ -903,12 +901,9 @@ static void test_invalid_sts_creds_options(void) { nullptr, // actor_token_path nullptr // actor_token_type }; - grpc_uri* url_should_be_null; - grpc_error* error = grpc_core::ValidateStsCredentialsOptions( - &invalid_options, &url_should_be_null); - GPR_ASSERT(error != GRPC_ERROR_NONE); - GRPC_ERROR_UNREF(error); - GPR_ASSERT(url_should_be_null == nullptr); + absl::StatusOr url_should_be_invalid = + grpc_core::ValidateStsCredentialsOptions(&invalid_options); + GPR_ASSERT(!url_should_be_invalid.ok()); invalid_options = { test_sts_endpoint_url, // sts_endpoint_url @@ -921,11 +916,9 @@ static void test_invalid_sts_creds_options(void) { nullptr, // actor_token_path nullptr // actor_token_type }; - error = grpc_core::ValidateStsCredentialsOptions(&invalid_options, - &url_should_be_null); - GPR_ASSERT(error != GRPC_ERROR_NONE); - GRPC_ERROR_UNREF(error); - GPR_ASSERT(url_should_be_null == nullptr); + url_should_be_invalid = + grpc_core::ValidateStsCredentialsOptions(&invalid_options); + GPR_ASSERT(!url_should_be_invalid.ok()); invalid_options = { nullptr, // sts_endpoint_url (Required) @@ -938,11 +931,9 @@ static void test_invalid_sts_creds_options(void) { nullptr, // actor_token_path nullptr // actor_token_type }; - error = grpc_core::ValidateStsCredentialsOptions(&invalid_options, - &url_should_be_null); - GPR_ASSERT(error != GRPC_ERROR_NONE); - GRPC_ERROR_UNREF(error); - GPR_ASSERT(url_should_be_null == nullptr); + url_should_be_invalid = + grpc_core::ValidateStsCredentialsOptions(&invalid_options); + GPR_ASSERT(!url_should_be_invalid.ok()); invalid_options = { "not_a_valid_uri", // sts_endpoint_url @@ -955,11 +946,9 @@ static void test_invalid_sts_creds_options(void) { nullptr, // actor_token_path nullptr // actor_token_type }; - error = grpc_core::ValidateStsCredentialsOptions(&invalid_options, - &url_should_be_null); - GPR_ASSERT(error != GRPC_ERROR_NONE); - GRPC_ERROR_UNREF(error); - GPR_ASSERT(url_should_be_null == nullptr); + url_should_be_invalid = + grpc_core::ValidateStsCredentialsOptions(&invalid_options); + GPR_ASSERT(!url_should_be_invalid.ok()); invalid_options = { "ftp://ftp.is.not.a.valid.scheme/bar", // sts_endpoint_url @@ -972,11 +961,21 @@ static void test_invalid_sts_creds_options(void) { nullptr, // actor_token_path nullptr // actor_token_type }; - error = grpc_core::ValidateStsCredentialsOptions(&invalid_options, - &url_should_be_null); - GPR_ASSERT(error != GRPC_ERROR_NONE); - GRPC_ERROR_UNREF(error); - GPR_ASSERT(url_should_be_null == nullptr); + url_should_be_invalid = + grpc_core::ValidateStsCredentialsOptions(&invalid_options); + GPR_ASSERT(!url_should_be_invalid.ok()); +} + +static void assert_query_parameters(const grpc_core::URI& uri, + absl::string_view expected_key, + absl::string_view expected_val) { + const auto it = uri.query_parameter_map().find(expected_key); + GPR_ASSERT(it != uri.query_parameter_map().end()); + if (it->second != expected_val) { + gpr_log(GPR_ERROR, "%s!=%s", std::string(it->second).c_str(), + std::string(expected_val).c_str()); + } + GPR_ASSERT(it->second == expected_val); } static void validate_sts_token_http_request(const grpc_httpcli_request* request, @@ -988,26 +987,29 @@ static void validate_sts_token_http_request(const grpc_httpcli_request* request, GPR_ASSERT(request->handshaker == &grpc_httpcli_ssl); std::string get_url_equivalent = absl::StrFormat("%s?%s", test_sts_endpoint_url, body); - grpc_uri* url = grpc_uri_parse(get_url_equivalent.c_str(), false); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(url, "resource"), "resource") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(url, "audience"), "audience") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(url, "scope"), "scope") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(url, "requested_token_type"), - "requested_token_type") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(url, "subject_token"), - test_signed_jwt) == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(url, "subject_token_type"), - test_signed_jwt_token_type) == 0); + absl::StatusOr url = + grpc_core::URI::Parse(get_url_equivalent); + if (!url.ok()) { + gpr_log(GPR_ERROR, "%s", url.status().ToString().c_str()); + GPR_ASSERT(url.ok()); + } + assert_query_parameters(*url, "resource", "resource"); + assert_query_parameters(*url, "audience", "audience"); + assert_query_parameters(*url, "scope", "scope"); + assert_query_parameters(*url, "requested_token_type", "requested_token_type"); + assert_query_parameters(*url, "subject_token", test_signed_jwt); + assert_query_parameters(*url, "subject_token_type", + test_signed_jwt_token_type); if (expect_actor_token) { - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(url, "actor_token"), - test_signed_jwt2) == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(url, "actor_token_type"), - test_signed_jwt_token_type2) == 0); + assert_query_parameters(*url, "actor_token", test_signed_jwt2); + assert_query_parameters(*url, "actor_token_type", + test_signed_jwt_token_type2); } else { - GPR_ASSERT(grpc_uri_get_query_arg(url, "actor_token") == nullptr); - GPR_ASSERT(grpc_uri_get_query_arg(url, "actor_token_type") == nullptr); + GPR_ASSERT(url->query_parameter_map().find("actor_token") == + url->query_parameter_map().end()); + GPR_ASSERT(url->query_parameter_map().find("actor_token_type") == + url->query_parameter_map().end()); } - grpc_uri_destroy(url); // Check the rest of the request. GPR_ASSERT(strcmp(request->host, "foo.com:5555") == 0); @@ -1982,19 +1984,22 @@ static void validate_external_account_creds_token_exchage_request( GPR_ASSERT(request->handshaker == &grpc_httpcli_ssl); std::string get_url_equivalent = absl::StrFormat("%s?%s", "https://foo.com:5555/token", body); - grpc_uri* uri = grpc_uri_parse(get_url_equivalent.c_str(), false); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "audience"), "audience") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "grant_type"), - "urn:ietf:params:oauth:grant-type:token-exchange") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "requested_token_type"), - "urn:ietf:params:oauth:token-type:access_token") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "subject_token"), - "test_subject_token") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "subject_token_type"), - "subject_token_type") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "scope"), - "https://www.googleapis.com/auth/cloud-platform") == 0); - grpc_uri_destroy(uri); + absl::StatusOr uri = + grpc_core::URI::Parse(get_url_equivalent); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } + assert_query_parameters(*uri, "audience", "audience"); + assert_query_parameters(*uri, "grant_type", + "urn:ietf:params:oauth:grant-type:token-exchange"); + assert_query_parameters(*uri, "requested_token_type", + "urn:ietf:params:oauth:token-type:access_token"); + assert_query_parameters(*uri, "subject_token", "test_subject_token"); + assert_query_parameters(*uri, "subject_token_type", "subject_token_type"); + assert_query_parameters(*uri, "scope", + "https://www.googleapis.com/auth/cloud-platform"); + // Check the rest of the request. GPR_ASSERT(strcmp(request->host, "foo.com:5555") == 0); GPR_ASSERT(strcmp(request->http.path, "/token") == 0); @@ -2095,17 +2100,17 @@ static void validate_aws_external_account_creds_token_exchage_request( GPR_ASSERT(request->handshaker == &grpc_httpcli_ssl); std::string get_url_equivalent = absl::StrFormat("%s?%s", "https://foo.com:5555/token", body); - grpc_uri* uri = grpc_uri_parse(get_url_equivalent.c_str(), false); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "audience"), "audience") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "grant_type"), - "urn:ietf:params:oauth:grant-type:token-exchange") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "requested_token_type"), - "urn:ietf:params:oauth:token-type:access_token") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "subject_token_type"), - "subject_token_type") == 0); - GPR_ASSERT(strcmp(grpc_uri_get_query_arg(uri, "scope"), - "https://www.googleapis.com/auth/cloud-platform") == 0); - grpc_uri_destroy(uri); + absl::StatusOr uri = + grpc_core::URI::Parse(get_url_equivalent); + GPR_ASSERT(uri.ok()); + assert_query_parameters(*uri, "audience", "audience"); + assert_query_parameters(*uri, "grant_type", + "urn:ietf:params:oauth:grant-type:token-exchange"); + assert_query_parameters(*uri, "requested_token_type", + "urn:ietf:params:oauth:token-type:access_token"); + assert_query_parameters(*uri, "subject_token_type", "subject_token_type"); + assert_query_parameters(*uri, "scope", + "https://www.googleapis.com/auth/cloud-platform"); // Check the rest of the request. GPR_ASSERT(strcmp(request->host, "foo.com:5555") == 0); GPR_ASSERT(strcmp(request->http.path, "/token") == 0); @@ -2438,12 +2443,12 @@ test_url_external_account_creds_failure_invalid_credential_source_url(void) { auto creds = grpc_core::UrlExternalAccountCredentials::Create(options, {}, &error); GPR_ASSERT(creds == nullptr); - grpc_slice expected_error_slice = - grpc_slice_from_static_string("Invalid credential source url."); grpc_slice actual_error_slice; GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &actual_error_slice)); - GPR_ASSERT(grpc_slice_cmp(expected_error_slice, actual_error_slice) == 0); + absl::string_view actual_error = + grpc_core::StringViewFromSlice(actual_error_slice); + GPR_ASSERT(absl::StartsWith(actual_error, "Invalid credential source url.")); GRPC_ERROR_UNREF(error); } diff --git a/test/core/transport/chttp2/too_many_pings_test.cc b/test/core/transport/chttp2/too_many_pings_test.cc index 08d635a1d69..abb3ae5768a 100644 --- a/test/core/transport/chttp2/too_many_pings_test.cc +++ b/test/core/transport/chttp2/too_many_pings_test.cc @@ -362,15 +362,15 @@ grpc_core::Resolver::Result BuildResolverResult( const std::vector& addresses) { grpc_core::Resolver::Result result; for (const auto& address_str : addresses) { - grpc_uri* uri = grpc_uri_parse(address_str.c_str(), true); - if (uri == nullptr) { - gpr_log(GPR_ERROR, "Failed to parse uri:%s", address_str.c_str()); - GPR_ASSERT(0); + absl::StatusOr uri = grpc_core::URI::Parse(address_str); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "Failed to parse uri. Error: %s", + uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); } grpc_resolved_address address; - GPR_ASSERT(grpc_parse_uri(uri, &address)); + GPR_ASSERT(grpc_parse_uri(*uri, &address)); result.addresses.emplace_back(address.addr, address.len, nullptr); - grpc_uri_destroy(uri); } return result; } diff --git a/test/core/uri/uri_fuzzer_test.cc b/test/core/uri/uri_fuzzer_test.cc index 9124a310f92..de6e8dfc750 100644 --- a/test/core/uri/uri_fuzzer_test.cc +++ b/test/core/uri/uri_fuzzer_test.cc @@ -38,11 +38,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { { grpc_core::ExecCtx exec_ctx; - grpc_uri* x; - if ((x = grpc_uri_parse(s, true))) { - grpc_uri_destroy(x); - } - + (void)grpc_core::URI::Parse(s); gpr_free(s); } diff --git a/test/core/uri/uri_parser_test.cc b/test/core/uri/uri_parser_test.cc index 96591609bac..70f44ffe48d 100644 --- a/test/core/uri/uri_parser_test.cc +++ b/test/core/uri/uri_parser_test.cc @@ -16,138 +16,171 @@ * */ +// TODO(hork): rewrite with googletest + #include "src/core/lib/uri/uri_parser.h" -#include +#include "absl/strings/str_join.h" +#include "absl/strings/str_split.h" #include #include -#include "src/core/lib/iomgr/exec_ctx.h" #include "test/core/util/test_config.h" -static void test_succeeds(const char* uri_text, const char* scheme, - const char* authority, const char* path, - const char* query, const char* fragment) { - grpc_core::ExecCtx exec_ctx; - grpc_uri* uri = grpc_uri_parse(uri_text, false); - GPR_ASSERT(uri); - GPR_ASSERT(0 == strcmp(scheme, uri->scheme)); - GPR_ASSERT(0 == strcmp(authority, uri->authority)); - GPR_ASSERT(0 == strcmp(path, uri->path)); - GPR_ASSERT(0 == strcmp(query, uri->query)); - GPR_ASSERT(0 == strcmp(fragment, uri->fragment)); - - grpc_uri_destroy(uri); +static void test_succeeds( + absl::string_view uri_text, absl::string_view scheme, + absl::string_view authority, absl::string_view path, + const std::map& query_param_map, + const std::vector query_param_pairs, + absl::string_view fragment) { + absl::StatusOr uri = grpc_core::URI::Parse(uri_text); + GPR_ASSERT(uri.ok()); + GPR_ASSERT(scheme == uri->scheme()); + GPR_ASSERT(authority == uri->authority()); + GPR_ASSERT(path == uri->path()); + // query param map + GPR_ASSERT(uri->query_parameter_map().size() == query_param_map.size()); + for (const auto& expected_kv : query_param_map) { + const auto it = uri->query_parameter_map().find(expected_kv.first); + GPR_ASSERT(it != uri->query_parameter_map().end()); + GPR_ASSERT(it->second == expected_kv.second); + } + // query param pairs + GPR_ASSERT(uri->query_parameter_pairs() == query_param_pairs); + GPR_ASSERT(fragment == uri->fragment()); } -static void test_fails(const char* uri_text) { - grpc_core::ExecCtx exec_ctx; - GPR_ASSERT(nullptr == grpc_uri_parse(uri_text, 0)); +static void test_fails(absl::string_view uri_text) { + absl::StatusOr uri = grpc_core::URI::Parse(uri_text); + GPR_ASSERT(!uri.ok()); } -static void test_query_parts() { - { - grpc_core::ExecCtx exec_ctx; - const char* uri_text = "http://foo/path?a&b=B&c=&#frag"; - grpc_uri* uri = grpc_uri_parse(uri_text, false); - GPR_ASSERT(uri); - - GPR_ASSERT(0 == strcmp("http", uri->scheme)); - GPR_ASSERT(0 == strcmp("foo", uri->authority)); - GPR_ASSERT(0 == strcmp("/path", uri->path)); - GPR_ASSERT(0 == strcmp("a&b=B&c=&", uri->query)); - GPR_ASSERT(4 == uri->num_query_parts); - - GPR_ASSERT(0 == strcmp("a", uri->query_parts[0])); - GPR_ASSERT(nullptr == uri->query_parts_values[0]); - - GPR_ASSERT(0 == strcmp("b", uri->query_parts[1])); - GPR_ASSERT(0 == strcmp("B", uri->query_parts_values[1])); - - GPR_ASSERT(0 == strcmp("c", uri->query_parts[2])); - GPR_ASSERT(0 == strcmp("", uri->query_parts_values[2])); - - GPR_ASSERT(0 == strcmp("", uri->query_parts[3])); - GPR_ASSERT(nullptr == uri->query_parts_values[3]); - - GPR_ASSERT(nullptr == grpc_uri_get_query_arg(uri, "a")); - GPR_ASSERT(0 == strcmp("B", grpc_uri_get_query_arg(uri, "b"))); - GPR_ASSERT(0 == strcmp("", grpc_uri_get_query_arg(uri, "c"))); - GPR_ASSERT(nullptr == grpc_uri_get_query_arg(uri, "")); +static void test_query_param_map() { + test_succeeds("http://localhost:8080/whatzit?mi_casa=su_casa", "http", + "localhost:8080", "/whatzit", {{"mi_casa", "su_casa"}}, + {{"mi_casa", "su_casa"}}, ""); + test_succeeds("http://localhost:8080/whatzit?1=2#buckle/my/shoe", "http", + "localhost:8080", "/whatzit", {{"1", "2"}}, {{"1", "2"}}, + "buckle/my/shoe"); + test_succeeds( + "http://localhost:8080/?too=many=equals&are=present=here#fragged", "http", + "localhost:8080", "/", {{"are", "present=here"}, {"too", "many=equals"}}, + {{"too", "many=equals"}, {"are", "present=here"}}, "fragged"); + test_succeeds("http://foo/path?a&b=B&c=&#frag", "http", "foo", "/path", + {{"c", ""}, {"a", ""}, {"b", "B"}}, + {{"a", ""}, {"b", "B"}, {"c", ""}}, "frag"); + test_succeeds("http://auth/path?foo=bar=baz&foobar===", "http", "auth", + "/path", {{"foo", "bar=baz"}, {"foobar", "=="}}, + {{"foo", "bar=baz"}, {"foobar", "=="}}, ""); +} - GPR_ASSERT(0 == strcmp("frag", uri->fragment)); +static void test_repeated_query_param_pairs() { + absl::StatusOr uri = + grpc_core::URI::Parse("http://foo/path?a=2&a=1&a=3"); + GPR_ASSERT(uri.ok()); + GPR_ASSERT(uri->query_parameter_map().size() == 1); + GPR_ASSERT(uri->query_parameter_map().find("a")->second == "3"); + std::vector expected( + {{"a", "2"}, {"a", "1"}, {"a", "3"}}); + GPR_ASSERT(uri->query_parameter_pairs() == expected); +} - grpc_uri_destroy(uri); - } +static void test_query_param_validity_after_move() { + grpc_core::URI uri_copy; { - /* test the current behavior of multiple query part values */ - grpc_core::ExecCtx exec_ctx; - const char* uri_text = "http://auth/path?foo=bar=baz&foobar=="; - grpc_uri* uri = grpc_uri_parse(uri_text, false); - GPR_ASSERT(uri); - - GPR_ASSERT(0 == strcmp("http", uri->scheme)); - GPR_ASSERT(0 == strcmp("auth", uri->authority)); - GPR_ASSERT(0 == strcmp("/path", uri->path)); - GPR_ASSERT(0 == strcmp("foo=bar=baz&foobar==", uri->query)); - GPR_ASSERT(2 == uri->num_query_parts); - - GPR_ASSERT(0 == strcmp("bar", grpc_uri_get_query_arg(uri, "foo"))); - GPR_ASSERT(0 == strcmp("", grpc_uri_get_query_arg(uri, "foobar"))); - - grpc_uri_destroy(uri); + absl::StatusOr uri = + grpc_core::URI::Parse("http://foo/path?a=2&b=1&c=3"); + GPR_ASSERT(uri.ok()); + uri_copy = std::move(*uri); } - { - /* empty query */ - grpc_core::ExecCtx exec_ctx; - const char* uri_text = "http://foo/path"; - grpc_uri* uri = grpc_uri_parse(uri_text, false); - GPR_ASSERT(uri); - - GPR_ASSERT(0 == strcmp("http", uri->scheme)); - GPR_ASSERT(0 == strcmp("foo", uri->authority)); - GPR_ASSERT(0 == strcmp("/path", uri->path)); - GPR_ASSERT(0 == strcmp("", uri->query)); - GPR_ASSERT(0 == uri->num_query_parts); - GPR_ASSERT(nullptr == uri->query_parts); - GPR_ASSERT(nullptr == uri->query_parts_values); - GPR_ASSERT(0 == strcmp("", uri->fragment)); + GPR_ASSERT(uri_copy.query_parameter_map().find("a")->second == "2"); +} - grpc_uri_destroy(uri); +static void test_query_param_validity_after_copy() { + // Since the query parameter map points to objects stored in the param pair + // vector, this test checks that the param map pointers remain valid after + // a copy. Ideally {a,m}san will catch this if there's a problem. + // testing copy operator=: + grpc_core::URI uri_copy; + { + absl::StatusOr del_uri = + grpc_core::URI::Parse("http://foo/path?a=2&b=1&c=3"); + GPR_ASSERT(del_uri.ok()); + uri_copy = *del_uri; } + GPR_ASSERT(uri_copy.query_parameter_map().find("a")->second == "2"); + // testing copy constructor: + grpc_core::URI* del_uri2 = new grpc_core::URI(uri_copy); + grpc_core::URI uri_copy2(*del_uri2); + delete del_uri2; + GPR_ASSERT(uri_copy2.query_parameter_map().find("a")->second == "2"); } int main(int argc, char** argv) { grpc::testing::TestEnvironment env(argc, argv); grpc_init(); - test_succeeds("http://www.google.com", "http", "www.google.com", "", "", ""); - test_succeeds("dns:///foo", "dns", "", "/foo", "", ""); - test_succeeds("http://www.google.com:90", "http", "www.google.com:90", "", "", + test_succeeds("http://www.google.com", "http", "www.google.com", "", {}, {}, ""); - test_succeeds("a192.4-df:foo.coom", "a192.4-df", "", "foo.coom", "", ""); - test_succeeds("a+b:foo.coom", "a+b", "", "foo.coom", "", ""); + test_succeeds("dns:///foo", "dns", "", "/foo", {}, {}, ""); + test_succeeds("http://www.google.com:90", "http", "www.google.com:90", "", {}, + {}, ""); + test_succeeds("a192.4-df:foo.coom", "a192.4-df", "", "foo.coom", {}, {}, ""); + test_succeeds("a+b:foo.coom", "a+b", "", "foo.coom", {}, {}, ""); test_succeeds("zookeeper://127.0.0.1:2181/foo/bar", "zookeeper", - "127.0.0.1:2181", "/foo/bar", "", ""); + "127.0.0.1:2181", "/foo/bar", {}, {}, ""); test_succeeds("http://www.google.com?yay-i'm-using-queries", "http", - "www.google.com", "", "yay-i'm-using-queries", ""); - test_succeeds("dns:foo.com#fragment-all-the-things", "dns", "", "foo.com", "", - "fragment-all-the-things"); - test_succeeds("http:?legit", "http", "", "", "legit", ""); - test_succeeds("unix:#this-is-ok-too", "unix", "", "", "", "this-is-ok-too"); - test_succeeds("http:?legit#twice", "http", "", "", "legit", "twice"); - test_succeeds("http://foo?bar#lol?", "http", "foo", "", "bar", "lol?"); - test_succeeds("http://foo?bar#lol?/", "http", "foo", "", "bar", "lol?/"); + "www.google.com", "", {{"yay-i'm-using-queries", ""}}, + {{"yay-i'm-using-queries", ""}}, ""); + test_succeeds("dns:foo.com#fragment-all-the-things", "dns", "", "foo.com", {}, + {}, "fragment-all-the-things"); + test_succeeds("http:?legit", "http", "", "", {{"legit", ""}}, {{"legit", ""}}, + ""); + test_succeeds("unix:#this-is-ok-too", "unix", "", "", {}, {}, + "this-is-ok-too"); + test_succeeds("http:?legit#twice", "http", "", "", {{"legit", ""}}, + {{"legit", ""}}, "twice"); + test_succeeds("http://foo?bar#lol?", "http", "foo", "", {{"bar", ""}}, + {{"bar", ""}}, "lol?"); + test_succeeds("http://foo?bar#lol?/", "http", "foo", "", {{"bar", ""}}, + {{"bar", ""}}, "lol?/"); test_succeeds("ipv6:[2001:db8::1%252]:12345", "ipv6", "", - "[2001:db8::1%2]:12345", "", ""); - + "[2001:db8::1%2]:12345", {}, {}, ""); + test_succeeds("https://www.google.com/?a=1%26b%3D2&c=3", "https", + "www.google.com", "/", {{"c", "3"}, {"a", "1&b=2"}}, + {{"a", "1&b=2"}, {"c", "3"}}, ""); + // Artificial examples to show that embedded nulls are supported. + test_succeeds(std::string("unix-abstract:\0should-be-ok", 27), + "unix-abstract", "", std::string("\0should-be-ok", 13), {}, {}, + ""); + test_succeeds( + "https://foo.com:5555/v1/" + "token-exchange?subject_token=eyJhbGciO&subject_token_type=urn:ietf:" + "params:oauth:token-type:id_token", + "https", "foo.com:5555", "/v1/token-exchange", + {{"subject_token", "eyJhbGciO"}, + {"subject_token_type", "urn:ietf:params:oauth:token-type:id_token"}}, + {{"subject_token", "eyJhbGciO"}, + {"subject_token_type", "urn:ietf:params:oauth:token-type:id_token"}}, + ""); + test_succeeds("http:?dangling-pct-%0", "http", "", "", + {{"dangling-pct-%0", ""}}, {{"dangling-pct-%0", ""}}, ""); + test_succeeds("unix-abstract:%00x", "unix-abstract", "", + std::string("\0x", 2), {}, {}, ""); + test_succeeds("x:y?%xx", "x", "", "y", {{"%xx", ""}}, {{"%xx", ""}}, ""); + test_succeeds("scheme:path//is/ok", "scheme", "", "path//is/ok", {}, {}, ""); + test_succeeds("fake:///", "fake", "", "/", {}, {}, ""); test_fails("xyz"); - test_fails("http:?dangling-pct-%0"); test_fails("http://foo?[bar]"); test_fails("http://foo?x[bar]"); test_fails("http://foo?bar#lol#"); - - test_query_parts(); + test_fails(""); + test_fails(":no_scheme"); + test_fails("0invalid_scheme:must_start/with?alpha"); + test_query_param_map(); + test_repeated_query_param_pairs(); + test_query_param_validity_after_move(); + test_query_param_validity_after_copy(); grpc_shutdown(); return 0; } diff --git a/test/cpp/client/client_channel_stress_test.cc b/test/cpp/client/client_channel_stress_test.cc index 8bbd786d64e..5b61d0e7dba 100644 --- a/test/cpp/client/client_channel_stress_test.cc +++ b/test/cpp/client/client_channel_stress_test.cc @@ -225,16 +225,15 @@ class ClientChannelStressTest { grpc_core::ServerAddressList addresses; for (const auto& addr : address_data) { std::string lb_uri_str = absl::StrCat("ipv4:127.0.0.1:", addr.port); - grpc_uri* lb_uri = grpc_uri_parse(lb_uri_str.c_str(), true); - GPR_ASSERT(lb_uri != nullptr); + absl::StatusOr lb_uri = grpc_core::URI::Parse(lb_uri_str); + GPR_ASSERT(lb_uri.ok()); grpc_resolved_address address; - GPR_ASSERT(grpc_parse_uri(lb_uri, &address)); + GPR_ASSERT(grpc_parse_uri(*lb_uri, &address)); grpc_arg arg = grpc_core::CreateAuthorityOverrideChannelArg( addr.balancer_name.c_str()); grpc_channel_args* args = grpc_channel_args_copy_and_add(nullptr, &arg, 1); addresses.emplace_back(address.addr, address.len, args); - grpc_uri_destroy(lb_uri); } return addresses; } diff --git a/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc b/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc index f8692fb68d4..535a56e84a0 100644 --- a/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc +++ b/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc @@ -60,12 +60,11 @@ void TryConnectAndDestroy() { // The precise behavior is dependant on the test runtime environment though, // since connect() attempts on this address may unfortunately result in // "network unreachable" errors in some test runtime environments. - const char* uri_str = "ipv6:[0100::1234]:443"; - grpc_uri* lb_uri = grpc_uri_parse(uri_str, true); - ASSERT_NE(lb_uri, nullptr); + absl::StatusOr lb_uri = + grpc_core::URI::Parse("ipv6:[0100::1234]:443"); + ASSERT_TRUE(lb_uri.ok()); grpc_resolved_address address; - ASSERT_TRUE(grpc_parse_uri(lb_uri, &address)); - grpc_uri_destroy(lb_uri); + ASSERT_TRUE(grpc_parse_uri(*lb_uri, &address)); grpc_core::ServerAddressList addresses; addresses.emplace_back(address.addr, address.len, nullptr); grpc_core::Resolver::Result lb_address_result; diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index 61b9c17bf24..644d782e116 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -199,12 +199,11 @@ class FakeResolverResponseGeneratorWrapper { nullptr) { grpc_core::Resolver::Result result; for (const int& port : ports) { - std::string lb_uri_str = - absl::StrCat(ipv6_only ? "ipv6:[::1]:" : "ipv4:127.0.0.1:", port); - grpc_uri* lb_uri = grpc_uri_parse(lb_uri_str.c_str(), true); - GPR_ASSERT(lb_uri != nullptr); + absl::StatusOr lb_uri = grpc_core::URI::Parse( + absl::StrCat(ipv6_only ? "ipv6:[::1]:" : "ipv4:127.0.0.1:", port)); + GPR_ASSERT(lb_uri.ok()); grpc_resolved_address address; - GPR_ASSERT(grpc_parse_uri(lb_uri, &address)); + GPR_ASSERT(grpc_parse_uri(*lb_uri, &address)); std::map> attributes; @@ -213,7 +212,6 @@ class FakeResolverResponseGeneratorWrapper { } result.addresses.emplace_back(address.addr, address.len, nullptr /* args */, std::move(attributes)); - grpc_uri_destroy(lb_uri); } if (service_config_json != nullptr) { result.service_config = grpc_core::ServiceConfig::Create( diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index 7858878ab33..a2183201839 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -540,18 +540,17 @@ class GrpclbEnd2endTest : public ::testing::Test { const std::vector& address_data) { grpc_core::ServerAddressList addresses; for (const auto& addr : address_data) { - std::string lb_uri_str = absl::StrCat( - ipv6_only_ ? "ipv6:[::1]:" : "ipv4:127.0.0.1:", addr.port); - grpc_uri* lb_uri = grpc_uri_parse(lb_uri_str.c_str(), true); - GPR_ASSERT(lb_uri != nullptr); + absl::StatusOr lb_uri = + grpc_core::URI::Parse(absl::StrCat( + ipv6_only_ ? "ipv6:[::1]:" : "ipv4:127.0.0.1:", addr.port)); + GPR_ASSERT(lb_uri.ok()); grpc_resolved_address address; - GPR_ASSERT(grpc_parse_uri(lb_uri, &address)); + GPR_ASSERT(grpc_parse_uri(*lb_uri, &address)); grpc_arg arg = grpc_core::CreateAuthorityOverrideChannelArg( addr.balancer_name.c_str()); grpc_channel_args* args = grpc_channel_args_copy_and_add(nullptr, &arg, 1); addresses.emplace_back(address.addr, address.len, args); - grpc_uri_destroy(lb_uri); } return addresses; } diff --git a/test/cpp/end2end/service_config_end2end_test.cc b/test/cpp/end2end/service_config_end2end_test.cc index 385512498e7..07f383a14a9 100644 --- a/test/cpp/end2end/service_config_end2end_test.cc +++ b/test/cpp/end2end/service_config_end2end_test.cc @@ -177,13 +177,12 @@ class ServiceConfigEnd2endTest : public ::testing::Test { for (const int& port : ports) { std::string lb_uri_str = absl::StrCat(ipv6_only_ ? "ipv6:[::1]:" : "ipv4:127.0.0.1:", port); - grpc_uri* lb_uri = grpc_uri_parse(lb_uri_str.c_str(), true); - GPR_ASSERT(lb_uri != nullptr); + absl::StatusOr lb_uri = grpc_core::URI::Parse(lb_uri_str); + GPR_ASSERT(lb_uri.ok()); grpc_resolved_address address; - GPR_ASSERT(grpc_parse_uri(lb_uri, &address)); + GPR_ASSERT(grpc_parse_uri(*lb_uri, &address)); result.addresses.emplace_back(address.addr, address.len, nullptr /* args */); - grpc_uri_destroy(lb_uri); } return result; } diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index 70c2891d195..fdff4c371ab 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -1807,14 +1807,12 @@ class XdsEnd2endTest : public ::testing::TestWithParam { const std::vector& ports) { grpc_core::ServerAddressList addresses; for (int port : ports) { - std::string lb_uri_str = - absl::StrCat(ipv6_only_ ? "ipv6:[::1]:" : "ipv4:127.0.0.1:", port); - grpc_uri* lb_uri = grpc_uri_parse(lb_uri_str.c_str(), true); - GPR_ASSERT(lb_uri != nullptr); + absl::StatusOr lb_uri = grpc_core::URI::Parse( + absl::StrCat(ipv6_only_ ? "ipv6:[::1]:" : "ipv4:127.0.0.1:", port)); + GPR_ASSERT(lb_uri.ok()); grpc_resolved_address address; - GPR_ASSERT(grpc_parse_uri(lb_uri, &address)); + GPR_ASSERT(grpc_parse_uri(*lb_uri, &address)); addresses.emplace_back(address.addr, address.len, nullptr); - grpc_uri_destroy(lb_uri); } return addresses; }