Skip to content

Commit

Permalink
[HTTP/2] Throw meaningful exception if we get GOAWAY while reading re…
Browse files Browse the repository at this point in the history
…sponse body (#104707)

* Throw HttpProtocolException in case we get a GOAWAY frame while waiting for next frame on response

* Fix helper method names

* Apply suggestions from code review

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>

* Code review feedback

* Revert method names

* Fix test with the new behavior

---------

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
  • Loading branch information
liveans and MihaZupan authored Jul 12, 2024
1 parent 0d5a9e7 commit 27e9b9d
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ internal sealed partial class Http2Connection : HttpConnectionBase
// _shutdown above is true, and requests in flight have been (or are being) failed.
private Exception? _abortException;

private Http2ProtocolErrorCode? _goAwayErrorCode;

private const int MaxStreamId = int.MaxValue;

// Temporary workaround for request burst handling on connection start.
Expand Down Expand Up @@ -410,7 +412,11 @@ private async ValueTask<FrameHeader> ReadFrameAsync(bool initialFrame = false)
_incomingBuffer.Commit(bytesRead);
if (bytesRead == 0)
{
if (_incomingBuffer.ActiveLength == 0)
if (_goAwayErrorCode is not null)
{
ThrowProtocolError(_goAwayErrorCode.Value, SR.net_http_http2_connection_close);
}
else if (_incomingBuffer.ActiveLength == 0)
{
ThrowMissingFrame();
}
Expand Down Expand Up @@ -1070,6 +1076,7 @@ private void ProcessGoAwayFrame(FrameHeader frameHeader)

Debug.Assert(lastStreamId >= 0);
Exception resetException = HttpProtocolException.CreateHttp2ConnectionException(errorCode, SR.net_http_http2_connection_close);
_goAwayErrorCode = errorCode;

// There is no point sending more PING frames for RTT estimation:
_rttEstimator.OnGoAwayReceived();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,36 @@ public async Task GoAwayFrame_RequestWithBody_ServerDisconnect_AbortStreamsAndTh
}
}

[ConditionalFact(nameof(SupportsAlpn))]
public async Task GoAwayFrame_RequestServerDisconnects_ThrowsHttpProtocolExceptionWithProperErrorCode()
{
await Http2LoopbackServer.CreateClientAndServerAsync(async uri =>
{
// Client starts an HTTP/2 request and awaits response headers
using HttpClient client = CreateHttpClient();
HttpResponseMessage response = await client.GetAsync(uri, HttpCompletionOption.ResponseHeadersRead);

// Client reads from response stream
using Stream responseStream = await response.Content.ReadAsStreamAsync();
Memory<byte> buffer = new byte[1024];
HttpProtocolException exception = await Assert.ThrowsAsync<HttpProtocolException>(() => responseStream.ReadAsync(buffer).AsTask());
Assert.Equal(ProtocolErrors.ENHANCE_YOUR_CALM, (ProtocolErrors) exception.ErrorCode);
Assert.Contains("The HTTP/2 server closed the connection.", exception.Message);

},
async server =>
{
// Server returns response headers
await using Http2LoopbackConnection connection = await server.EstablishConnectionAsync();
int streamId = await connection.ReadRequestHeaderAsync();
await connection.SendDefaultResponseHeadersAsync(streamId);

// Server sends GOAWAY frame
await connection.SendGoAway(streamId, ProtocolErrors.ENHANCE_YOUR_CALM);
connection.ShutdownSend();
});
}

[ConditionalFact(nameof(SupportsAlpn))]
public async Task GoAwayFrame_UnprocessedStreamFirstRequestFinishedFirst_RequestRestarted()
{
Expand Down

0 comments on commit 27e9b9d

Please sign in to comment.