Skip to content

Commit cb9b9f7

Browse files
authored
Handle missing HTTP3 error cases (dotnet#101923)
* Check for HttpRequestError.HttpProtocolError when expecting HTTP3 protocol errors in test * WIP * Fix data race in ServerClosesOutboundControlStream_ClientClosesConnection * Handle more exceptions explicitly in Http3RequestStream * WIP * Remove debug guard * Fix dotnet#102202
1 parent 362a95d commit cb9b9f7

File tree

3 files changed

+53
-4
lines changed

3 files changed

+53
-4
lines changed

src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/Frames/Http3ErrorCode.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,5 +91,20 @@ internal enum Http3ErrorCode : long
9191
/// The requested operation cannot be served over HTTP/3. The peer should retry over HTTP/1.1.
9292
/// </summary>
9393
VersionFallback = 0x110,
94+
/// <summary>
95+
/// H3_QPACK_DECOMPRESSION_FAILED (0x200):
96+
/// The decoder failed to interpret an encoded field section and is not able to continue decoding that field section.
97+
/// </summary>
98+
QPackDecompressionFailed = 0x200,
99+
/// <summary>
100+
/// H3_QPACK_ENCODER_STREAM_ERROR (0x201):
101+
/// The decoder failed to interpret an encoder instruction received on the encoder stream.
102+
/// </summary>
103+
QPackEncoderStreamError = 0x201,
104+
/// <summary>
105+
/// H3_QPACK_DECODER_STREAM_ERROR (0x202):
106+
/// The encoder failed to interpret an decoder instruction received on the decoder stream.
107+
/// </summary>
108+
QPackDecoderStreamError = 0x202,
94109
}
95110
}

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,12 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s
277277
}
278278
catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null)
279279
{
280-
// we close the connection, propagate the AbortException
281-
throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, _connection.AbortException);
280+
// we closed the connection already, propagate the AbortException
281+
HttpRequestError httpRequestError = _connection.AbortException is HttpProtocolException
282+
? HttpRequestError.HttpProtocolError
283+
: HttpRequestError.Unknown;
284+
285+
throw new HttpRequestException(httpRequestError, SR.net_http_client_execution_error, _connection.AbortException);
282286
}
283287
// It is possible for user's Content code to throw an unexpected OperationCanceledException.
284288
catch (OperationCanceledException ex) when (ex.CancellationToken == _requestBodyCancellationSource.Token || ex.CancellationToken == cancellationToken)
@@ -300,13 +304,26 @@ await Task.WhenAny(sendRequestTask, readResponseTask).ConfigureAwait(false) == s
300304
_connection.Abort(ex);
301305
throw new HttpRequestException(ex.HttpRequestError, SR.net_http_client_execution_error, ex);
302306
}
307+
catch (QPackDecodingException ex)
308+
{
309+
Exception abortException = _connection.Abort(HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.QPackDecompressionFailed));
310+
throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.net_http_invalid_response, ex);
311+
}
312+
catch (QPackEncodingException ex)
313+
{
314+
_stream.Abort(QuicAbortDirection.Write, (long)Http3ErrorCode.InternalError);
315+
throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, ex);
316+
}
303317
catch (Exception ex)
304318
{
305319
_stream.Abort(QuicAbortDirection.Write, (long)Http3ErrorCode.InternalError);
306320
if (ex is HttpRequestException)
307321
{
308322
throw;
309323
}
324+
325+
// all exceptions should be already handled above
326+
Debug.Fail($"Unexpected exception type in Http3RequestStream.SendAsync: {ex}");
310327
throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, ex);
311328
}
312329
finally
@@ -1253,6 +1270,13 @@ private void HandleReadResponseContentException(Exception ex, CancellationToken
12531270
_connection.Abort(exception);
12541271
throw exception;
12551272

1273+
case QuicException e when (e.QuicError == QuicError.OperationAborted && _connection.AbortException != null):
1274+
// we closed the connection already, propagate the AbortException
1275+
HttpRequestError httpRequestError = _connection.AbortException is HttpProtocolException
1276+
? HttpRequestError.HttpProtocolError
1277+
: HttpRequestError.Unknown;
1278+
throw new HttpRequestException(httpRequestError, SR.net_http_client_execution_error, _connection.AbortException);
1279+
12561280
case HttpIOException:
12571281
_connection.Abort(ex);
12581282
ExceptionDispatchInfo.Throw(ex); // Rethrow.

src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ private async Task AssertProtocolErrorAsync(long errorCode, Func<Task> task)
3535
{
3636
Exception outerEx = await Assert.ThrowsAnyAsync<Exception>(task);
3737
_output.WriteLine(outerEx.ToString());
38-
Assert.IsType<HttpRequestException>(outerEx);
38+
39+
HttpRequestException httpReqException = Assert.IsType<HttpRequestException>(outerEx);
40+
Assert.Equal(HttpRequestError.HttpProtocolError, httpReqException.HttpRequestError);
41+
3942
HttpProtocolException protocolEx = Assert.IsType<HttpProtocolException>(outerEx.InnerException);
4043
Assert.Equal(errorCode, protocolEx.ErrorCode);
4144
}
@@ -1759,7 +1762,14 @@ public async Task ServerClosesOutboundControlStream_ClientClosesConnection(Close
17591762
return;
17601763
}
17611764
await connection.OutboundControlStream.DisposeAsync();
1762-
await connection.EstablishControlStreamAsync(Array.Empty<SettingsEntry>());
1765+
try
1766+
{
1767+
await connection.EstablishControlStreamAsync(Array.Empty<SettingsEntry>());
1768+
}
1769+
catch (QuicException ex) when (ex.QuicError == QuicError.ConnectionAborted && ex.ApplicationErrorCode == Http3LoopbackConnection.H3_CLOSED_CRITICAL_STREAM)
1770+
{
1771+
// Data race, connection closed between WaitAsync and EstablishControlStreamAsync. Ignore this.
1772+
}
17631773
await Task.Delay(100);
17641774
}
17651775
}

0 commit comments

Comments
 (0)