From b37370464f5be87c4adbbb66a04b51006222e176 Mon Sep 17 00:00:00 2001 From: Alon Kolyakov Date: Tue, 17 Mar 2026 23:49:22 +0200 Subject: [PATCH 1/4] Treat empty query strings as null for nullable value types When binding query parameters like ?myBool= (empty value) to nullable value types (bool?, int?, etc.), the framework was attempting to parse the empty string and failing with a 500 error. Controllers handle this correctly by treating empty query values as null for nullable parameters. Minimal APIs should do the same. The fix: for nullable value types with FromQuery binding, check for empty strings before attempting TryParse. Empty strings are skipped, leaving the parameter at its default value (null). Changes: - RequestDelegateFactory: Use NotNullOrEmpty check for nullable value types instead of NotNull - Add tests for bool? and int? with empty query string values Fixes #65754 --- .../src/RequestDelegateFactory.cs | 11 ++++- ...stDelegateCreationTests.QueryParameters.cs | 40 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index c51e7aae5a36..b836973d4fdd 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1798,9 +1798,16 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres Expression.Assign(parameter.ParameterType.IsArray ? Expression.ArrayAccess(argument, index) : argument, Expression.Convert(parsedValue, targetParseType)), failBlock)); + // For nullable value types, treat empty strings as null (don't attempt to parse them). + // This matches controller behavior and avoids parse failures on ?param= (empty value). + var isNullableValueType = Nullable.GetUnderlyingType(parameter.ParameterType) != null; + var sourceCheckExpr = isOptional && isNullableValueType + ? TempSourceStringIsNotNullOrEmptyExpr + : TempSourceStringNotNullExpr; + var ifNotNullTryParse = !parameter.HasDefaultValue - ? Expression.IfThen(TempSourceStringNotNullExpr, tryParseExpression) - : Expression.IfThenElse(TempSourceStringNotNullExpr, tryParseExpression, + ? Expression.IfThen(sourceCheckExpr, tryParseExpression) + : Expression.IfThenElse(sourceCheckExpr, tryParseExpression, Expression.Assign(argument, CreateDefaultValueExpression(parameter.DefaultValue, parameter.ParameterType))); diff --git a/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.QueryParameters.cs b/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.QueryParameters.cs index 41dde7279e17..e9b8d4202d28 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.QueryParameters.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.QueryParameters.cs @@ -180,4 +180,44 @@ public async Task MapAction_ExplicitQueryParam_NameTest(string name, string look await endpoint.RequestDelegate(httpContext); await VerifyResponseBodyAsync(httpContext, "test", 200); } + + [Fact] + public async Task MapAction_NullableBoolParam_WithEmptyQueryStringValue_SetsNull() + { + // Regression test for https://github.com/dotnet/aspnetcore/issues/65754 + // ?myBool= should bind to null for bool? parameter, not throw. + var (results, compilation) = await RunGeneratorAsync(""" +app.MapGet("/test", ([FromQuery] bool? myBool) => myBool.HasValue ? $"Value: {myBool.Value}" : "null"); +"""); + var endpoint = GetEndpointFromCompilation(compilation); + + VerifyStaticEndpointModel(results, endpointModel => + { + Assert.Equal("MapGet", endpointModel.HttpMethod); + var p = Assert.Single(endpointModel.Parameters); + Assert.Equal(EndpointParameterSource.Query, p.Source); + Assert.Equal("myBool", p.SymbolName); + }); + + var httpContext = CreateHttpContext(); + httpContext.Request.QueryString = new QueryString("?myBool="); + + await endpoint.RequestDelegate(httpContext); + await VerifyResponseBodyAsync(httpContext, "null"); + } + + [Fact] + public async Task MapAction_NullableIntParam_WithEmptyQueryStringValue_SetsNull() + { + var (results, compilation) = await RunGeneratorAsync(""" +app.MapGet("/test", ([FromQuery] int? value) => value.HasValue ? $"{value.Value}" : "null"); +"""); + var endpoint = GetEndpointFromCompilation(compilation); + + var httpContext = CreateHttpContext(); + httpContext.Request.QueryString = new QueryString("?value="); + + await endpoint.RequestDelegate(httpContext); + await VerifyResponseBodyAsync(httpContext, "null"); + } } From a7360805bb949f5607e6e7cdc2b365c07d5c31fb Mon Sep 17 00:00:00 2001 From: BloodShop Date: Wed, 18 Mar 2026 12:46:03 +0200 Subject: [PATCH 2/4] fix: resolve type mismatch in nullable query string binding The conditional expression was mixing UnaryExpression and BinaryExpression types, which caused CS0173 compiler error. Cast to common Expression type to fix. Also removed incorrect 'isOptional' check - nullable value types should always treat empty query strings as null, not just when optional. --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index b836973d4fdd..a5c9cbbc5975 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1801,9 +1801,9 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres // For nullable value types, treat empty strings as null (don't attempt to parse them). // This matches controller behavior and avoids parse failures on ?param= (empty value). var isNullableValueType = Nullable.GetUnderlyingType(parameter.ParameterType) != null; - var sourceCheckExpr = isOptional && isNullableValueType + var sourceCheckExpr = (Expression)(isNullableValueType ? TempSourceStringIsNotNullOrEmptyExpr - : TempSourceStringNotNullExpr; + : TempSourceStringNotNullExpr); var ifNotNullTryParse = !parameter.HasDefaultValue ? Expression.IfThen(sourceCheckExpr, tryParseExpression) From 8dfa9ef47b1669c26d3e1959470431c02c4e9fd0 Mon Sep 17 00:00:00 2001 From: BloodShop Date: Wed, 18 Mar 2026 13:01:55 +0200 Subject: [PATCH 3/4] test: add regression test for OpenAPI request body comment bug (issue #65805) When an endpoint has multiple parameters including [FromBody] and other injected dependencies, the request body description should use the [FromBody] parameter's XML comment, not another parameter's comment. Example: PostData([FromBody] SomeData data, ILogger logger, CancellationToken ct) Expected: description from 'data' parameter Actual: description from 'ct' parameter (incorrectly) --- test_issue_65805.cs | 70 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 test_issue_65805.cs diff --git a/test_issue_65805.cs b/test_issue_65805.cs new file mode 100644 index 000000000000..03b73f33f9d7 --- /dev/null +++ b/test_issue_65805.cs @@ -0,0 +1,70 @@ +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.OpenApi; +using Microsoft.OpenApi.Models; +using Xunit; + +namespace Microsoft.AspNetCore.OpenApi.Tests; + +public class RequestBodyCommentTests +{ + [Fact] + public async Task RequestBodyDescriptionUsesCorrectParameterComment() + { + // Regression test for issue #65805 + // When a method has multiple parameters including [FromBody] and [FromServices], + // the request body description should use the [FromBody] parameter's XML comment, + // not the last parameter's comment. + + var app = CreateMinimalApp(); + var openApiDocument = await GetOpenApiDocument(app); + + var postFooOperation = openApiDocument.Paths["/foo"].Operations[OperationType.Post]; + var requestBody = postFooOperation.RequestBody; + + // The request body description should be from the [FromBody] SomeData parameter + Assert.NotNull(requestBody); + Assert.Equal("Sample data provided by the user.", requestBody.Description); + // Should NOT be "Injected cancellation token." (from the CancellationToken parameter) + } + + private WebApplication CreateMinimalApp() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.AddOpenApi(); + + var app = builder.Build(); + app.MapOpenApi(); + + // Map the endpoint from issue #65805 + app.MapPost("/foo", PostSampleData) + .WithName("PostSampleData") + .WithOpenApi(); + + return app; + } + + // This mirrors the reproduction case from issue #65805 + /// + /// Process some sample input. + /// + /// Sample data provided by the user. + /// Logger for diagnostics and tracing. + /// Injected cancellation token. + /// The number the user supplied. + public record SomeData(int Number, string Text); + + public static IResult PostSampleData( + [FromBody] SomeData data, + [FromServices] ILogger logger, + CancellationToken cancellation) + { + ArgumentNullException.ThrowIfNull(data); + return Results.Ok(data.Number); + } + + private async Task GetOpenApiDocument(WebApplication app) + { + var openApiDocumentProvider = app.Services.GetRequiredService(); + return await openApiDocumentProvider.GetOpenApiDocumentAsync(app.Services); + } +} From d3e7a95906e1c74bc40781ca018126e74ddc9c29 Mon Sep 17 00:00:00 2001 From: BloodShop Date: Sun, 22 Mar 2026 16:17:23 +0200 Subject: [PATCH 4/4] fix: add comprehensive tests for nullable query string handling and cleanup - Add 4 new tests to verify empty query strings are treated as null for nullable types - Test both bool? and int? with empty strings and valid values - Tests use RequestDelegateFactory.Create() to exercise the runtime path (not just source generator) - Remove unrelated test_issue_65805.cs file that was committed by mistake - These tests verify the fix for issue #65754 works correctly --- .../test/RequestDelegateFactoryTests.cs | 84 +++++++++++++++++++ test_issue_65805.cs | 70 ---------------- 2 files changed, 84 insertions(+), 70 deletions(-) delete mode 100644 test_issue_65805.cs diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 61e7d2a46965..485b4fae635a 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -263,6 +263,90 @@ public async Task NullRouteParametersPrefersRouteOverQueryString() Assert.Equal(42, httpContext.Items["input"]); } + [Fact] + public async Task EmptyQueryStringValueTreatedAsNullForNullableBool() + { + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create((bool? value, HttpContext httpContext) => + { + httpContext.Items["value"] = value; + }); + + httpContext.Request.Query = new QueryCollection(new Dictionary + { + ["value"] = "" // Empty string should be treated as null + }); + + var requestDelegate = factoryResult.RequestDelegate; + await requestDelegate(httpContext); + + Assert.Null(httpContext.Items["value"]); + } + + [Fact] + public async Task EmptyQueryStringValueTreatedAsNullForNullableInt() + { + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create((int? value, HttpContext httpContext) => + { + httpContext.Items["value"] = value; + }); + + httpContext.Request.Query = new QueryCollection(new Dictionary + { + ["value"] = "" // Empty string should be treated as null + }); + + var requestDelegate = factoryResult.RequestDelegate; + await requestDelegate(httpContext); + + Assert.Null(httpContext.Items["value"]); + } + + [Fact] + public async Task NonEmptyQueryStringValueParsedCorrectlyForNullableBool() + { + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create((bool? value, HttpContext httpContext) => + { + httpContext.Items["value"] = value; + }); + + httpContext.Request.Query = new QueryCollection(new Dictionary + { + ["value"] = "true" + }); + + var requestDelegate = factoryResult.RequestDelegate; + await requestDelegate(httpContext); + + Assert.True((bool?)httpContext.Items["value"]); + } + + [Fact] + public async Task NonEmptyQueryStringValueParsedCorrectlyForNullableInt() + { + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create((int? value, HttpContext httpContext) => + { + httpContext.Items["value"] = value; + }); + + httpContext.Request.Query = new QueryCollection(new Dictionary + { + ["value"] = "42" + }); + + var requestDelegate = factoryResult.RequestDelegate; + await requestDelegate(httpContext); + + Assert.Equal(42, (int?)httpContext.Items["value"]); + } + [Fact] public async Task CreatingDelegateWithInstanceMethodInfoCreatesInstancePerCall() { diff --git a/test_issue_65805.cs b/test_issue_65805.cs deleted file mode 100644 index 03b73f33f9d7..000000000000 --- a/test_issue_65805.cs +++ /dev/null @@ -1,70 +0,0 @@ -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.OpenApi; -using Microsoft.OpenApi.Models; -using Xunit; - -namespace Microsoft.AspNetCore.OpenApi.Tests; - -public class RequestBodyCommentTests -{ - [Fact] - public async Task RequestBodyDescriptionUsesCorrectParameterComment() - { - // Regression test for issue #65805 - // When a method has multiple parameters including [FromBody] and [FromServices], - // the request body description should use the [FromBody] parameter's XML comment, - // not the last parameter's comment. - - var app = CreateMinimalApp(); - var openApiDocument = await GetOpenApiDocument(app); - - var postFooOperation = openApiDocument.Paths["/foo"].Operations[OperationType.Post]; - var requestBody = postFooOperation.RequestBody; - - // The request body description should be from the [FromBody] SomeData parameter - Assert.NotNull(requestBody); - Assert.Equal("Sample data provided by the user.", requestBody.Description); - // Should NOT be "Injected cancellation token." (from the CancellationToken parameter) - } - - private WebApplication CreateMinimalApp() - { - var builder = WebApplication.CreateBuilder(); - builder.Services.AddOpenApi(); - - var app = builder.Build(); - app.MapOpenApi(); - - // Map the endpoint from issue #65805 - app.MapPost("/foo", PostSampleData) - .WithName("PostSampleData") - .WithOpenApi(); - - return app; - } - - // This mirrors the reproduction case from issue #65805 - /// - /// Process some sample input. - /// - /// Sample data provided by the user. - /// Logger for diagnostics and tracing. - /// Injected cancellation token. - /// The number the user supplied. - public record SomeData(int Number, string Text); - - public static IResult PostSampleData( - [FromBody] SomeData data, - [FromServices] ILogger logger, - CancellationToken cancellation) - { - ArgumentNullException.ThrowIfNull(data); - return Results.Ok(data.Number); - } - - private async Task GetOpenApiDocument(WebApplication app) - { - var openApiDocumentProvider = app.Services.GetRequiredService(); - return await openApiDocumentProvider.GetOpenApiDocumentAsync(app.Services); - } -}