Skip to content

Commit 96c885b

Browse files
BrennanConroyBrennanConroy
committed
fix state bug
1 parent 9bb37ca commit 96c885b

File tree

6 files changed

+63
-15
lines changed

6 files changed

+63
-15
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ private void CommitChunkInternal(ref BufferWriter<PipeWriter> writer, ReadOnlySp
334334
writer.Commit();
335335
}
336336

337-
public void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, bool appComplete, bool canWriteBody)
337+
public void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, bool appComplete)
338338
{
339339
lock (_contextLock)
340340
{
@@ -345,8 +345,6 @@ public void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpRespo
345345
return;
346346
}
347347

348-
_canWriteBody = canWriteBody;
349-
350348
var buffer = _pipeWriter;
351349
var writer = new BufferWriter<PipeWriter>(buffer);
352350
WriteResponseHeadersInternal(ref writer, statusCode, reasonPhrase, responseHeaders, autoChunk);
@@ -544,6 +542,11 @@ public ValueTask<FlushResult> FirstWriteChunkedAsync(int statusCode, string? rea
544542
}
545543
}
546544

545+
public void SetCanWriteBody(bool canWriteBody)
546+
{
547+
_canWriteBody = canWriteBody;
548+
}
549+
547550
public void Reset()
548551
{
549552
Debug.Assert(_currentSegmentOwner == null);
@@ -554,6 +557,7 @@ public void Reset()
554557
_writeStreamSuffixCalled = false;
555558
_currentChunkMemoryUpdated = false;
556559
_startCalled = false;
560+
_canWriteBody = true;
557561
}
558562

559563
private ValueTask<FlushResult> WriteAsync(

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,7 @@ private void ProduceStart(bool appCompleted)
999999

10001000
var responseHeaders = CreateResponseHeaders(appCompleted);
10011001

1002-
Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders, _autoChunk, appCompleted, canWriteBody: _canWriteResponseBody);
1002+
Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders, _autoChunk, appCompleted);
10031003
}
10041004

10051005
private void VerifyInitializeState(int firstWriteByteCount)
@@ -1162,6 +1162,7 @@ private HttpResponseHeaders CreateResponseHeaders(bool appCompleted)
11621162

11631163
// Set whether response can have body
11641164
_canWriteResponseBody = CanWriteResponseBody();
1165+
Output.SetCanWriteBody(_canWriteResponseBody);
11651166

11661167
if (!_canWriteResponseBody && hasTransferEncoding)
11671168
{
@@ -1645,7 +1646,7 @@ private ValueTask<FlushResult> FirstWriteAsyncInternal(ReadOnlyMemory<byte> data
16451646
{
16461647
if (data.Length == 0)
16471648
{
1648-
Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders, _autoChunk, appCompleted: false, canWriteBody: _canWriteResponseBody);
1649+
Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders, _autoChunk, appCompleted: false);
16491650
return Output.FlushAsync(cancellationToken);
16501651
}
16511652

@@ -1659,7 +1660,7 @@ private ValueTask<FlushResult> FirstWriteAsyncInternal(ReadOnlyMemory<byte> data
16591660
}
16601661
else
16611662
{
1662-
Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders, _autoChunk, appCompleted: false, canWriteBody: _canWriteResponseBody);
1663+
Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders, _autoChunk, appCompleted: false);
16631664
HandleNonBodyResponseWrite();
16641665
return Output.FlushAsync(cancellationToken);
16651666
}

‎src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ internal interface IHttpOutputProducer
1313
ValueTask<FlushResult> WriteChunkAsync(ReadOnlySpan<byte> data, CancellationToken cancellationToken);
1414
ValueTask<FlushResult> FlushAsync(CancellationToken cancellationToken);
1515
ValueTask<FlushResult> Write100ContinueAsync();
16-
void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, bool appCompleted, bool canWriteBody);
16+
void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, bool appCompleted);
1717
// This takes ReadOnlySpan instead of ReadOnlyMemory because it always synchronously copies data before flushing.
1818
ValueTask<FlushResult> WriteDataToPipeAsync(ReadOnlySpan<byte> data, CancellationToken cancellationToken);
1919
// Test hook
@@ -28,4 +28,6 @@ internal interface IHttpOutputProducer
2828
ValueTask<FlushResult> FirstWriteAsync(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, ReadOnlySpan<byte> data, CancellationToken cancellationToken);
2929
ValueTask<FlushResult> FirstWriteChunkedAsync(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, ReadOnlySpan<byte> data, CancellationToken cancellationToken);
3030
void Reset();
31+
// Only used by Http1OutputProducer for non-body responses
32+
void SetCanWriteBody(bool canWriteBody) { }
3133
}

