Skip to content

Commit

Permalink
fix MaxResponseHeadersLength tests (#74479)
Browse files Browse the repository at this point in the history 
* fix MaxResponseHeadersLength tests

* add EnablePreviewFeatures

* fix winhttp

* feedback from review

* feedback from review
  • Loading branch information
@wfurt
wfurt authored Aug 26, 2022
1 parent a3960a8 commit ecb3038
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 19 deletions.
27 changes: 25 additions & 2 deletions src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.MaxResponseHeadersLength.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.IO;
using System.Net.Test.Common;
#if !WINHTTPHANDLER_TEST
using System.Net.Quic;
#endif
using System.Net.Sockets;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -20,6 +25,8 @@ namespace System.Net.Http.Functional.Tests

public abstract class HttpClientHandler_MaxResponseHeadersLength_Test : HttpClientHandlerTestBase
{
private const int Http3ExcessiveLoad = 0x107;

public HttpClientHandler_MaxResponseHeadersLength_Test(ITestOutputHelper output) : base(output) { }

[Theory]
Expand Down Expand Up @@ -81,7 +88,15 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
},
async server =>
{
await server.HandleRequestAsync(headers: new[] { new HttpHeaderData("Foo", new string('a', handler.MaxResponseHeadersLength * 1024)) });
try
{
await server.HandleRequestAsync(headers: new[] { new HttpHeaderData("Foo", new string('a', handler.MaxResponseHeadersLength * 1024)) });
}
// Client can respond by closing/aborting the underlying stream while we are still sending the headers, ignore these exceptions
catch (IOException ex) when (ex.InnerException is SocketException se && se.SocketErrorCode == SocketError.Shutdown) { }
#if !WINHTTPHANDLER_TEST
catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted && ex.ApplicationErrorCode == Http3ExcessiveLoad) {}
#endif
});
}

Expand Down Expand Up @@ -125,7 +140,15 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
headers.Add(new HttpHeaderData($"Custom-{i}", new string('a', 480)));
}
await server.HandleRequestAsync(headers: headers);
try
{
await server.HandleRequestAsync(headers: headers);
}
// Client can respond by closing/aborting the underlying stream while we are still sending the headers, ignore these exceptions
catch (IOException ex) when (ex.InnerException is SocketException se && se.SocketErrorCode == SocketError.Shutdown) { }
#if !WINHTTPHANDLER_TEST
catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted && ex.ApplicationErrorCode == Http3ExcessiveLoad) {}
#endif
});
}
}
Expand Down
1 change: 1 addition & 0 deletions ...nHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<TargetFrameworks>$(NetCoreAppCurrent)-windows;net48</TargetFrameworks>
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
<DefineConstants>$(DefineConstants);WINHTTPHANDLER_TEST</DefineConstants>
<EnablePreviewFeatures>true</EnablePreviewFeatures>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(CommonTestPath)System\Net\Configuration.cs"
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ private async ValueTask ReadHeadersAsync(long headersLength, CancellationToken c
// https://tools.ietf.org/html/draft-ietf-quic-http-24#section-4.1.1
if (headersLength > _headerBudgetRemaining)
{
_stream.Abort(QuicAbortDirection.Write, (long)Http3ErrorCode.ExcessiveLoad);
_stream.Abort(QuicAbortDirection.Read, (long)Http3ErrorCode.ExcessiveLoad);
throw new HttpRequestException(SR.Format(SR.net_http_response_headers_exceeded_length, _connection.Pool.Settings.MaxResponseHeadersByteLength));
}

Expand Down
1 change: 0 additions & 1 deletion src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1240,7 +1240,6 @@ public SocketsHttpHandler_HttpClientHandler_MaxResponseHeadersLength_Http2(ITest
protected override Version UseVersion => HttpVersion.Version20;
}

[ActiveIssue("https://github.com/dotnet/runtime/issues/73930")]
[ConditionalClass(typeof(HttpClientHandlerTestBase), nameof(IsQuicSupported))]
public sealed class SocketsHttpHandler_HttpClientHandler_MaxResponseHeadersLength_Http3 : HttpClientHandler_MaxResponseHeadersLength_Test
{
Expand Down
67 changes: 58 additions & 9 deletions src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,60 +450,104 @@ private unsafe int HandleEventConnected(ref CONNECTED_DATA data)
QuicAddr localAddress = MsQuicHelpers.GetMsQuicParameter<QuicAddr>(_handle, QUIC_PARAM_CONN_LOCAL_ADDRESS);
_localEndPoint = localAddress.ToIPEndPoint();

_connectedTcs.TrySetResult();

if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Connection connected {LocalEndPoint} -> {RemoteEndPoint}");
NetEventSource.Info(this, $"{this} Received event CONNECTED {LocalEndPoint} -> {RemoteEndPoint}");
}

_connectedTcs.TrySetResult();
return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleEventShutdownInitiatedByTransport(ref SHUTDOWN_INITIATED_BY_TRANSPORT_DATA data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event SHUTDOWN_INITIATED_BY_TRANSPORT with {nameof(data.Status)}={data.Status}");
}

// TODO: we should propagate transport error code.
// https://github.com/dotnet/runtime/issues/72666
Exception exception = ExceptionDispatchInfo.SetCurrentStackTrace(ThrowHelper.GetExceptionForMsQuicStatus(data.Status));
_connectedTcs.TrySetException(exception);
_acceptQueue.Writer.TryComplete(exception);
return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleEventShutdownInitiatedByPeer(ref SHUTDOWN_INITIATED_BY_PEER_DATA data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event SHUTDOWN_INITIATED_BY_PEER_DATA with {nameof(data.ErrorCode)}={data.ErrorCode}");
}

_acceptQueue.Writer.TryComplete(ExceptionDispatchInfo.SetCurrentStackTrace(ThrowHelper.GetConnectionAbortedException((long)data.ErrorCode)));
return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleEventShutdownComplete(ref SHUTDOWN_COMPLETE_DATA data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event SHUTDOWN_INITIATED_BY_PEER_DATA");
}

_shutdownTcs.TrySetResult();
_acceptQueue.Writer.TryComplete(ExceptionDispatchInfo.SetCurrentStackTrace(ThrowHelper.GetOperationAbortedException()));
return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleEventLocalAddressChanged(ref LOCAL_ADDRESS_CHANGED_DATA data)
{
_localEndPoint = data.Address->ToIPEndPoint();
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event LOCAL_ADDRESS_CHANGED with {nameof(data.Address)}={_localEndPoint}");
}

return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleEventPeerAddressChanged(ref PEER_ADDRESS_CHANGED_DATA data)
{
_remoteEndPoint = data.Address->ToIPEndPoint();
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event LOCAL_ADDRESS_CHANGED with {nameof(data.Address)}={_remoteEndPoint}");
}

return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleEventPeerStreamStarted(ref PEER_STREAM_STARTED_DATA data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event PEER_STREAM_STARTED");
}

QuicStream stream = new QuicStream(_handle, data.Stream, data.Flags, _defaultStreamErrorCode);
if (!_acceptQueue.Writer.TryWrite(stream))
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Error(this, $"{this} Unable to enqueue incoming stream {stream}");
}

