DNS resolver fix port usage (#1493)

Co-authored-by: Vuk Vasić <[email protected]>

Author: JamesNK

src/Grpc.Net.Client/Balancer/DnsResolver.cs

@@ -42,7 +42,9 @@ internal sealed class DnsResolver : Resolver
         // To prevent excessive re-resolution, we enforce a rate limit on DNS resolution requests.
         private static readonly TimeSpan MinimumDnsResolutionRate = TimeSpan.FromSeconds(15);
 
-        private readonly Uri _address;
+        private readonly Uri _originalAddress;
+        private readonly string _dnsAddress;
+        private readonly int _port;
         private readonly TimeSpan _refreshInterval;
         private readonly ILogger _logger;
 
@@ -56,11 +58,18 @@ internal sealed class DnsResolver : Resolver
         /// Initializes a new instance of the <see cref="DnsResolver"/> class with the specified target <see cref="Uri"/>.
         /// </summary>
         /// <param name="address">The target <see cref="Uri"/>.</param>
+        /// <param name="defaultPort">The default port.</param>
         /// <param name="loggerFactory">The logger factory.</param>
         /// <param name="refreshInterval">An interval for automatically refreshing the DNS hostname.</param>
-        public DnsResolver(Uri address, ILoggerFactory loggerFactory, TimeSpan refreshInterval) : base(loggerFactory)
+        public DnsResolver(Uri address, int defaultPort, ILoggerFactory loggerFactory, TimeSpan refreshInterval) : base(loggerFactory)
         {
-            _address = address;
+            _originalAddress = address;
+
+            // DNS address has the format: dns:[//authority/]host[:port]
+            // Because the host is specified in the path, the port needs to be parsed manually
+            var addressParsed = new Uri("temp://" + address.AbsolutePath.TrimStart('/'));
+            _dnsAddress = addressParsed.Host;
+            _port = addressParsed.Port == -1 ? defaultPort : addressParsed.Port;
             _refreshInterval = refreshInterval;
             _logger = loggerFactory.CreateLogger<DnsResolver>();
         }
@@ -92,28 +101,25 @@ protected override async Task ResolveAsync(CancellationToken cancellationToken)
 
                 _lastResolveStart = SystemClock.UtcNow;
 
-                var dnsAddress = _address.AbsolutePath.TrimStart('/');
-
-                if (string.IsNullOrEmpty(dnsAddress))
+                if (string.IsNullOrEmpty(_dnsAddress))
                 {
-                    throw new InvalidOperationException($"Resolver address '{_address}' doesn't have a path.");
+                    throw new InvalidOperationException($"Resolver address '{_originalAddress}' is not valid. Please use dns:/// for DNS provider.");
                 }
 
-                DnsResolverLog.StartingDnsQuery(_logger, _address);
-                var addresses = await Dns.GetHostAddressesAsync(dnsAddress).ConfigureAwait(false);
+                DnsResolverLog.StartingDnsQuery(_logger, _dnsAddress);
+                var addresses = await Dns.GetHostAddressesAsync(_dnsAddress).ConfigureAwait(false);
 
-                DnsResolverLog.ReceivedDnsResults(_logger, addresses.Length, _address, addresses);
+                DnsResolverLog.ReceivedDnsResults(_logger, addresses.Length, _dnsAddress, addresses);
 
-                var resolvedPort = _address.Port == -1 ? 80 : _address.Port;
-                var endpoints = addresses.Select(a => new BalancerAddress(a.ToString(), resolvedPort)).ToArray();
+                var endpoints = addresses.Select(a => new BalancerAddress(a.ToString(), _port)).ToArray();
                 var resolverResult = ResolverResult.ForResult(endpoints);
                 Listener(resolverResult);
             }
             catch (Exception ex)
             {
-                var message = $"Error getting DNS hosts for address '{_address}'.";
+                var message = $"Error getting DNS hosts for address '{_dnsAddress}'.";
 
-                DnsResolverLog.ErrorQueryingDns(_logger, _address, ex);
+                DnsResolverLog.ErrorQueryingDns(_logger, _dnsAddress, ex);
                 Listener(ResolverResult.ForFailure(GrpcProtocolHelpers.CreateStatusFromException(message, ex, StatusCode.Unavailable)));
             }
         }
@@ -144,14 +150,14 @@ internal static class DnsResolverLog
         private static readonly Action<ILogger, TimeSpan, TimeSpan, Exception?> _startingRateLimitDelay =
             LoggerMessage.Define<TimeSpan, TimeSpan>(LogLevel.Debug, new EventId(1, "StartingRateLimitDelay"), "Starting rate limit delay of {DelayDuration}. DNS resolution rate limit is once every {RateLimitDuration}.");
 
