From ff0e49ca64f39a5501f52a2adf51879d773fd407 Mon Sep 17 00:00:00 2001 From: ladeak Date: Sat, 7 Jun 2025 09:44:48 +0200 Subject: [PATCH 1/5] Enhance HTTP/3 request body detection and testing - Updated `IHttpRequestBodyDetectionFeature` to include HTTP/3 descriptions for the `END_STREAM` flag. - Modified request body handling logic to return zero length content body instance when there endstream flag is set. It also completes the message body for empty requests, so that the RequestBodyPipe.Reader is closed (needed when stream is reused by the pool and the Pipe is reset). - Added unit test `CanHaveBody_ReturnsFalseWithoutRequestBody` to validate behavior when no request body is present. --- .../src/IHttpRequestBodyDetectionFeature.cs | 6 +++++ .../Core/src/Internal/Http3/Http3Stream.cs | 20 +++++++++++++- .../Http3/Http3StreamTests.cs | 26 +++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/Http/Http.Features/src/IHttpRequestBodyDetectionFeature.cs b/src/Http/Http.Features/src/IHttpRequestBodyDetectionFeature.cs index 0e02daa33063..55a98f4eeebd 100644 --- a/src/Http/Http.Features/src/IHttpRequestBodyDetectionFeature.cs +++ b/src/Http/Http.Features/src/IHttpRequestBodyDetectionFeature.cs @@ -20,6 +20,9 @@ public interface IHttpRequestBodyDetectionFeature /// /// It's an HTTP/2 request that did not set the END_STREAM flag on the initial headers frame. /// + /// + /// It's an HTTP/3 request that did not set the END_STREAM flag on the initial headers frame. + /// /// /// The final request body length may still be zero for the chunked or HTTP/2 scenarios. /// @@ -35,6 +38,9 @@ public interface IHttpRequestBodyDetectionFeature /// /// It's an HTTP/2 request that set END_STREAM on the initial headers frame. /// + /// + /// It's an HTTP/3 request that set END_STREAM on the initial headers frame. + /// /// /// /// When false, the request body should never return data. diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index 36c87cd8ef7b..0439c5a66ffb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -54,6 +54,7 @@ internal abstract partial class Http3Stream : HttpProtocol, IHttp3Stream, IHttpS private bool _isMethodConnect; private bool _isWebTransportSessionAccepted; private Http3MessageBody? _messageBody; + private bool _requestBodyStarted; private readonly ManualResetValueTaskSource _appCompletedTaskSource = new(); private readonly Lock _completionLock = new(); @@ -65,6 +66,17 @@ internal abstract partial class Http3Stream : HttpProtocol, IHttp3Stream, IHttpS private bool IsAbortedRead => (_completionState & StreamCompletionFlags.AbortedRead) == StreamCompletionFlags.AbortedRead; public bool IsCompleted => (_completionState & StreamCompletionFlags.Completed) == StreamCompletionFlags.Completed; + public bool ReceivedEmptyRequestBody + { + get + { + lock (_completionLock) + { + return EndStreamReceived && !_requestBodyStarted; + } + } + } + public Pipe RequestBodyPipe { get; private set; } = default!; public long? InputRemaining { get; internal set; } public QPackDecoder QPackDecoder { get; private set; } = default!; @@ -928,7 +940,7 @@ private Task ProcessDataFrameAsync(in ReadOnlySequence payload) { return Task.CompletedTask; } - + _requestBodyStarted = true; foreach (var segment in payload) { RequestBodyPipe.Writer.Write(segment.Span); @@ -975,6 +987,12 @@ protected override MessageBody CreateMessageBody() _messageBody = new Http3MessageBody(this); } + if (ReceivedEmptyRequestBody) + { + _messageBody.Complete(exception: null); + return MessageBody.ZeroContentLengthClose; + } + return _messageBody; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs index b7697117452e..eff3b384b784 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs @@ -3291,6 +3291,32 @@ public async Task Control_FrameParsingSingleByteAtATimeWorks() await outboundcontrolStream.ReceiveEndAsync(); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task CanHaveBody_ReturnsFalseWithoutRequestBody(bool endstream) + { + var headers = new[] + { + new KeyValuePair(InternalHeaderNames.Method, "GET"), + new KeyValuePair(InternalHeaderNames.Path, "/"), + new KeyValuePair(InternalHeaderNames.Scheme, "http"), + new KeyValuePair(InternalHeaderNames.Authority, "localhost:80"), + }; + + var requestStream = await Http3Api.InitializeConnectionAndStreamsAsync(context => + { + Assert.Equal(!endstream, context.Features.Get().CanHaveBody); + context.Response.StatusCode = 200; + return Task.CompletedTask; + }, headers, endStream: endstream); + + var responseHeaders = await requestStream.ExpectHeadersAsync(); + Assert.Equal("200", responseHeaders[InternalHeaderNames.Status]); + + await requestStream.ExpectReceiveEndOfStream(); + } + private async Task WriteOneByteAtATime(PipeReader reader, PipeWriter writer) { var res = await reader.ReadAsync(); From a6be382e8ead29967f8bed696920c6616aad299b Mon Sep 17 00:00:00 2001 From: ladeak Date: Tue, 10 Jun 2025 22:44:48 +0200 Subject: [PATCH 2/5] review feedback - http3 and http2 differed on CompleteStream (the reader was not closed in http3) causing Reset to throw when stream was re-used. --- .../Kestrel/Core/src/Internal/Http3/Http3Stream.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index 0439c5a66ffb..732604d7a825 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -571,7 +571,7 @@ private void CompleteStream(bool errored) TryClose(); } - + RequestBodyPipe.Reader.Complete(); _http3Output.Complete(); // Stream will be pooled after app completed. @@ -978,6 +978,11 @@ protected override string CreateRequestId() protected override MessageBody CreateMessageBody() { + if (ReceivedEmptyRequestBody) + { + return MessageBody.ZeroContentLengthClose; + } + if (_messageBody != null) { _messageBody.Reset(); @@ -987,12 +992,6 @@ protected override MessageBody CreateMessageBody() _messageBody = new Http3MessageBody(this); } - if (ReceivedEmptyRequestBody) - { - _messageBody.Complete(exception: null); - return MessageBody.ZeroContentLengthClose; - } - return _messageBody; } From 5024214f03dc66e700edca8d9f9b9e2500cd4fc0 Mon Sep 17 00:00:00 2001 From: ladeak Date: Wed, 2 Jul 2025 09:16:21 +0200 Subject: [PATCH 3/5] Update src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs Co-authored-by: Brennan --- src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index 732604d7a825..ef86a5936829 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -940,7 +940,9 @@ private Task ProcessDataFrameAsync(in ReadOnlySequence payload) { return Task.CompletedTask; } + _requestBodyStarted = true; + foreach (var segment in payload) { RequestBodyPipe.Writer.Write(segment.Span); From 49f6791ea40cbbb4e17716586545c41e2c5ce4f8 Mon Sep 17 00:00:00 2001 From: ladeak Date: Wed, 2 Jul 2025 09:16:34 +0200 Subject: [PATCH 4/5] Update src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs Co-authored-by: Brennan --- src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index ef86a5936829..672c566b7538 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -571,7 +571,9 @@ private void CompleteStream(bool errored) TryClose(); } + RequestBodyPipe.Reader.Complete(); + _http3Output.Complete(); // Stream will be pooled after app completed. From 5a1ab024049e18c00969c71ef96504a6de922964 Mon Sep 17 00:00:00 2001 From: ladeak Date: Wed, 2 Jul 2025 09:18:38 +0200 Subject: [PATCH 5/5] Review suggestion --- .../test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs index eff3b384b784..0d5e22d295a4 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs @@ -3306,7 +3306,7 @@ public async Task CanHaveBody_ReturnsFalseWithoutRequestBody(bool endstream) var requestStream = await Http3Api.InitializeConnectionAndStreamsAsync(context => { - Assert.Equal(!endstream, context.Features.Get().CanHaveBody); + Assert.NotEqual(endstream, context.Features.Get().CanHaveBody); context.Response.StatusCode = 200; return Task.CompletedTask; }, headers, endStream: endstream);