stream.Dispose();
return QUIC_STATUS_SUCCESS;
}

return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleEventPeerCertificateReceived(ref PEER_CERTIFICATE_RECEIVED_DATA data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event PEER_CERTIFICATE_RECEIVED_DATA");
}

try
{
return _sslConnectionOptions.ValidateCertificate((QUIC_BUFFER*)data.Certificate, (QUIC_BUFFER*)data.Chain, out _remoteCertificate);
Expand All @@ -515,6 +559,16 @@ private unsafe int HandleEventPeerCertificateReceived(ref PEER_CERTIFICATE_RECEI
}
}

private int HandleConnectionEvent(QUIC_CONNECTION_EVENT_TYPE type)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event {type}");
}

return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleConnectionEvent(ref QUIC_CONNECTION_EVENT connectionEvent)
=> connectionEvent.Type switch
{
Expand All @@ -526,7 +580,7 @@ private unsafe int HandleConnectionEvent(ref QUIC_CONNECTION_EVENT connectionEve
QUIC_CONNECTION_EVENT_TYPE.PEER_ADDRESS_CHANGED => HandleEventPeerAddressChanged(ref connectionEvent.PEER_ADDRESS_CHANGED),
QUIC_CONNECTION_EVENT_TYPE.PEER_STREAM_STARTED => HandleEventPeerStreamStarted(ref connectionEvent.PEER_STREAM_STARTED),
QUIC_CONNECTION_EVENT_TYPE.PEER_CERTIFICATE_RECEIVED => HandleEventPeerCertificateReceived(ref connectionEvent.PEER_CERTIFICATE_RECEIVED),
_ => QUIC_STATUS_SUCCESS
_ => HandleConnectionEvent(connectionEvent.Type),
};

#pragma warning disable CS3016
Expand All @@ -548,11 +602,6 @@ private static unsafe int NativeCallback(QUIC_HANDLE* connection, void* context,

try
{
// Process the event.
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(instance, $"{instance} Received event {connectionEvent->Type}");
}
return instance.HandleConnectionEvent(ref *connectionEvent);
}
catch (Exception ex)
Expand Down
68 changes: 62 additions & 6 deletions src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,11 @@ public void Abort(QuicAbortDirection abortDirection, long errorCode)
return;
}

if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Aborting {abortDirection} with {errorCode}");
}