-        private static readonly Action<ILogger, Uri, Exception?> _startingDnsQuery =
-            LoggerMessage.Define<Uri>(LogLevel.Trace, new EventId(2, "StartingDnsQuery"), "Starting DNS query to get hosts from '{DnsAddress}'.");
+        private static readonly Action<ILogger, string, Exception?> _startingDnsQuery =
+            LoggerMessage.Define<string>(LogLevel.Trace, new EventId(2, "StartingDnsQuery"), "Starting DNS query to get hosts from '{DnsAddress}'.");
 
-        private static readonly Action<ILogger, int, Uri, string, Exception?> _receivedDnsResults =
-            LoggerMessage.Define<int, Uri, string>(LogLevel.Debug, new EventId(3, "ReceivedDnsResults"), "Received {ResultCount} DNS results from '{DnsAddress}'. Results: {DnsResults}");
+        private static readonly Action<ILogger, int, string, string, Exception?> _receivedDnsResults =
+            LoggerMessage.Define<int, string, string>(LogLevel.Debug, new EventId(3, "ReceivedDnsResults"), "Received {ResultCount} DNS results from '{DnsAddress}'. Results: {DnsResults}");
 
-        private static readonly Action<ILogger, Uri, Exception?> _errorQueryingDns =
-            LoggerMessage.Define<Uri>(LogLevel.Error, new EventId(4, "ErrorQueryingDns"), "Error querying DNS hosts for '{DnsAddress}'.");
+        private static readonly Action<ILogger, string, Exception?> _errorQueryingDns =
+            LoggerMessage.Define<string>(LogLevel.Error, new EventId(4, "ErrorQueryingDns"), "Error querying DNS hosts for '{DnsAddress}'.");
 
         private static readonly Action<ILogger, Exception?> _errorFromRefreshInterval =
             LoggerMessage.Define(LogLevel.Error, new EventId(5, "ErrorFromRefreshIntervalTimer"), "Error from refresh interval timer.");
@@ -161,20 +167,20 @@ public static void StartingRateLimitDelay(ILogger logger, TimeSpan delayDuration
             _startingRateLimitDelay(logger, delayDuration, rateLimitDuration, null);
         }
 
-        public static void StartingDnsQuery(ILogger logger, Uri dnsAddress)
+        public static void StartingDnsQuery(ILogger logger, string dnsAddress)
         {
             _startingDnsQuery(logger, dnsAddress, null);
         }
 
-        public static void ReceivedDnsResults(ILogger logger, int resultCount, Uri dnsAddress, IList<IPAddress> dnsResults)
+        public static void ReceivedDnsResults(ILogger logger, int resultCount, string dnsAddress, IList<IPAddress> dnsResults)
         {
             if (logger.IsEnabled(LogLevel.Debug))
             {
                 _receivedDnsResults(logger, resultCount, dnsAddress, string.Join(", ", dnsResults), null);
             }
         }
 
-        public static void ErrorQueryingDns(ILogger logger, Uri dnsAddress, Exception ex)
+        public static void ErrorQueryingDns(ILogger logger, string dnsAddress, Exception ex)
         {
             _errorQueryingDns(logger, dnsAddress, ex);
         }
@@ -211,7 +217,7 @@ public DnsResolverFactory(TimeSpan refreshInterval)
         /// <inheritdoc />
         public override Resolver Create(ResolverOptions options)
         {
-            return new DnsResolver(options.Address, options.LoggerFactory, _refreshInterval);
+            return new DnsResolver(options.Address, options.DefaultPort, options.LoggerFactory, _refreshInterval);
         }
     }
 }

src/Grpc.Net.Client/Balancer/ResolverOptions.cs

@@ -33,9 +33,10 @@ public sealed class ResolverOptions
         /// <summary>
         /// Initializes a new instance of the <see cref="ResolverOptions"/> class.
         /// </summary>
-        internal ResolverOptions(Uri address, bool disableServiceConfig, ILoggerFactory loggerFactory)
+        internal ResolverOptions(Uri address, int defaultPort, bool disableServiceConfig, ILoggerFactory loggerFactory)
         {
             Address = address;
+            DefaultPort = defaultPort;
             DisableServiceConfig = disableServiceConfig;
             LoggerFactory = loggerFactory;
         }
