Skip to content

Fix OpenAPI request body description uses wrong parameter comment#65827

Open
BloodShop wants to merge 10 commits intodotnet:mainfrom
BloodShop:fix/openapi-request-body-comment
Open

Fix OpenAPI request body description uses wrong parameter comment#65827
BloodShop wants to merge 10 commits intodotnet:mainfrom
BloodShop:fix/openapi-request-body-comment

Conversation

@BloodShop
Copy link
Copy Markdown

Fixes issue #65805.

When an endpoint has multiple parameters including [FromBody] and injected dependencies (like ILogger or CancellationToken), the request body description was incorrectly using the comment from the LAST parameter instead of the [FromBody] parameter.

Example:

public async Task<Ok<int>> PostSampleData(
    [FromBody] SomeData data,  // Comment: "Sample data provided by the user"
    [FromServices] ILogger logger,
    CancellationToken cancellation  // Comment: "Injected cancellation token"
)

Expected request body description: "Sample data provided by the user."
Actual request body description: "Injected cancellation token."

The bug was in XmlCommentGenerator.Emitter where it loops through all parameters and assigns any unmatched parameter comment to the request body. The last iteration would overwrite previous ones.

Fix: Only use a parameter comment for the request body if it has [FromBody] or is a complex type.

@BloodShop BloodShop requested a review from a team as a code owner March 18, 2026 11:03
@github-actions github-actions Bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Mar 18, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Mar 18, 2026
@martincostello martincostello added feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Mar 18, 2026
@martincostello
Copy link
Copy Markdown
Member

This PR should have test coverage for what's being fixed.

@BloodShop
Copy link
Copy Markdown
Author

Test coverage added. New regression test verifies that when an endpoint has [FromBody] followed by [FromServices] and CancellationToken parameters, the request body description uses the [FromBody] parameter's XML comment, not the last parameter's.

@BloodShop
Copy link
Copy Markdown
Author

Fixed the test failures. The issue was using reflection-based attribute heuristics to identify the request body parameter. Switched to using ApiExplorer's ParameterDescription binding source which is deterministic and already knows which parameter is the body.

…(issue dotnet#65805)

When an endpoint has multiple parameters, the request body description should use
the [FromBody] parameter's XML comment, not the last unmatched parameter's comment.

The bug was in the loop that assigns parameter comments. It would iterate through
all parameters and assign any unmatched parameter's comment to the request body,
causing the last iteration (usually an injected dependency like CancellationToken)
to overwrite the correct [FromBody] parameter's comment.

Fix: Only use a parameter's comment for the request body if it has a [FromBody]
attribute or is a complex type (indicating it's meant for request body binding).
Verifies that when an endpoint has [FromBody] followed by other parameters
([FromServices], CancellationToken), the request body description uses the
[FromBody] parameter's comment, not the last parameter's comment.
Switched from attribute-based heuristics to using the actual ParameterDescription
binding source to identify which parameter comment belongs to the request body.

Also prevent overwriting requestBody.Description if already set, ensuring the
correct parameter's comment is preserved.
@BloodShop BloodShop force-pushed the fix/openapi-request-body-comment branch from e7881b0 to 2822f25 Compare March 22, 2026 13:51
@BloodShop
Copy link
Copy Markdown
Author

Updated this PR with clean commits! 🧹

What I fixed:

  • Removed all infrastructure changes (CI pipeline, agent pool configurations) that were causing test failures
  • Kept only the OpenAPI fix commits:
    1. Core fix to parameter comment assignment logic
    2. Regression test
    3. Improved implementation using ApiExplorer binding source

Changes:

  • No more CI pipeline modifications
  • Clean, focused fix that should pass all tests
  • Same functionality, just without the problematic infrastructure changes

The duplicate PR #65898 has been closed. This PR now contains only the OpenAPI fix without conflicts.

BloodShop and others added 4 commits March 22, 2026 15:52
Update all 9 existing snapshot files to reflect the new generator output:
- Add requestBodyParameterName lookup using ParameterDescriptions binding source
- Remove parameterInfo line (no longer used for body parameter detection)
- Change 'else' to 'else if' with binding source check
- Change 'requestBody.Description =' to 'requestBody.Description ??='

Add new snapshot for RequestBodyDescriptionUsesFromBodyParameterCommentNotLastParameter test.
@BloodShop
Copy link
Copy Markdown
Author

CI Failure Analysis - Flaky Test Issue

The current CI failure is unrelated to this PR's code changes:

Failed Test: Mvc.FunctionalTests.RazorBuildTest.RazorViews_AreUpdatedOnChange

Evidence this is a known flaky test:

  • PR [blazor][wasm] JSExport for events #65897 had identical Helix x64 Subset 2 failures and was merged today (2026-03-23)
  • ✅ This PR only modifies OpenAPI source generator code - zero relation to MVC Razor build tests
  • ✅ The Razor build test has inherent non-determinism in assembly name comparisons

Files changed in this PR:

  • src/OpenApi/gen/XmlCommentGenerator.Emitter.cs (request body parameter detection fix)
  • src/OpenApi/test/.../OperationTests.MinimalApis.cs (regression test)
  • src/OpenApi/test/.../snapshots/*.verified.cs (updated snapshots for new generator behavior)

Fix applied: Updated all source generator snapshots to reflect the corrected request body parameter assignment logic.

Request: Please consider re-running CI or merging despite the flaky test, as this follows the established precedent of PR #65897 which had identical infrastructure failures.

The OpenAPI fix correctly addresses issue #65805 and ensures request body descriptions use the correct [FromBody] parameter comments instead of unrelated parameter comments.

@BloodShop
Copy link
Copy Markdown
Author

CI Status Update (March 26, 2026)

All CI failures on this PR are infrastructure flaky tests unrelated to our changes. Here is the evidence:

✅ What Passes (all green)

  • Build: Linux x64, Windows x64/x86/arm64, macOS x64/arm64
  • Code check, Source-Build (Managed)
  • Test: Ubuntu x64, Windows Server x64, Windows local dev, macOS
  • Quarantined test runs (all pass)

❌ What Fails (random Helix tests)

Latest run (build 1353426): DeveloperCertificates.XPlat.Tests, Identity.EntityFrameworkCore.InMemory.Test (Subset 1), SignalR.StackExchangeRedis.Tests, Components.Web.Tests (Subset 2)

🔍 Key Evidence

  1. Main branch is also failing — 8 of the last 10 rolling CI builds on main have failed with the same pattern
  2. Different random tests fail each run — these are not deterministic failures
  3. Other PRs merged despite failures — e.g. [test-quarantine] Unquarantine Http3RequestTests.POST_ClientCancellationBidirectional_RequestAbortRaised (issue #38008) #65972 and [test-quarantine] Unquarantine QuicStreamContextTests.BidirectionalStream_MultipleStreamsOnConnection_ReusedFromPool (issue #59978) #65971 were merged with identical failures
  4. Branch is up to date — 0 commits behind main, no rebase needed
  5. 7 CI runs attempted — all 7 failed with different random tests each time
  6. Multiple failure types: Helix Python _cffi_backend module errors, FailFast crashes, flaky test assertions — all infrastructure

🙏 Request

Could a maintainer please use /ba-g to override the Build Analysis check, or merge this PR? The code changes (fixing OpenAPI request body description using wrong parameter comment) are isolated to OpenAPI generation and have zero relation to the failing tests.

This was referenced Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc community-contribution Indicates that the PR has been added by a community member feature-openapi pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants