Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,14 +412,10 @@ private async ValueTask<FrameHeader> ReadFrameAsync(bool initialFrame = false)
_incomingBuffer.Commit(bytesRead);
if (bytesRead == 0)
{
if (_incomingBuffer.ActiveLength == 0)
{
ThrowMissingFrame();
}
else
{
ThrowPrematureEOF(FrameHeader.Size);
}
HttpIOException exception = _incomingBuffer.ActiveLength == 0 ?
GetMissingFrameException() : GetPrematureEOFException(FrameHeader.Size);
throw _goAwayErrorCode is not null ?
new HttpProtocolException((long)_goAwayErrorCode, exception.Message, exception) : exception;
liveans marked this conversation as resolved.
Show resolved Hide resolved
}
}
while (_incomingBuffer.ActiveLength < FrameHeader.Size);
Expand Down Expand Up @@ -449,19 +447,19 @@ private async ValueTask<FrameHeader> ReadFrameAsync(bool initialFrame = false)

int bytesRead = await _stream.ReadAsync(_incomingBuffer.AvailableMemory).ConfigureAwait(false);
_incomingBuffer.Commit(bytesRead);
if (bytesRead == 0) ThrowPrematureEOF(frameHeader.PayloadLength);
if (bytesRead == 0) throw GetPrematureEOFException(frameHeader.PayloadLength);
}
while (_incomingBuffer.ActiveLength < frameHeader.PayloadLength);
}

// Return the read frame header.
return frameHeader;

void ThrowPrematureEOF(int requiredBytes) =>
throw new HttpIOException(HttpRequestError.ResponseEnded, SR.Format(SR.net_http_invalid_response_premature_eof_bytecount, requiredBytes - _incomingBuffer.ActiveLength));
HttpIOException GetPrematureEOFException(int requiredBytes) =>
new HttpIOException(HttpRequestError.ResponseEnded, SR.Format(SR.net_http_invalid_response_premature_eof_bytecount, requiredBytes - _incomingBuffer.ActiveLength));

void ThrowMissingFrame() =>
throw new HttpIOException(HttpRequestError.ResponseEnded, SR.net_http_invalid_response_missing_frame);
HttpIOException GetMissingFrameException() =>
new HttpIOException(HttpRequestError.ResponseEnded, SR.net_http_invalid_response_missing_frame);
}

private async Task ProcessIncomingFramesAsync()
Expand Down Expand Up @@ -1070,6 +1068,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,39 @@ public async Task GoAwayFrame_RequestWithBody_ServerDisconnect_AbortStreamsAndTh
}
}

[ConditionalFact(nameof(SupportsAlpn))]
public async Task GoAwayFrame_RequestServerDisconnects_ThrowsHttpProtocolExceptionWithProperErrorCode()
{
_output.WriteLine("Started!");
liveans marked this conversation as resolved.
Show resolved Hide resolved
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);
_output.WriteLine("After response header read");
// 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.IsType<HttpIOException>(exception.InnerException);
Assert.Contains("ended prematurely while waiting", exception.Message);

},
async server =>
{
// Server returns response headers
Http2LoopbackConnection connection = await server.EstablishConnectionAsync();
liveans marked this conversation as resolved.
Show resolved Hide resolved
int streamId = await connection.ReadRequestHeaderAsync();
await connection.SendDefaultResponseHeadersAsync(streamId);
_output.WriteLine("Server sent response headers!");
// Server sends GOAWAY frame
await connection.SendGoAway(streamId, ProtocolErrors.ENHANCE_YOUR_CALM);
_output.WriteLine("Server sent GOAWAY!");
connection.ShutdownSend();
}).WaitAsync(TimeSpan.FromSeconds(10));
liveans marked this conversation as resolved.
Show resolved Hide resolved
}

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