@@ -45,6 +46,11 @@ internal ResolverOptions(Uri address, bool disableServiceConfig, ILoggerFactory
         /// </summary>
         public Uri Address { get; }
 
+        /// <summary>
+        /// Gets the default port. This port is used when the resolver address doesn't specify a port.
+        /// </summary>
+        public int DefaultPort { get; }
+
         /// <summary>
         /// Gets a flag indicating whether the resolver should disable resolving a service config.
         /// </summary>

src/Grpc.Net.Client/GrpcChannel.cs

@@ -55,6 +55,8 @@ public sealed class GrpcChannel : ChannelBase, IDisposable
         private readonly ConcurrentDictionary<IMethod, GrpcMethodInfo> _methodInfoCache;
         private readonly Func<IMethod, GrpcMethodInfo> _createMethodInfoFunc;
         private readonly Dictionary<MethodKey, MethodConfig>? _serviceConfigMethods;
+        private readonly bool _isSecure;
+        private readonly List<CallCredentials>? _callCredentials;
         // Internal for testing
         internal readonly HashSet<IDisposable> ActiveCalls;
 
@@ -69,8 +71,8 @@ public sealed class GrpcChannel : ChannelBase, IDisposable
         internal ILoggerFactory LoggerFactory { get; }
         internal ILogger Logger { get; }
         internal bool ThrowOperationCanceledOnCancellation { get; }
-        internal bool IsSecure { get; }
-        internal List<CallCredentials>? CallCredentials { get; }
+        internal bool IsSecure => _isSecure;
+        internal List<CallCredentials>? CallCredentials => _callCredentials;
         internal Dictionary<string, ICompressionProvider> CompressionProviders { get; }
         internal string MessageAcceptEncoding { get; }
         internal bool Disposed { get; private set; }
@@ -118,21 +120,18 @@ internal GrpcChannel(Uri address, GrpcChannelOptions channelOptions) : base(addr
             HttpHandlerType = CalculateHandlerType(channelOptions);
 
 #if SUPPORT_LOAD_BALANCING
+            var resolverFactory = GetResolverFactory(channelOptions);
+            ResolveCredentials(channelOptions, out _isSecure, out _callCredentials);
+
             SubchannelTransportFactory = ResolveService<ISubchannelTransportFactory>(channelOptions.ServiceProvider, new SubChannelTransportFactory(this));
 
             if (!IsHttpOrHttpsAddress() || channelOptions.ServiceConfig?.LoadBalancingConfigs.Count > 0)
             {
                 ValidateHttpHandlerSupportsConnectivity();
             }
 
-            // Special case http and https schemes. These schemes don't use a dynamic resolver. An http/https
-            // address is always just one address and that is enabled using the static resolver.
-            //
-            // Even with just one address we still want to use the load balancing infrastructure. This enables
-            // the connectivity APIs on channel like GrpcChannel.State and GrpcChannel.WaitForStateChanged.
-            var resolver = IsHttpOrHttpsAddress()
-                ? new StaticResolver(new[] { new BalancerAddress(Address.Host, Address.Port) }, LoggerFactory)
-                : CreateResolver(channelOptions);
+            var defaultPort = IsSecure ? 443 : 80;
+            var resolver = resolverFactory.Create(new ResolverOptions(Address, defaultPort, channelOptions.DisableResolverServiceConfig, LoggerFactory));
 
             ConnectionManager = new ConnectionManager(
                 resolver,
@@ -149,6 +148,7 @@ internal GrpcChannel(Uri address, GrpcChannelOptions channelOptions) : base(addr
             {
                 throw new ArgumentException($"Address '{address.OriginalString}' doesn't have a host. Address should include a scheme, host, and optional port. For example, 'https://localhost:5001'.");
             }
+            ResolveCredentials(channelOptions, out _isSecure, out _callCredentials);
 #endif
 
             HttpInvoker = channelOptions.HttpClient ?? CreateInternalHttpInvoker(channelOptions.HttpHandler);
@@ -169,25 +169,37 @@ internal GrpcChannel(Uri address, GrpcChannelOptions channelOptions) : base(addr
                 _serviceConfigMethods = CreateServiceConfigMethods(serviceConfig);
             }
 
+            // Non-HTTP addresses (e.g. dns:///custom-hostname) usually specify a path instead of an authority.
+            // Only log about a path being present if HTTP or HTTPS.
+            if (!string.IsNullOrEmpty(Address.PathAndQuery) &&
+                Address.PathAndQuery != "/" &&
+                (Address.Scheme == Uri.UriSchemeHttps || Address.Scheme == Uri.UriSchemeHttp))
+            {
+                Log.AddressPathUnused(Logger, Address.OriginalString);
+            }
+        }
+
+        private void ResolveCredentials(GrpcChannelOptions channelOptions, out bool isSecure, out List<CallCredentials>? callCredentials)
+        {
             if (channelOptions.Credentials != null)
             {
                 var configurator = new DefaultChannelCredentialsConfigurator();
                 channelOptions.Credentials.InternalPopulateConfiguration(configurator, null);
 
-                IsSecure = configurator.IsSecure ?? false;
-                CallCredentials = configurator.CallCredentials;
+                isSecure = configurator.IsSecure ?? false;
+                callCredentials = configurator.CallCredentials;
 
                 ValidateChannelCredentials();
             }
             else
             {
                 if (Address.Scheme == Uri.UriSchemeHttp)
                 {
-                    IsSecure = false;
+                    isSecure = false;
                 }
                 else if (Address.Scheme == Uri.UriSchemeHttps)
                 {
-                    IsSecure = true;
+                    isSecure = true;
                 }
                 else
                 {
@@ -196,15 +208,7 @@ internal GrpcChannel(Uri address, GrpcChannelOptions channelOptions) : base(addr
                         "To call TLS endpoints, set credentials to 'new SslCredentials()'. " +
                         "To call non-TLS endpoints, set credentials to 'ChannelCredentials.Insecure'.");
                 }
-            }
-
-            // Non-HTTP addresses (e.g. dns:///custom-hostname) usually specify a path instead of an authority.
-            // Only log about a path being present if HTTP or HTTPS.
-            if (!string.IsNullOrEmpty(Address.PathAndQuery) &&
-                Address.PathAndQuery != "/" &&
-                (Address.Scheme == Uri.UriSchemeHttps || Address.Scheme == Uri.UriSchemeHttp))
-            {
-                Log.AddressPathUnused(Logger, Address.OriginalString);
+                callCredentials = null;
             }
         }
 
@@ -227,6 +231,32 @@ private static HttpHandlerType CalculateHandlerType(GrpcChannelOptions channelOp
         }
 
 #if SUPPORT_LOAD_BALANCING
+        private ResolverFactory GetResolverFactory(GrpcChannelOptions options)
+        {
+            // Special case http and https schemes. These schemes don't use a dynamic resolver. An http/https
+            // address is always just one address and that is enabled using the static resolver.
+            //
+            // Even with just one address we still want to use the load balancing infrastructure. This enables
+            // the connectivity APIs on channel like GrpcChannel.State and GrpcChannel.WaitForStateChanged.
+            if (IsHttpOrHttpsAddress())
+            {
+                return new StaticResolverFactory(uri => new[] { new BalancerAddress(Address.Host, Address.Port) });
+            }
+
+            var factories = ResolveService<IEnumerable<ResolverFactory>>(options.ServiceProvider, Array.Empty<ResolverFactory>());
+            factories = factories.Union(ResolverFactory.KnownLoadResolverFactories);
+
+            foreach (var factory in factories)
+            {
+                if (string.Equals(factory.Name, Address.Scheme, StringComparison.OrdinalIgnoreCase))
+                {
+                    return factory;
+                }
+            }
+
+            throw new InvalidOperationException($"No address resolver configured for the scheme '{Address.Scheme}'.");
+        }
+
         private void ValidateHttpHandlerSupportsConnectivity()
         {
             if (HttpHandlerType == HttpHandlerType.SocketsHttpHandler)
@@ -250,22 +280,6 @@ private void ValidateHttpHandlerSupportsConnectivity()
                 $"The HTTP transport must be configured on the channel using {nameof(GrpcChannelOptions)}.{nameof(GrpcChannelOptions.HttpHandler)}.");
         }
 
-        private Resolver CreateResolver(GrpcChannelOptions options)
-        {
-            var factories = ResolveService<IEnumerable<ResolverFactory>>(options.ServiceProvider, Array.Empty<ResolverFactory>());
-            factories = factories.Union(ResolverFactory.KnownLoadResolverFactories);
-
-            foreach (var factory in factories)
-            {
-                if (string.Equals(factory.Name, Address.Scheme, StringComparison.OrdinalIgnoreCase))
-                {
-                    return factory.Create(new ResolverOptions(Address, options.DisableResolverServiceConfig, LoggerFactory));
-                }
-            }
-
-            throw new InvalidOperationException($"No address resolver configured for the scheme '{Address.Scheme}'.");
-        }
-
         private LoadBalancerFactory[] ResolveLoadBalancerFactories(IServiceProvider? serviceProvider)
         {
             var serviceFactories = ResolveService<IEnumerable<LoadBalancerFactory>?>(serviceProvider, defaultValue: null);