Skip to content

Commit ce5e4a8

Browse files
github-actions[bot]JamesNK
github-actions[bot]
and
JamesNK
authored
[release/7.0] gRPC JSON transcoding: Fix not supporting JSON name in querystring names (#46750)
* Fix not supporting JSON name in gRPC JSON transcoding query parameters * Clean up * Optimize * Comment * Comment * Improve test * Fix tests * Update src/Grpc/JsonTranscoding/src/Shared/ServiceDescriptorHelpers.cs --------- Co-authored-by: James Newton-King <[email protected]>
1 parent 836e0f0 commit ce5e4a8

File tree

6 files changed

+135
-12
lines changed

6 files changed

+135
-12
lines changed

‎src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Binding/JsonTranscodingProviderServiceBinder.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Diagnostics.CodeAnalysis;
5-
using System.Linq;
65
using Google.Api;
76
using Google.Protobuf.Reflection;
87
using Grpc.AspNetCore.Server;

‎src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/JsonRequestHelpers.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ private static async ValueTask<byte[]> ReadDataAsync(JsonTranscodingServerCallCo
338338
{
339339
return serverCallContext.DescriptorInfo.PathDescriptorsCache.GetOrAdd(path, p =>
340340
{
341-
ServiceDescriptorHelpers.TryResolveDescriptors(requestMessage.Descriptor, p.Split('.'), out var pathDescriptors);
341+
ServiceDescriptorHelpers.TryResolveDescriptors(requestMessage.Descriptor, p.Split('.'), allowJsonName: true, out var pathDescriptors);
342342
return pathDescriptors;
343343
});
344344
}

‎src/Grpc/JsonTranscoding/src/Shared/ServiceDescriptorHelpers.cs

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,26 +63,30 @@ internal static bool IsWrapperType(MessageDescriptor m) =>
6363
throw new InvalidOperationException($"Get not find Descriptor property on {serviceReflectionType.Name}.");
6464
}
6565

66-
public static bool TryResolveDescriptors(MessageDescriptor messageDescriptor, IList<string> path, [NotNullWhen(true)]out List<FieldDescriptor>? fieldDescriptors)
66+
public static bool TryResolveDescriptors(MessageDescriptor messageDescriptor, IList<string> path, bool allowJsonName, [NotNullWhen(true)]out List<FieldDescriptor>? fieldDescriptors)
6767
{
6868
fieldDescriptors = null;
6969
MessageDescriptor? currentDescriptor = messageDescriptor;
7070

7171
foreach (var fieldName in path)
7272
{
73-
var field = currentDescriptor?.FindFieldByName(fieldName);
74-
if (field == null)
73+
FieldDescriptor? field = null;
74+
if (currentDescriptor != null)
7575
{
76-
fieldDescriptors = null;
77-
return false;
76+
field = allowJsonName
77+
? GetFieldByName(currentDescriptor, fieldName)
78+
: currentDescriptor.FindFieldByName(fieldName);
7879
}
7980

80-
if (fieldDescriptors == null)
81+
if (field == null)
8182
{
82-
fieldDescriptors = new List<FieldDescriptor>();
83+
fieldDescriptors = null;
84+
return false;
8385
}
8486

87+
fieldDescriptors ??= new List<FieldDescriptor>();
8588
fieldDescriptors.Add(field);
89+
8690
if (field.FieldType == FieldType.Message)
8791
{
8892
currentDescriptor = field.MessageType;
@@ -96,6 +100,34 @@ public static bool TryResolveDescriptors(MessageDescriptor messageDescriptor, IL
96100
return fieldDescriptors != null;
97101
}
98102

103+
private static FieldDescriptor? GetFieldByName(MessageDescriptor messageDescriptor, string fieldName)
104+
{
105+
// Search fields by field name and JSON name. Both names can be referenced.
106+
// If there are conflicts, then the last field with a name wins.
107+
// This logic matches how properties are used in JSON serialization's MessageTypeInfoResolver.
108+
var fields = messageDescriptor.Fields.InFieldNumberOrder();
109+
110+
FieldDescriptor? fieldDescriptor = null;
111+
for (var i = fields.Count - 1; i >= 0; i--)
112+
{
113+
// We're checking JSON name first, in reverse order through fields.
114+
// That means the method can exit early on match because the match has the highest precedence.
115+
var field = fields[i];
116+
if (field.JsonName == fieldName)
117+
{
118+
fieldDescriptor = field;
119+
break;
120+
}
121+
else if (field.Name == fieldName)
122+
{
123+
fieldDescriptor = field;
124+
break;
125+
}
126+
}
127+
128+
return fieldDescriptor;
129+
}
130+
99131
private static object? ConvertValue(object? value, FieldDescriptor descriptor)
100132
{
101133
switch (descriptor.FieldType)
@@ -323,7 +355,7 @@ public static Dictionary<string, RouteParameter> ResolveRouteParameterDescriptor
323355
foreach (var variable in variables)
324356
{
325357
var path = variable.FieldPath;
326-
if (!TryResolveDescriptors(messageDescriptor, path, out var fieldDescriptors))
358+
if (!TryResolveDescriptors(messageDescriptor, path, allowJsonName: false, out var fieldDescriptors))
327359
{
328360
throw new InvalidOperationException($"Couldn't find matching field for route parameter '{string.Join(".", path)}' on {messageDescriptor.Name}.");
329361
}

‎src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,23 @@ public JsonConverterReadTests(ITestOutputHelper output)
2626
public void NonJsonName()
2727
{
2828
var json = @"{
29+
""hiding_field_name"": ""A field name""
30+
}";
31+
32+
var m = AssertReadJson<HelloRequest>(json);
33+
Assert.Equal("A field name", m.HidingFieldName);
34+
}
35+
36+
[Fact]
37+
public void HidingJsonName()
38+
{
39+
var json = @"{
2940
""field_name"": ""A field name""
3041
}";
3142

3243
var m = AssertReadJson<HelloRequest>(json);
33-
Assert.Equal("A field name", m.FieldName);
44+
Assert.Equal("", m.FieldName);
45+
Assert.Equal("A field name", m.HidingFieldName);
3446
}
3547

3648
[Fact]

‎src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/Proto/transcoding.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ message HelloRequest {
200200
google.protobuf.FieldMask field_mask_value = 21;
201201
string field_name = 22 [json_name="json_customized_name"];
202202
google.protobuf.FloatValue float_value = 23;
203+
string hiding_field_name = 24 [json_name="field_name"];
203204
}
204205

205206
message HelloReply {

‎src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,85 @@ public async Task HandleCallAsync_MatchingQueryStringValues_SetOnRequestMessage(
337337
Assert.Equal("TestSubfield!", request!.Sub.Subfield);
338338
}
339339

340+
[Fact]
341+
public async Task HandleCallAsync_MatchingQueryStringValues_JsonName_SetOnRequestMessage()
342+
{
343+
// Arrange
344+
HelloRequest? request = null;
345+
UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
346+
{
347+
request = r;
348+
return Task.FromResult(new HelloReply());
349+
};
350+
351+
var unaryServerCallHandler = CreateCallHandler(invoker);
352+
var httpContext = TestHelpers.CreateHttpContext();
353+
httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
354+
{
355+
["json_customized_name"] = "TestName!"
356+
});
357+
358+
// Act
359+
await unaryServerCallHandler.HandleCallAsync(httpContext);
360+
361+
// Assert
362+
Assert.NotNull(request);
363+
Assert.Equal("TestName!", request!.FieldName);
364+
}
365+
366+
[Fact]
367+
public async Task HandleCallAsync_MatchingQueryStringValues_JsonNameAndValueObject_SetOnRequestMessage()
368+
{
369+
// Arrange
370+
HelloRequest? request = null;
371+
UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
372+
{
373+
request = r;
374+
return Task.FromResult(new HelloReply());
375+
};
376+
377+
var unaryServerCallHandler = CreateCallHandler(invoker);
378+
var httpContext = TestHelpers.CreateHttpContext();
379+
httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
380+
{
381+
["float_value"] = "1.1"
382+
});
383+
384+
// Act
385+
await unaryServerCallHandler.HandleCallAsync(httpContext);
386+
387+
// Assert
388+
Assert.NotNull(request);
389+
Assert.Equal(1.1f, request!.FloatValue);
390+
}
391+
392+
[Fact]
393+
public async Task HandleCallAsync_MatchingQueryStringValues_JsonNameHidesFieldName_SetOnRequestMessage()
394+
{
395+
// Arrange
396+
HelloRequest? request = null;
397+
UnaryServerMethod<JsonTranscodingGreeterService, HelloRequest, HelloReply> invoker = (s, r, c) =>
398+
{
399+
request = r;
400+
return Task.FromResult(new HelloReply());
401+
};
402+
403+
var unaryServerCallHandler = CreateCallHandler(invoker);
404+
var httpContext = TestHelpers.CreateHttpContext();
405+
httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
406+
{
407+
["field_name"] = "TestName!"
408+
});
409+
410+
// Act
411+
await unaryServerCallHandler.HandleCallAsync(httpContext);
412+
413+
// Assert
414+
Assert.NotNull(request);
415+
Assert.Equal("", request!.FieldName);
416+
Assert.Equal("TestName!", request!.HidingFieldName);
417+
}
418+
340419
[Fact]
341420
public async Task HandleCallAsync_SuccessfulResponse_DefaultValuesInResponseJson()
342421
{
@@ -414,7 +493,7 @@ public async Task HandleCallAsync_MalformedRequestBody_RepeatedBody_BadRequestRe
414493
return Task.FromResult(new HelloReply());
415494
};
416495

417-
ServiceDescriptorHelpers.TryResolveDescriptors(HelloRequest.Descriptor, new[] { "repeated_strings" }, out var bodyFieldDescriptors);
496+
ServiceDescriptorHelpers.TryResolveDescriptors(HelloRequest.Descriptor, new[] { "repeated_strings" }, allowJsonName: false, out var bodyFieldDescriptors);
418497

419498
var descriptorInfo = TestHelpers.CreateDescriptorInfo(
420499
bodyDescriptor: HelloRequest.Types.SubMessage.Descriptor,

0 commit comments

Comments
 (0)