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 1 commit
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
Next Next commit
Throw HttpProtocolException in case we get a GOAWAY frame while waiti…
…ng for next frame on response
  • Loading branch information
liveans committed Jul 11, 2024
commit b64f89fc8cce3235d5b27eda9ac41360e76d1ec6
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 ?
ThrowMissingFrame() : ThrowPrematureEOF(FrameHeader.Size);
throw _goAwayErrorCode is not null ?
new HttpProtocolException((long)_goAwayErrorCode, exception.Message, exception) : exception;
}
}
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 ThrowPrematureEOF(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 ThrowPrematureEOF(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 ThrowMissingFrame() =>
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