gRPC transcoding: respecting json-name aliases#67108
Merged
Merged
Conversation
… aliases Fixed issue where in grpc transcoding query parameter processing overwrites route or body-bound fields. Also fixing unmatched query parameters stored in the descriptor cache
BrennanConroy
approved these changes
Jun 10, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates gRPC JSON transcoding binding behavior to better respect protobuf json_name aliases, ensuring query string parameters that target a route-bound field (via either proto name or JSON name) don’t overwrite values already bound from the route/body.
Changes:
- Prevent query-string binding from overwriting route-bound fields when the query uses the field’s JSON name alias.
- Avoid caching unresolved query-string paths in
PathDescriptorsCache(only cache successful resolutions). - Add unit + integration test coverage for alias/non-alias overwrite scenarios and for not caching unmatched query paths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/JsonRequestHelpers.cs | Updates query-string binding eligibility (body field JSON-name blocking + route JSON-path blocking) and adjusts descriptor-path caching behavior. |
| src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/CallHandlers/CallHandlerDescriptorInfo.cs | Adds storage for route-parameter JSON paths and changes the descriptor-path cache to store only non-null descriptor paths. |
| src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs | Adds unit tests for alias overwrite prevention and for not caching unmatched query-string keys. |
| src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.IntegrationTests/RouteTests.cs | Adds integration test ensuring nested JSON-name query params don’t overwrite route-bound values. |
| src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/Proto/transcoding.proto | Adds sub_data to enable coverage for body-field JSON-name prefix blocking (subData.*). |
Comment on lines
+29
to
35
| var jsonPaths = new HashSet<string>(StringComparer.Ordinal); | ||
| foreach (var routeParameter in routeParameterDescriptors.Values) | ||
| { | ||
| jsonPaths.Add(routeParameter.JsonPath); | ||
| } | ||
| RouteParameterJsonPaths = jsonPaths; | ||
| } |
Comment on lines
362
to
+375
| private static List<FieldDescriptor>? GetPathDescriptors(JsonTranscodingServerCallContext serverCallContext, IMessage requestMessage, string path) | ||
| { | ||
| return serverCallContext.DescriptorInfo.PathDescriptorsCache.GetOrAdd(path, p => | ||
| // Must not add null values for paths that don't resolve to a descriptor | ||
| var cache = serverCallContext.DescriptorInfo.PathDescriptorsCache; | ||
| if (cache.TryGetValue(path, out var pathDescriptors)) | ||
| { | ||
| ServiceDescriptorHelpers.TryResolveDescriptors(requestMessage.Descriptor, p.Split('.'), allowJsonName: true, out var pathDescriptors); | ||
| return pathDescriptors; | ||
| }); | ||
| } | ||
| if (ServiceDescriptorHelpers.TryResolveDescriptors(requestMessage.Descriptor, path.Split('.'), allowJsonName: true, out pathDescriptors)) | ||
| { | ||
| cache.TryAdd(path, pathDescriptors); | ||
| return pathDescriptors; | ||
| } | ||
| return null; |
| public async Task HandleCallAsync_QueryStringProtoName_DoesNotOverwriteRouteValue() | ||
| { | ||
| // Arrange | ||
| // A query parameter using the proto name is not overwritting the route-bound field. |
Member
Author
|
/ba-g unrelated mac failure |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.