‎src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,7 @@ public ValueTask<FlushResult> Write100ContinueAsync()
358358
}
359359
}
360360

361-
// canWriteBody is ignored as we don't have chunked bodies in HTTP/2 and so writing headers is not affected by canWriteBody
362-
public void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, bool appCompleted, bool canWriteBody)
361+
public void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, bool appCompleted)
363362
{
364363
lock (_dataWriterLock)
365364
{
@@ -557,8 +556,7 @@ public ValueTask<FlushResult> FirstWriteAsync(int statusCode, string? reasonPhra
557556
{
558557
lock (_dataWriterLock)
559558
{
560-
// canWriteBody is hardcoded to true as we don't have chunked bodies in HTTP/2 and so writing headers is not affected by canWriteBody
561-
WriteResponseHeaders(statusCode, reasonPhrase, responseHeaders, autoChunk, appCompleted: false, canWriteBody: true);
559+
WriteResponseHeaders(statusCode, reasonPhrase, responseHeaders, autoChunk, appCompleted: false);
562560

563561
return WriteDataToPipeAsync(data, cancellationToken);
564562
}

‎src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,7 @@ public ValueTask<FlushResult> FirstWriteAsync(int statusCode, string? reasonPhra
153153
{
154154
lock (_dataWriterLock)
155155
{
156-
// canWriteBody is hardcoded to true as we don't have chunked bodies in HTTP/3 and so writing headers is not affected by canWriteBody
157-
WriteResponseHeaders(statusCode, reasonPhrase, responseHeaders, autoChunk, appCompleted: false, canWriteBody: true);
156+
WriteResponseHeaders(statusCode, reasonPhrase, responseHeaders, autoChunk, appCompleted: false);
158157

159158
return WriteDataToPipeAsync(data, cancellationToken);
160159
}
@@ -376,8 +375,7 @@ public ValueTask<FlushResult> WriteDataToPipeAsync(ReadOnlySpan<byte> data, Canc
376375
}
377376
}
378377

379-
// canWriteBody is ignored as we don't have chunked bodies in HTTP/3 and so writing headers is not affected by canWriteBody
380-
public void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, bool appCompleted, bool canWriteBody)
378+
public void WriteResponseHeaders(int statusCode, string? reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk, bool appCompleted)
381379
{
382380
// appCompleted flag is not used here. The write FIN is sent via the transport and not via the frame.
383381
// Headers are written to buffer and flushed with a FIN when Http3FrameWriter.CompleteAsync is called

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1593,6 +1593,51 @@ await connection.Receive(
15931593
}
15941594
}
15951595

1596+
// Rough attempt at checking that a non-body response doesn't affect future body responses
1597+
[Fact]
1598+
public async Task GetRequestAfterHeadRequestWorks()
1599+
{
1600+
var serviceContext = new TestServiceContext(LoggerFactory) { ServerOptions = { AllowSynchronousIO = true } };
1601+
1602+
await using (var server = new TestServer(async httpContext =>
1603+
{
1604+
await httpContext.Response.BodyWriter.WriteAsync(new byte[] { 35, 35, 35 });
1605+
}, serviceContext))
1606+
{
1607+
using (var connection = server.CreateConnection())
1608+
{
1609+
await connection.Send(
1610+
"HEAD / HTTP/1.1",
1611+
"Host:",
1612+
"",
1613+
"");
1614+
await connection.Receive(
1615+
"HTTP/1.1 200 OK",
1616+
$"Date: {server.Context.DateHeaderValue}",
1617+
"",
1618+
"");
1619+
1620+
await connection.Send(
1621+
"GET /a HTTP/1.1",
1622+
"Host:",
1623+
"",
1624+
"");
1625+
await connection.Receive(
1626+
"HTTP/1.1 200 OK",
1627+
$"Date: {server.Context.DateHeaderValue}",
1628+
"Transfer-Encoding: chunked",
1629+
"",
1630+
"3",
1631+
"###",
1632+
"0",
1633+
"",
1634+
"");
1635+
connection.ShutdownSend();
1636+
await connection.ReceiveEnd();
1637+
}
1638+
}
1639+
}
1640+
15961641
[Fact]
15971642
public async Task HeadResponseBodyNotWrittenWithAdvanceBeforeAndAfterFlush()
15981643
{

0 commit comments

Comments
 (0)