[resolvers] use proper %-encoding of authority by default (#33571)

- Change the `ResolverFactory::GetDefaultAuthority()` method to %-encode
the authority by default, so individual resolver impls don't need to
remember to do this.
- Remove the hack in the xds resolver for setting the authority to
everything after the last `/` character.
- Change the `unix`, `unix-abstract`, and `vsock` resolvers to use a
real authority instead of hard-coding to "localhost".

src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc

@@ -112,6 +112,8 @@ OrphanablePtr<Resolver> CreateSockaddrResolver(
 
 class IPv4ResolverFactory : public ResolverFactory {
  public:
+  absl::string_view scheme() const override { return "ipv4"; }
+
   bool IsValidUri(const URI& uri) const override {
     return ParseUri(uri, grpc_parse_ipv4, nullptr);
   }
@@ -119,12 +121,12 @@ class IPv4ResolverFactory : public ResolverFactory {
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
     return CreateSockaddrResolver(std::move(args), grpc_parse_ipv4);
   }
-
-  absl::string_view scheme() const override { return "ipv4"; }
 };
 
 class IPv6ResolverFactory : public ResolverFactory {
  public:
+  absl::string_view scheme() const override { return "ipv6"; }
+
   bool IsValidUri(const URI& uri) const override {
     return ParseUri(uri, grpc_parse_ipv6, nullptr);
   }
@@ -132,13 +134,13 @@ class IPv6ResolverFactory : public ResolverFactory {
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
     return CreateSockaddrResolver(std::move(args), grpc_parse_ipv6);
   }
-
-  absl::string_view scheme() const override { return "ipv6"; }
 };
 
 #ifdef GRPC_HAVE_UNIX_SOCKET
 class UnixResolverFactory : public ResolverFactory {
  public:
+  absl::string_view scheme() const override { return "unix"; }
+
   bool IsValidUri(const URI& uri) const override {
     return ParseUri(uri, grpc_parse_unix, nullptr);
   }
@@ -146,12 +148,6 @@ class UnixResolverFactory : public ResolverFactory {
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
     return CreateSockaddrResolver(std::move(args), grpc_parse_unix);
   }
-
-  std::string GetDefaultAuthority(const URI& /*uri*/) const override {
-    return "localhost";
-  }
-
-  absl::string_view scheme() const override { return "unix"; }
 };
 
 class UnixAbstractResolverFactory : public ResolverFactory {
@@ -165,16 +161,14 @@ class UnixAbstractResolverFactory : public ResolverFactory {
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
     return CreateSockaddrResolver(std::move(args), grpc_parse_unix_abstract);
   }
-
-  std::string GetDefaultAuthority(const URI& /*uri*/) const override {
-    return "localhost";
-  }
 };
 #endif  // GRPC_HAVE_UNIX_SOCKET
 
 #ifdef GRPC_HAVE_VSOCK
 class VSockResolverFactory : public ResolverFactory {
  public:
+  absl::string_view scheme() const override { return "vsock"; }
+
   bool IsValidUri(const URI& uri) const override {
     return ParseUri(uri, grpc_parse_vsock, nullptr);
   }
@@ -182,12 +176,6 @@ class VSockResolverFactory : public ResolverFactory {
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
     return CreateSockaddrResolver(std::move(args), grpc_parse_vsock);
   }
-
-  std::string GetDefaultAuthority(const URI& /*uri*/) const override {
-    return "localhost";
-  }
-
-  absl::string_view scheme() const override { return "vsock"; }
 };
 
 #endif  // GRPC_HAVE_VSOCK

src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc

@@ -17,7 +17,6 @@
 #include <grpc/support/port_platform.h>
 
 #include <stdint.h>
-#include <stdlib.h>
 #include <string.h>
 
 #include <algorithm>
@@ -103,36 +102,19 @@ TraceFlag grpc_xds_resolver_trace(false, "xds_resolver");
 
 namespace {
 
-std::string GetDefaultAuthorityInternal(const URI& uri) {
-  // Obtain the authority to use for the data plane connections, which is
-  // also used to select the right VirtualHost from the RouteConfiguration.
-  // We need to take the part of the URI path following the last
-  // "/" character or the entire path if the path contains no "/" character.
-  size_t pos = uri.path().find_last_of('/');
-  if (pos == uri.path().npos) return uri.path();
-  return uri.path().substr(pos + 1);
-}
-
-std::string GetDataPlaneAuthority(const ChannelArgs& args, const URI& uri) {
-  absl::optional<std::string> authority =
-      args.GetOwnedString(GRPC_ARG_DEFAULT_AUTHORITY);
-  if (authority.has_value()) return std::move(*authority);
-  return GetDefaultAuthorityInternal(uri);
-}
-
 //
 // XdsResolver
 //
 
 class XdsResolver : public Resolver {
  public:
-  explicit XdsResolver(ResolverArgs args)
+  XdsResolver(ResolverArgs args, std::string data_plane_authority)
       : work_serializer_(std::move(args.work_serializer)),
         result_handler_(std::move(args.result_handler)),
         args_(std::move(args.args)),
         interested_parties_(args.pollset_set),
         uri_(std::move(args.uri)),
-        data_plane_authority_(GetDataPlaneAuthority(args_, uri_)),
+        data_plane_authority_(std::move(data_plane_authority)),
         channel_id_(absl::Uniform<uint64_t>(absl::BitGen())) {
     if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_resolver_trace)) {
       gpr_log(
@@ -1268,15 +1250,22 @@ class XdsResolverFactory : public ResolverFactory {
     return true;
   }
 
-  std::string GetDefaultAuthority(const URI& uri) const override {
-    return GetDefaultAuthorityInternal(uri);
-  }
-
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
     if (!IsValidUri(args.uri)) return nullptr;
-    return MakeOrphanable<XdsResolver>(std::move(args));
+    std::string authority = GetDataPlaneAuthority(args.args, args.uri);
+    return MakeOrphanable<XdsResolver>(std::move(args), std::move(authority));
+  }
+
+ private:
+  std::string GetDataPlaneAuthority(const ChannelArgs& args,
+                                    const URI& uri) const {
+    absl::optional<absl::string_view> authority =
+        args.GetString(GRPC_ARG_DEFAULT_AUTHORITY);
+    if (authority.has_value()) return URI::PercentEncodeAuthority(*authority);
+    return GetDefaultAuthority(uri);
   }
 };
+
 }  // namespace
 
 void RegisterXdsResolver(CoreConfiguration::Builder* builder) {

src/core/lib/resolver/resolver_factory.h

@@ -66,9 +66,10 @@ class ResolverFactory {
   virtual OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const = 0;
 
   /// Returns a string representing the default authority to use for this
-  /// scheme.
+  /// scheme.  By default, we %-encode the path part of the target URI,
+  /// excluding the initial '/' character.
   virtual std::string GetDefaultAuthority(const URI& uri) const {
-    return std::string(absl::StripPrefix(uri.path(), "/"));
+    return URI::PercentEncodeAuthority(absl::StripPrefix(uri.path(), "/"));
   }
 };
 

test/core/end2end/tests/default_host.cc

@@ -60,8 +60,10 @@ CORE_END2END_TEST(CoreClientChannelTest, DefaultHost) {
   if (GetParam()->overridden_call_host != nullptr) {
     EXPECT_EQ(GetParam()->overridden_call_host, s.host());
   } else {
-    EXPECT_THAT(s.host(), AnyOf(StartsWith("localhost"),
-                                StartsWith("127.0.0.1"), StartsWith("[::1]")));
+    EXPECT_THAT(s.host(),
+                AnyOf(StartsWith("localhost"), StartsWith("127.0.0.1"),
+                      StartsWith("[::1]"), StartsWith("grpc_fullstack_test."),
+                      StartsWith("tmp%2Fgrpc_fullstack_test.")));
   }
   EXPECT_FALSE(client_close.was_cancelled());
 }