Skip to content

Fix RequestDelegateFactory to handle nullable types correctly in empty request body scenarios #62861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

opuzakov
Copy link

Fix RequestDelegateFactory to handle nullable types correctly in empty request body scenarios

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

This change fixes an issue where RequestDelegateFactory would incorrectly handle nullable parameter types when the request body is empty (aligning behavior with the controller-based API)

The changes include:

  • Add nullable type detection
  • Additional test coverage for nullable parameter types with an empty request body

Fixes #57055

@Copilot Copilot AI review requested due to automatic review settings July 22, 2025 14:34
@github-actions github-actions bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Jul 22, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 22, 2025
Copy link
Contributor

Thanks for your PR, @@opuzakov. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

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 an issue where RequestDelegateFactory incorrectly handled nullable parameter types when the request body is empty, ensuring behavior consistency with controller-based APIs.

  • Adds nullable type detection to distinguish between regular value types and nullable value types
  • Updates the default value assignment logic to leave nullable types as null when the request body is empty
  • Includes comprehensive test coverage for the nullable parameter scenario

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
RequestDelegateFactory.cs Adds nullable type detection and fixes default value assignment logic for empty request bodies
RequestDelegateFactoryTests.cs Adds test case to verify nullable parameters are handled correctly with empty request bodies

Comment on lines +1379 to +1380

if (allowEmptyRequestBody && bodyType.IsValueType && !isNullableType) // Nullable types should be left null
Copy link
Preview

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider extracting the complex condition into a well-named local variable or method to improve readability. For example: var shouldCreateDefaultValueType = allowEmptyRequestBody && bodyType.IsValueType && !isNullableType;

Suggested change
if (allowEmptyRequestBody && bodyType.IsValueType && !isNullableType) // Nullable types should be left null
var shouldCreateDefaultValueType = allowEmptyRequestBody && bodyType.IsValueType && !isNullableType;
if (shouldCreateDefaultValueType) // Nullable types should be left null

Copilot uses AI. Check for mistakes.

@opuzakov
Copy link
Author

@dotnet-policy-service agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FromBody] attribute with an empty body request should set the default value of nullable value types to null.
1 participant