unsafe
{
ThrowHelper.ThrowIfMsQuicError(MsQuicApi.Api.ApiTable->StreamShutdown(
Expand Down Expand Up @@ -452,6 +457,11 @@ public void CompleteWrites()

private unsafe int HandleEventStartComplete(ref START_COMPLETE data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event START_COMPLETE with {nameof(data.ID)}={data.ID}, {nameof(data.Status)}={data.Status} and {nameof(data.PeerAccepted)}={data.PeerAccepted}");
}

_id = unchecked((long)data.ID);
if (StatusSucceeded(data.Status))
{
Expand All @@ -477,6 +487,12 @@ private unsafe int HandleEventReceive(ref RECEIVE data)
new ReadOnlySpan<QUIC_BUFFER>(data.Buffers, (int) data.BufferCount),
(int) data.TotalBufferLength,
data.Flags.HasFlag(QUIC_RECEIVE_FLAGS.FIN));

if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event RECEIVE with {nameof(data.BufferCount)}={data.BufferCount}, {nameof(data.TotalBufferLength)}={data.TotalBufferLength} and {nameof(totalCopied)}={totalCopied}");
}

if (totalCopied < data.TotalBufferLength)
{
Volatile.Write(ref _receivedNeedsEnable, 1);
Expand All @@ -489,6 +505,11 @@ private unsafe int HandleEventReceive(ref RECEIVE data)
}
private unsafe int HandleEventSendComplete(ref SEND_COMPLETE data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event SEND_COMPLETE with {nameof(data.Canceled)}={data.Canceled}");
}

_sendBuffers.Reset();
if (data.Canceled == 0)
{
Expand All @@ -499,6 +520,11 @@ private unsafe int HandleEventSendComplete(ref SEND_COMPLETE data)
}
private unsafe int HandleEventPeerSendShutdown()
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event PEER_SEND_SHUTDOWN.");
}

// Same as RECEIVE with FIN flag. Remember that no more RECEIVE events will come.
// Don't set the task to its final state yet, but wait for all the buffered data to get consumed first.
_receiveBuffers.SetFinal();
Expand All @@ -507,16 +533,31 @@ private unsafe int HandleEventPeerSendShutdown()
}
private unsafe int HandleEventPeerSendAborted(ref PEER_SEND_ABORTED data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event PEER_SEND_ABORTED with {nameof(data.ErrorCode)}={data.ErrorCode}");
}

_receiveTcs.TrySetException(ThrowHelper.GetStreamAbortedException((long)data.ErrorCode), final: true);
return QUIC_STATUS_SUCCESS;
}
private unsafe int HandleEventPeerReceiveAborted(ref PEER_RECEIVE_ABORTED data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event PEER_RECEIVE_ABORTED with {nameof(data.ErrorCode)}={data.ErrorCode}");
}

_sendTcs.TrySetException(ThrowHelper.GetStreamAbortedException((long)data.ErrorCode), final: true);
return QUIC_STATUS_SUCCESS;
}
private unsafe int HandleEventSendShutdownComplete(ref SEND_SHUTDOWN_COMPLETE data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event PEER_RECEIVE_ABORTED with {nameof(data.Graceful)}={data.Graceful}");
}

if (data.Graceful != 0)
{
_sendTcs.TrySetResult(final: true);
Expand All @@ -526,6 +567,11 @@ private unsafe int HandleEventSendShutdownComplete(ref SEND_SHUTDOWN_COMPLETE da
}
private unsafe int HandleEventShutdownComplete(ref SHUTDOWN_COMPLETE data)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event SHUTDOWN_COMPLETE with {nameof(data.ConnectionShutdown)}={data.ConnectionShutdown}");
}

if (data.ConnectionShutdown != 0)
{
bool shutdownByApp = data.ConnectionShutdownByApp != 0;
Expand Down Expand Up @@ -554,10 +600,25 @@ private unsafe int HandleEventShutdownComplete(ref SHUTDOWN_COMPLETE data)
}
private unsafe int HandleEventPeerAccepted()
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event PEER_ACCEPTED");
}

_startedTcs.TrySetResult();
return QUIC_STATUS_SUCCESS;
}

private int HandleStreamEvent(QUIC_STREAM_EVENT_TYPE type)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(this, $"{this} Received event {type}");
}

return QUIC_STATUS_SUCCESS;
}

private unsafe int HandleStreamEvent(ref QUIC_STREAM_EVENT streamEvent)
=> streamEvent.Type switch
{
Expand All @@ -570,7 +631,7 @@ private unsafe int HandleStreamEvent(ref QUIC_STREAM_EVENT streamEvent)
QUIC_STREAM_EVENT_TYPE.SEND_SHUTDOWN_COMPLETE => HandleEventSendShutdownComplete(ref streamEvent.SEND_SHUTDOWN_COMPLETE),
QUIC_STREAM_EVENT_TYPE.SHUTDOWN_COMPLETE => HandleEventShutdownComplete(ref streamEvent.SHUTDOWN_COMPLETE),
QUIC_STREAM_EVENT_TYPE.PEER_ACCEPTED => HandleEventPeerAccepted(),
_ => QUIC_STATUS_SUCCESS
_ => HandleStreamEvent(streamEvent.Type)
};

#pragma warning disable CS3016
Expand All @@ -592,11 +653,6 @@ private static unsafe int NativeCallback(QUIC_HANDLE* connection, void* context,

try
{
// Process the event.
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(instance, $"{instance} Received event {streamEvent->Type}");
}
return instance.HandleStreamEvent(ref *streamEvent);
}
catch (Exception ex)
Expand Down

0 comments on commit ecb3038

Please sign in to comment.