Add validation of the channel address (#1258)

grpc · Apr 6, 2021 · 78306e4 · 78306e4
1 parent 57a7ad5
commit 78306e4
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 0 deletions.
diff --git a/src/Grpc.Net.Client/GrpcChannel.cs b/src/Grpc.Net.Client/GrpcChannel.cs
@@ -64,6 +64,7 @@ public sealed class GrpcChannel : ChannelBase, IDisposable
         internal long? MaxRetryBufferSize { get; }
         internal long? MaxRetryBufferPerCallSize { get; }
         internal ILoggerFactory LoggerFactory { get; }
+        internal ILogger Logger { get; }
         internal bool ThrowOperationCanceledOnCancellation { get; }
         internal bool? IsSecure { get; }
         internal List<CallCredentials>? CallCredentials { get; }
@@ -105,6 +106,7 @@ internal GrpcChannel(Uri address, GrpcChannelOptions channelOptions) : base(addr
             CompressionProviders = ResolveCompressionProviders(channelOptions.CompressionProviders);
             MessageAcceptEncoding = GrpcProtocolHelpers.GetMessageAcceptEncoding(CompressionProviders);
             LoggerFactory = channelOptions.LoggerFactory ?? NullLoggerFactory.Instance;
+            Logger = LoggerFactory.CreateLogger<GrpcChannel>();
             ThrowOperationCanceledOnCancellation = channelOptions.ThrowOperationCanceledOnCancellation;
             _createMethodInfoFunc = CreateMethodInfo;
             ActiveCalls = new HashSet<IDisposable>();
@@ -125,6 +127,11 @@ internal GrpcChannel(Uri address, GrpcChannelOptions channelOptions) : base(addr
 
                 ValidateChannelCredentials();
             }
+
+            if (!string.IsNullOrEmpty(Address.PathAndQuery) && Address.PathAndQuery != "/")
+            {
+                Log.AddressPathUnused(Logger, Address.OriginalString);
+            }
         }
 
         private ChannelRetryThrottling CreateChannelRetryThrottling(RetryThrottlingPolicy retryThrottling)
@@ -339,6 +346,11 @@ public static GrpcChannel ForAddress(Uri address, GrpcChannelOptions channelOpti
                 throw new ArgumentNullException(nameof(channelOptions));
             }
 
+            if (string.IsNullOrEmpty(address.Host))
+            {
+                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'.");
+            }
+
             if (channelOptions.HttpClient != null && channelOptions.HttpHandler != null)
             {
                 throw new ArgumentException($"{nameof(GrpcChannelOptions.HttpClient)} and {nameof(GrpcChannelOptions.HttpHandler)} have been configured. " +
@@ -433,5 +445,16 @@ public override int GetHashCode() =>
                 (Service != null ? StringComparer.Ordinal.GetHashCode(Service) : 0) ^
                 (Method != null ? StringComparer.Ordinal.GetHashCode(Method) : 0);
         }
+
+        private static class Log
+        {
+            private static readonly Action<ILogger, string, Exception?> _addressPathUnused =
+                LoggerMessage.Define<string>(LogLevel.Debug, new EventId(1, "AddressPathUnused"), "The path in the channel's address '{Address}' won't be used when making gRPC calls. A DelegatingHandler can be used to include a path with gRPC calls. See https://aka.ms/aspnet/grpc/subdir for details.");
+
+            public static void AddressPathUnused(ILogger logger, string address)
+            {
+                _addressPathUnused(logger, address, null);
+            }
+        }
     }
 }
diff --git a/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs b/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs
@@ -26,12 +26,56 @@
 using Grpc.Net.Client.Configuration;
 using Grpc.Tests.Shared;
 using NUnit.Framework;
+using Microsoft.Extensions.Logging.Testing;
+using System.Linq;
+using Microsoft.Extensions.Logging;
 
 namespace Grpc.Net.Client.Tests
 {
     [TestFixture]
     public class GrpcChannelTests
     {
+        [Test]
+        public void Build_AddressWithoutHost_Error()
+        {
+            // Arrange & Act
+            var ex = Assert.Throws<ArgumentException>(() => GrpcChannel.ForAddress("test.example.com:5001"))!;
+
+            // Assert
+            Assert.AreEqual("Address 'test.example.com:5001' doesn't have a host. Address should include a scheme, host, and optional port. For example, 'https://localhost:5001'.", ex.Message);
+        }
+
+        [TestCase("https://localhost:5001/path", true)]
+        [TestCase("https://localhost:5001/?query=ya", true)]
+        [TestCase("https://localhost:5001//", true)]
+        [TestCase("https://localhost:5001/", false)]
+        [TestCase("https://localhost:5001", false)]
+        public void Build_AddressWithPath_Log(string address, bool hasPathOrQuery)
+        {
+            // Arrange
+            var testSink = new TestSink();
+            var testFactory = new TestLoggerFactory(testSink, enabled: true);
+
+            // Act
+            GrpcChannel.ForAddress(address, CreateGrpcChannelOptions(o => o.LoggerFactory = testFactory));
+
+            // Assert
+            var log = testSink.Writes.SingleOrDefault(w => w.EventId.Name == "AddressPathUnused");
+            if (hasPathOrQuery)
+            {
+                Assert.IsNotNull(log);
+                Assert.AreEqual(LogLevel.Debug, log!.LogLevel);
+
+                var message = $"The path in the channel's address '{address}' won't be used when making gRPC calls. " +
+                    "A DelegatingHandler can be used to include a path with gRPC calls. See https://aka.ms/aspnet/grpc/subdir for details.";
+                Assert.AreEqual(message, log.Message);
+            }
+            else
+            {
+                Assert.IsNull(log);
+            }
+        }
+
         [Test]
         public void Build_SslCredentialsWithHttps_Success()
         {