VSadov/Satori
1
0
Fork 0
mirror of https://github.com/VSadov/Satori.git synced 2025-06-09 17:44:48 +09:00
Code Activity

[HTTP/2] Throw meaningful exception if we get GOAWAY while reading response body (#104707)

Browse source 
* 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>
This commit is contained in:
Ahmet Ibrahim Aksoy 2024-07-12 06:11:59 +02:00 • committed by GitHub
parent 0d5a9e7a59
commit 27e9b9db7a
Signed by: github
GPG key ID: B5690EEEBB952194
2 changed files with 38 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

9
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
View file

@ -72,6 +72,8 @@ namespace System.Net.Http
// _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.
@ -410,7 +412,11 @@ namespace System.Net.Http
_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();
}
@ -1070,6 +1076,7 @@ namespace System.Net.Http
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();

30
src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
View file

@ -1051,6 +1051,36 @@ namespace System.Net.Http.Functional.Tests
}
}
[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()
{