Skip to content

Conversation

desjoerd
Copy link
Contributor

When a route parameter does not have a bound model it would case a null reference exception because the ModelMetadata on the ParameterDescriptor was null. This is fixed by adding a null reference check

Fixes #63757

@Copilot Copilot AI review requested due to automatic review settings September 29, 2025 15:32
@desjoerd desjoerd requested review from a team and captainsafia as code owners September 29, 2025 15:32
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a null reference exception in the OpenAPI source generator that occurs when processing route parameters that don't have bound models. The fix adds a null check for ModelMetadata before accessing its properties.

  • Adds null safety check for ModelMetadata in XML comment transformer generation
  • Updates all generated test snapshots to reflect the fix
  • Includes a new test case to verify the fix works correctly

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

File Description
src/OpenApi/gen/XmlCommentGenerator.Emitter.cs Adds null check before accessing ModelMetadata properties
src/OpenApi/test/.../OperationTests.Controllers.cs Adds test case for unused route parameters scenario
Multiple snapshot files Updates generated code snapshots to include the null check

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Sep 29, 2025
@github-actions github-actions bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 29, 2025
@desjoerd
Copy link
Contributor Author

desjoerd commented Oct 7, 2025

@captainsafia could you review this? Maybe it's still possible to backport this fix into .NET 10. Also when thinking of fixing potential review comments 😊.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Oct 14, 2025
@captainsafia
Copy link
Member

/azp run

@dotnet-policy-service dotnet-policy-service bot removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Oct 16, 2025
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@captainsafia captainsafia enabled auto-merge (squash) October 16, 2025 09:29
@desjoerd
Copy link
Contributor Author

I see that one of the snapshot tests is failing, will check this evening!

…ment transformers

When a route parameter does not have a bound model it would case a null reference exception because the ModelMetadata on the ParameterDescriptor was null.
This is fixed by adding a null reference check

Fixes dotnet#63757
As I think this was the encoding before my change
auto-merge was automatically disabled October 16, 2025 15:30

Head branch was pushed to by a user without write access

@desjoerd
Copy link
Contributor Author

@captainsafia the build is green, the problem was some whitespace in one of the snapshots 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates community-contribution Indicates that the PR has been added by a community member feature-openapi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XmlCommentOperationTransformer null reference exception

3 participants