Skip to content

Commit

Permalink
Http2Stream throws a wrapped Http2ConnectionException on GO_AWAY (#54625
Browse files Browse the repository at this point in the history
)

If server sends `GO_AWAY` to client, `Http2Connection` handles it and sets a correct `Http2ConnectionException` to `_resetException` field followed by resetting all active Http2Streams. Each of these streams is expected to rethrow that `_resetException` to communicate the original protocol error to the application code. However, the method `Http2Stream.SendDataAsync` currently doesn't take into account that field, thus when it gets cancelled as part of a stream reset it just throws `OperationCanceledException` which doesn't contain any details. This PR fixes that and makes `Http2Stream.SendDataAsync` throw the original `Http2ConnectionException` wrapped by `IOException`.

Fixes #42472
  • Loading branch information
alnikola authored Jul 2, 2021
1 parent 97898a4 commit 4c92aef
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,23 @@ private async ValueTask SendDataAsync(ReadOnlyMemory<byte> buffer, CancellationT
await _connection.SendStreamDataAsync(StreamId, current, flush, _requestBodyCancellationSource.Token).ConfigureAwait(false);
}
}
catch (OperationCanceledException e) when (e.CancellationToken == _requestBodyCancellationSource.Token)
{
lock (SyncObject)
{
if (_resetException is Exception resetException)
{
if (_canRetry)
{
ThrowRetry(SR.net_http_request_aborted, resetException);
}

ThrowRequestAborted(resetException);
}
}

throw;
}
finally
{
linkedRegistration.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,39 @@ await Assert.ThrowsAnyAsync<HttpRequestException>(() =>
}
}

[ConditionalFact(nameof(SupportsAlpn))]
public async Task GoAwayFrame_RequestWithBody_ServerDisconnect_AbortStreamsAndThrowIOException()
{
using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer())
using (HttpClient client = CreateHttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Post, server.Address);
request.Version = new Version(2, 0);
var content = new string('*', 300);
var stream = new CustomContent.SlowTestStream(Encoding.UTF8.GetBytes(content), null, count: 60);
request.Content = new CustomContent(stream);

Task<HttpResponseMessage> sendTask = client.SendAsync(request);

Http2LoopbackConnection connection = await server.EstablishConnectionAsync();
(int streamId, _) = await connection.ReadAndParseRequestHeaderAsync(readBody: false);
await connection.SendDefaultResponseHeadersAsync(streamId);

await connection.SendGoAway(0, errorCode: ProtocolErrors.PROTOCOL_ERROR);

// Expect client to detect that server has disconnected and throw an exception
var exception = await Assert.ThrowsAnyAsync<HttpRequestException>(() =>
new Task[]
{
sendTask
}.WhenAllOrAnyFailed(TestHelper.PassingTestTimeoutMilliseconds));

Assert.IsType<IOException>(exception.InnerException);
Assert.NotNull(exception.InnerException.InnerException);
Assert.Contains("PROTOCOL_ERROR", exception.InnerException.InnerException.Message);
}
}

[ConditionalFact(nameof(SupportsAlpn))]
public async Task GoAwayFrame_UnprocessedStreamFirstRequestFinishedFirst_RequestRestarted()
{
Expand Down Expand Up @@ -2790,8 +2823,8 @@ public async Task PostAsyncDuplex_ServerResetsStream_Throws()
// Trying to read on the response stream should fail now, and client should ignore any data received
await AssertProtocolErrorForIOExceptionAsync(SendAndReceiveResponseDataAsync(contentBytes, responseStream, connection, streamId), ProtocolErrors.ENHANCE_YOUR_CALM);

// Attempting to write on the request body should now fail with OperationCanceledException.
Exception e = await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => { await SendAndReceiveRequestDataAsync(contentBytes, requestStream, connection, streamId); });
// Attempting to write on the request body should now fail with IOException.
Exception e = await Assert.ThrowsAnyAsync<IOException>(async () => { await SendAndReceiveRequestDataAsync(contentBytes, requestStream, connection, streamId); });

// Propagate the exception to the request stream serialization task.
// This allows the request processing to complete.
Expand Down

0 comments on commit 4c92aef

Please sign in to comment.