fix: In GrpcNetClientAdapter, be smart when guessing the scheme to us…

…e for schemeless URLs

Google.Api.Gax.Grpc.IntegrationTests/GrpcAdapterTest.cs

@@ -23,10 +23,7 @@ public class GrpcAdapterTest
         public void CreateChannelMakeCall()
         {
             var adapter = GrpcAdapter.GetFallbackAdapter(TestServiceMetadata.TestService);
-            // This is unfortunate, but required for the test.
-            // ("localhost:12345" is only valid in Grpc.Core; "http://localhost:12345" is only valid in Grpc.Net.Client.)
-            var endpoint = adapter is GrpcNetClientAdapter ? _fixture.HttpEndpoint : _fixture.Endpoint;
-            var channel = adapter.CreateChannel(TestServiceMetadata.TestService, endpoint, ChannelCredentials.Insecure, GrpcChannelOptions.Empty);
+            var channel = adapter.CreateChannel(TestServiceMetadata.TestService, _fixture.Endpoint, ChannelCredentials.Insecure, GrpcChannelOptions.Empty);
             var client = new TestServiceClient(channel);
             var response = client.DoSimple(new SimpleRequest { Name = "test-call" });
             Assert.Equal("test-call", response.Name);

Google.Api.Gax.Grpc.IntegrationTests/GrpcNetClientAdapterTest.cs

@@ -23,7 +23,7 @@ public class GrpcNetClientAdapterTest
         [Fact]
         public void CreateChannelMakeCall()
         {
-            var channel = GrpcNetClientAdapter.Default.CreateChannel(TestServiceMetadata.TestService, _fixture.HttpEndpoint, ChannelCredentials.Insecure, GrpcChannelOptions.Empty);
+            var channel = GrpcNetClientAdapter.Default.CreateChannel(TestServiceMetadata.TestService, _fixture.Endpoint, ChannelCredentials.Insecure, GrpcChannelOptions.Empty);
             var client = new TestServiceClient(channel);
             var response = client.DoSimple(new SimpleRequest { Name = "test-call" });
             Assert.Equal("test-call", response.Name);

Google.Api.Gax.Grpc.IntegrationTests/TestServiceFixture.cs

@@ -26,12 +26,6 @@ public class TestServiceFixture : IDisposable, ICollectionFixture<TestServiceFix
         public int Port => _server.Ports.First().BoundPort;
         public string Endpoint => $"localhost:{Port}";
 
-        // GrpcNetClientAdapter assumes https for any scheme-less URLs; it has to assume
-        // something as Grpc.Net.Client doesn't support them. We actually want HTTP here, so let's be explicit.
-        // This shouldn't be a problem for real services (where we'll use https anyway).
-        // (Grpc.Core doesn't like http as a scheme, so we can't just use that...)
-        public string HttpEndpoint => $"http://{Endpoint}";
-
         public TestServiceFixture()
         {
 #if NETCOREAPP3_1

Google.Api.Gax.Grpc.Tests/GrpcNetClientAdapterTest.cs

@@ -5,6 +5,7 @@
  * https://developers.google.com/open-source/licenses/bsd
  */
 
+using Grpc.Auth;
 using Grpc.Core;
 using System;
 using Xunit;
@@ -63,18 +64,36 @@ public void ConvertOptions_MaxReceiveMessageSize()
         }
 
         [Theory]
-        [InlineData("10.20.30.40", "https://10.20.30.40")]
-        [InlineData("10.20.30.40:1234", "https://10.20.30.40:1234")]
-        [InlineData("http://10.20.30.40", "http://10.20.30.40")]
-        [InlineData("https://10.20.30.40", "https://10.20.30.40")]
-        [InlineData("https://10.20.30.40:1234", "https://10.20.30.40:1234")]
-        [InlineData("service.googleapis.com", "https://service.googleapis.com")]
-        [InlineData("https://service.googleapis.com", "https://service.googleapis.com")]
-        [InlineData("service.googleapis.com:1234", "https://service.googleapis.com:1234")]
-        [InlineData("https://service.googleapis.com:1234", "https://service.googleapis.com:1234")]
-        public void ConvertEndpoint(string input, string expectedOutput)
+        [InlineData("10.20.30.40", true, "https://10.20.30.40")]
+        [InlineData("10.20.30.40", false, "http://10.20.30.40")]
+        [InlineData("10.20.30.40:1234", true, "https://10.20.30.40:1234")]
+        [InlineData("10.20.30.40:1234", false, "http://10.20.30.40:1234")]
+        [InlineData("http://10.20.30.40", true, "http://10.20.30.40")]
+        [InlineData("http://10.20.30.40", false, "http://10.20.30.40")]
+        [InlineData("https://10.20.30.40", true, "https://10.20.30.40")]
+        [InlineData("https://10.20.30.40:1234", true, "https://10.20.30.40:1234")]
+        [InlineData("service.googleapis.com", true, "https://service.googleapis.com")]
+        [InlineData("https://service.googleapis.com", true, "https://service.googleapis.com")]
+        [InlineData("service.googleapis.com:1234", true, "https://service.googleapis.com:1234")]
+        [InlineData("https://service.googleapis.com:1234", true, "https://service.googleapis.com:1234")]
+        public void ConvertEndpoint(string input, bool secureCredentials, string expectedOutput)
         {
-            Assert.Equal(expectedOutput, GrpcNetClientAdapter.ConvertEndpoint(input));
+            Assert.Equal(expectedOutput, GrpcNetClientAdapter.ConvertEndpoint(input, secureCredentials));
+        }
+
+        public static TheoryData<ChannelCredentials, bool> SecureCredentialsDetectionData = new TheoryData<ChannelCredentials, bool>
+        {
+            { ChannelCredentials.SecureSsl, true },
+            { ChannelCredentials.Insecure, false },
+            { TestServiceCredentials.CreateTestServiceAccountCredential().ToChannelCredentials(), true }
+        };
+
+        [Theory]
+        [MemberData(nameof(SecureCredentialsDetectionData))]
+        public void SecureCredentialsDetection_Insecure(ChannelCredentials credentials, bool expectedResult)
+        {
+            var actualResult = GrpcNetClientAdapter.SecureChannelDetector.AreCredentialsSecure(credentials);
+            Assert.Equal(expectedResult, actualResult);
         }
     }
 }

