gRPC JSON transcoding: Fix not supporting JSON name in querystring na…

…mes (dotnet#46624)
Applesauce314 · Feb 18, 2023 · c9caab2 · c9caab2
1 parent 320c6a6
commit c9caab2
Show file tree

Hide file tree

Showing 6 changed files with 135 additions and 12 deletions.
diff --git a/....AspNetCore.Grpc.JsonTranscoding/Internal/Binding/JsonTranscodingProviderServiceBinder.cs b/....AspNetCore.Grpc.JsonTranscoding/Internal/Binding/JsonTranscodingProviderServiceBinder.cs
@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics.CodeAnalysis;
-using System.Linq;
 using Google.Api;
 using Google.Protobuf.Reflection;
 using Grpc.AspNetCore.Server;

diff --git a/...nTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/JsonRequestHelpers.cs b/...nTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/JsonRequestHelpers.cs
@@ -335,7 +335,7 @@ private static async ValueTask<byte[]> ReadDataAsync(JsonTranscodingServerCallCo
     {
         return serverCallContext.DescriptorInfo.PathDescriptorsCache.GetOrAdd(path, p =>
         {
-            ServiceDescriptorHelpers.TryResolveDescriptors(requestMessage.Descriptor, p.Split('.'), out var pathDescriptors);
+            ServiceDescriptorHelpers.TryResolveDescriptors(requestMessage.Descriptor, p.Split('.'), allowJsonName: true, out var pathDescriptors);
             return pathDescriptors;
         });
     }

diff --git a/src/Grpc/JsonTranscoding/src/Shared/ServiceDescriptorHelpers.cs b/src/Grpc/JsonTranscoding/src/Shared/ServiceDescriptorHelpers.cs
@@ -63,26 +63,30 @@ internal static bool IsWrapperType(DescriptorBase m) =>
         throw new InvalidOperationException($"Get not find Descriptor property on {serviceReflectionType.Name}.");
     }
 
-    public static bool TryResolveDescriptors(MessageDescriptor messageDescriptor, IList<string> path, [NotNullWhen(true)]out List<FieldDescriptor>? fieldDescriptors)
+    public static bool TryResolveDescriptors(MessageDescriptor messageDescriptor, IList<string> path, bool allowJsonName, [NotNullWhen(true)]out List<FieldDescriptor>? fieldDescriptors)
     {
         fieldDescriptors = null;
         MessageDescriptor? currentDescriptor = messageDescriptor;
 
         foreach (var fieldName in path)
         {
-            var field = currentDescriptor?.FindFieldByName(fieldName);
-            if (field == null)
+            FieldDescriptor? field = null;
+            if (currentDescriptor != null)
             {
-                fieldDescriptors = null;
-                return false;
+                field = allowJsonName
+                    ? GetFieldByName(currentDescriptor, fieldName)
+                    : currentDescriptor.FindFieldByName(fieldName);
             }
 
-            if (fieldDescriptors == null)
+            if (field == null)
             {
-                fieldDescriptors = new List<FieldDescriptor>();
+                fieldDescriptors = null;
+                return false;
             }
 
+            fieldDescriptors ??= new List<FieldDescriptor>();
             fieldDescriptors.Add(field);
+
             if (field.FieldType == FieldType.Message)
             {
                 currentDescriptor = field.MessageType;
@@ -96,6 +100,34 @@ public static bool TryResolveDescriptors(MessageDescriptor messageDescriptor, IL
         return fieldDescriptors != null;
     }
 
+    private static FieldDescriptor? GetFieldByName(MessageDescriptor messageDescriptor, string fieldName)
+    {
+        // Search fields by field name and JSON name. Both names can be referenced.
+        // If there are conflicts, then the last field with a name wins.
+        // This logic matches how properties are used in JSON serialization's MessageTypeInfoResolver.
+        var fields = messageDescriptor.Fields.InFieldNumberOrder();
+
+        FieldDescriptor? fieldDescriptor = null;
+        for (var i = fields.Count - 1; i >= 0; i--)
+        {
+            // We're checking JSON name first, in reverse order through fields.
+            // That means the method can exit early on match because the match has the highest precedence.
+            var field = fields[i];
+            if (field.JsonName == fieldName)
+            {
+                fieldDescriptor = field;
+                break;
+            }
+            else if (field.Name == fieldName)
+            {
+                fieldDescriptor = field;
+                break;
+            }
+        }
+
+        return fieldDescriptor;
+    }
+
     private static object? ConvertValue(object? value, FieldDescriptor descriptor)
     {
         switch (descriptor.FieldType)
@@ -298,7 +330,7 @@ public static Dictionary<string, RouteParameter> ResolveRouteParameterDescriptor
         foreach (var variable in variables)
         {
             var path = variable.FieldPath;
-            if (!TryResolveDescriptors(messageDescriptor, path, out var fieldDescriptors))
+            if (!TryResolveDescriptors(messageDescriptor, path, allowJsonName: false, out var fieldDescriptors))
             {
                 throw new InvalidOperationException($"Couldn't find matching field for route parameter '{string.Join(".", path)}' on {messageDescriptor.Name}.");
             }

diff --git a/.../Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs b/.../Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs
@@ -27,11 +27,23 @@ public JsonConverterReadTests(ITestOutputHelper output)
     public void NonJsonName()
     {
         var json = @"{
+  ""hiding_field_name"": ""A field name""
+}";
+
+        var m = AssertReadJson<HelloRequest>(json);
+        Assert.Equal("A field name", m.HidingFieldName);
+    }
+
+    [Fact]
+    public void HidingJsonName()
+    {
+        var json = @"{
   ""field_name"": ""A field name""
 }";
 
         var m = AssertReadJson<HelloRequest>(json);
-        Assert.Equal("A field name", m.FieldName);
+        Assert.Equal("", m.FieldName);
+        Assert.Equal("A field name", m.HidingFieldName);
     }
 
     [Fact]

diff --git a/...nTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/Proto/transcoding.proto b/...nTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/Proto/transcoding.proto
@@ -200,6 +200,7 @@ message HelloRequest {
   google.protobuf.FieldMask field_mask_value = 21;
   string field_name = 22 [json_name="json_customized_name"];
   google.protobuf.FloatValue float_value = 23;
+  string hiding_field_name = 24 [json_name="field_name"];
 }
 
 message HelloReply {

diff --git a/...oding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs b/...oding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs
@@ -337,6 +337,85 @@ public async Task HandleCallAsync_MatchingQueryStringValues_SetOnRequestMessage(
         Assert.Equal("TestSubfield!", request!.Sub.Subfield);
     }
 
+    [Fact]
+    public async Task HandleCallAsync_MatchingQueryStringValues_JsonName_SetOnRequestMessage()
+    {
+        // Arrange
+        HelloRequest? request = null;
+        UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
+        {
+            request = r;
+            return Task.FromResult(new HelloReply());
+        };
+
+        var unaryServerCallHandler = CreateCallHandler(invoker);
+        var httpContext = TestHelpers.CreateHttpContext();
+        httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
+        {
+            ["json_customized_name"] = "TestName!"
+        });
+
+        // Act
+        await unaryServerCallHandler.HandleCallAsync(httpContext);
+
+        // Assert
+        Assert.NotNull(request);
+        Assert.Equal("TestName!", request!.FieldName);
+    }
+
+    [Fact]
+    public async Task HandleCallAsync_MatchingQueryStringValues_JsonNameAndValueObject_SetOnRequestMessage()
+    {
+        // Arrange
+        HelloRequest? request = null;
+        UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
+        {
+            request = r;
+            return Task.FromResult(new HelloReply());
+        };
+
+        var unaryServerCallHandler = CreateCallHandler(invoker);
+        var httpContext = TestHelpers.CreateHttpContext();
+        httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
+        {
+            ["float_value"] = "1.1"
+        });
+
+        // Act
+        await unaryServerCallHandler.HandleCallAsync(httpContext);
+
+        // Assert
+        Assert.NotNull(request);
+        Assert.Equal(1.1f, request!.FloatValue);
+    }
+
+    [Fact]
+    public async Task HandleCallAsync_MatchingQueryStringValues_JsonNameHidesFieldName_SetOnRequestMessage()
+    {
+        // Arrange
+        HelloRequest? request = null;
+        UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
+        {
+            request = r;
+            return Task.FromResult(new HelloReply());
+        };
+
+        var unaryServerCallHandler = CreateCallHandler(invoker);
+        var httpContext = TestHelpers.CreateHttpContext();
+        httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
+        {
+            ["field_name"] = "TestName!"
+        });
+
+        // Act
+        await unaryServerCallHandler.HandleCallAsync(httpContext);
+
+        // Assert
+        Assert.NotNull(request);
+        Assert.Equal("", request!.FieldName);
+        Assert.Equal("TestName!", request!.HidingFieldName);
+    }
+
     [Fact]
     public async Task HandleCallAsync_SuccessfulResponse_DefaultValuesInResponseJson()
     {
@@ -414,7 +493,7 @@ public async Task HandleCallAsync_MalformedRequestBody_RepeatedBody_BadRequestRe
             return Task.FromResult(new HelloReply());
         };
 
-        ServiceDescriptorHelpers.TryResolveDescriptors(HelloRequest.Descriptor, new[] { "repeated_strings" }, out var bodyFieldDescriptors);
+        ServiceDescriptorHelpers.TryResolveDescriptors(HelloRequest.Descriptor, new[] { "repeated_strings" }, allowJsonName: false, out var bodyFieldDescriptors);
 
         var descriptorInfo = TestHelpers.CreateDescriptorInfo(
             bodyDescriptor: HelloRequest.Types.SubMessage.Descriptor,