Google.Api.Gax.Grpc/GrpcNetClientAdapter.cs

@@ -49,7 +49,8 @@ private protected override ChannelBase CreateChannelImpl(ServiceMetadata service
         {
             var grpcNetClientOptions = ConvertOptions(credentials, options);
             _optionsConfigurationAction?.Invoke(grpcNetClientOptions);
-            var address = ConvertEndpoint(endpoint);
+            bool secureCredentials = SecureChannelDetector.AreCredentialsSecure(credentials);
+            var address = ConvertEndpoint(endpoint, secureCredentials);
             return GrpcChannel.ForAddress(address, grpcNetClientOptions);
         }
 
@@ -77,9 +78,36 @@ private protected override ChannelBase CreateChannelImpl(ServiceMetadata service
         }
 
         // Internal for testing
-        internal static string ConvertEndpoint(string endpoint) =>
-            // Note that we assume HTTPS for any bare address; this feels like a reasonable assumption for now.
-            endpoint.StartsWith("http:", StringComparison.Ordinal) || endpoint.StartsWith("https:", StringComparison.Ordinal)
-            ? endpoint : $"https://{endpoint}";
+        internal static string ConvertEndpoint(string endpoint, bool secureCredentials)
+        {
+            string defaultScheme = secureCredentials ? "https" : "http";
+            // We assume HTTPS for any bare address if the credentials are secure, or HTTP otherwise.
+            // This matches what GrpcChannel itself does in terms of validation.
+            return endpoint.StartsWith("http:", StringComparison.Ordinal) || endpoint.StartsWith("https:", StringComparison.Ordinal)
+                ? endpoint : $"{defaultScheme}://{endpoint}";
+        }
+
+        // Internal for testing
+        internal class SecureChannelDetector : ChannelCredentialsConfiguratorBase
+        {
+            private bool? IsSecure { get; set; }
+
+            internal static bool AreCredentialsSecure(ChannelCredentials credentials)
+            {
+                var instance = new SecureChannelDetector();
+                credentials.InternalPopulateConfiguration(instance, null);
+                // Default to detecting credentials as insecure.
+                return instance.IsSecure ?? false;
+            }
+
+            public override void SetCompositeCredentials(object state, ChannelCredentials channelCredentials, CallCredentials callCredentials) =>
+                channelCredentials.InternalPopulateConfiguration(this, state);
+
+            public override void SetInsecureCredentials(object state) =>
+                IsSecure = false;
+
+            public override void SetSslCredentials(object state, string rootCertificates, KeyCertificatePair keyCertificatePair, VerifyPeerCallback verifyPeerCallback) =>
+                IsSecure = true;
+        }
     }
 }