Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -2116,6 +2116,45 @@ await server.AcceptConnectionAsync(async connection =>
});
}

[Fact]
public async Task SendAsync_Expect100Continue_RequestBodyFails_ThrowsContentException()
{
if (IsWinHttpHandler && UseVersion >= HttpVersion20.Value)
{
return;
}
if (!TestAsync && UseVersion >= HttpVersion20.Value)
{
return;
}

var clientFinished = new TaskCompletionSource<bool>();

await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
{
using (HttpClient client = CreateHttpClient())
{
HttpRequestMessage initialMessage = new HttpRequestMessage(HttpMethod.Post, uri) { Version = UseVersion };
initialMessage.Content = new ThrowingContent(() => new ThrowingContentException());
initialMessage.Headers.ExpectContinue = true;
Exception exception = await Assert.ThrowsAsync<ThrowingContentException>(() => client.SendAsync(TestAsync, initialMessage));

clientFinished.SetResult(true);
}
}, async server =>
{
await server.AcceptConnectionAsync(async connection =>
{
try
{
await connection.ReadRequestDataAsync(readBody: true);
}
catch { } // Eat errors from client disconnect.
await clientFinished.Task.TimeoutAfter(TimeSpan.FromMinutes(2));
});
});
}

[Fact]
public async Task SendAsync_No100ContinueReceived_RequestBodySentEventually()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
namespace System.Net.Http.Functional.Tests
{
/// <summary>HttpContent that mocks exceptions on serialization.</summary>
public class ThrowingContent : HttpContent
public partial class ThrowingContent : HttpContent
{
private readonly Func<Exception> _exnFactory;
private readonly int _length;
Expand All @@ -32,4 +32,7 @@ protected override bool TryComputeLength(out long length)
return true;
}
}

public class ThrowingContentException : Exception
{ }
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@
Link="Common\System\Net\Http\SchSendAuxRecordHttpTest.cs" />
<Compile Include="$(CommonTestPath)System\Net\Http\SyncBlockingContent.cs"
Link="Common\System\Net\Http\SyncBlockingContent.cs" />
<Compile Include="$(CommonTestPath)System\Net\Http\ThrowingContent.cs"
Link="Common\System\Net\Http\ThrowingContent.cs" />
<Compile Include="$(CommonTestPath)System\Threading\Tasks\TaskTimeoutExtensions.cs"
Link="Common\System\Threading\Tasks\TaskTimeoutExtensions.cs" />
<Compile Include="$(CommonTestPath)System\Threading\TrackingSynchronizationContext.cs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1747,7 +1747,8 @@ public sealed override async Task<HttpResponseMessage> SendAsync(HttpRequestMess
if (requestBodyTask.IsCompleted ||
duplex == false ||
await Task.WhenAny(requestBodyTask, responseHeadersTask).ConfigureAwait(false) == requestBodyTask ||
requestBodyTask.IsCompleted)
requestBodyTask.IsCompleted ||
http2Stream.SendRequestFinished)
{
// The sending of the request body completed before receiving all of the request headers (or we're
// ok waiting for the request body even if it hasn't completed, e.g. because we're not doing duplex).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ public void Initialize(int streamId, int initialWindowSize)

public int StreamId { get; private set; }

public bool SendRequestFinished => _requestCompletionState != StreamCompletionState.InProgress;

public HttpResponseMessage GetAndClearResponse()
{
// Once SendAsync completes, the Http2Stream should no longer hold onto the response message.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,21 @@ public async Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request,
// hook up a continuation that will log it.
if (sendRequestContentTask != null && !sendRequestContentTask.IsCompletedSuccessfully)
{
// In case the connection is disposed, it's most probable that
// expect100Continue timer expired and request content sending failed.
// We're awaiting the task to propagate the exception in this case.
if (Volatile.Read(ref _disposed) == 1)
{
if (async)
{
await sendRequestContentTask.ConfigureAwait(false);
}
else
{
// No way around it here if we want to get the exception from the task.
sendRequestContentTask.GetAwaiter().GetResult();
}
}
LogExceptions(sendRequestContentTask);
}

Expand Down Expand Up @@ -793,7 +808,8 @@ private async ValueTask SendRequestContentAsync(HttpRequestMessage request, Http
}

private async Task SendRequestContentWithExpect100ContinueAsync(
HttpRequestMessage request, Task<bool> allowExpect100ToContinueTask, HttpContentWriteStream stream, Timer expect100Timer, bool async, CancellationToken cancellationToken)
HttpRequestMessage request, Task<bool> allowExpect100ToContinueTask,
HttpContentWriteStream stream, Timer expect100Timer, bool async, CancellationToken cancellationToken)
{
// Wait until we receive a trigger notification that it's ok to continue sending content.
// This will come either when the timer fires or when we receive a response status line from the server.
Expand All @@ -806,7 +822,17 @@ private async Task SendRequestContentWithExpect100ContinueAsync(
if (sendRequestContent)
{
if (NetEventSource.Log.IsEnabled()) Trace($"Sending request content for Expect: 100-continue.");
await SendRequestContentAsync(request, stream, async, cancellationToken).ConfigureAwait(false);
try
{
await SendRequestContentAsync(request, stream, async, cancellationToken).ConfigureAwait(false);
}
catch
{
// Tear down the connection if called from the timer thread because caller's thread will wait for server status line indefinitely
// or till HttpClient.Timeout tear the connection itself.
Dispose();
throw;
}
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@
<Compile Include="$(CommonTestPath)System\Net\Http\HttpClientHandlerTest.Proxy.cs"
Link="Common\System\Net\Http\HttpClientHandlerTest.Proxy.cs" />
<Compile Include="HttpClientHandlerTest.Http3.cs" />

<Compile Include="HttpClientHandlerTest.ResponseDrain.cs" />
<Compile Include="$(CommonTestPath)System\Net\Http\HttpClientHandlerTest.ServerCertificates.cs"
Link="Common\System\Net\Http\HttpClientHandlerTest.ServerCertificates.cs" />
Expand Down Expand Up @@ -227,7 +226,9 @@
Link="Common\System\Net\Http\SyncBlockingContent.cs" />
<Compile Include="$(CommonTestPath)System\Net\Http\DefaultCredentialsTest.cs"
Link="Common\System\Net\Http\DefaultCredentialsTest.cs" />
<Compile Include="ThrowingContent.cs" />
<Compile Include="$(CommonTestPath)System\Net\Http\ThrowingContent.cs"
Link="Common\System\Net\Http\ThrowingContent.cs" />
<Compile Include="ThrowingContent.netcore.cs" />
<Compile Include="Watchdog.cs" />
</ItemGroup>
<!-- Windows specific files -->
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
using System.Threading;
using System.Threading.Tasks;

namespace System.Net.Http.Functional.Tests
{
public partial class ThrowingContent : HttpContent
{
protected override void SerializeToStream(Stream stream, TransportContext context, CancellationToken cancellationToken)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Shouldn't the default SerializeToStream(stream, context, cancellationToken) end up calling SerializeToStream(stream, context)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR yes

This is the only sync implementation we provide for ThrowingContent. The one in shared code is async and there's now automatic/default sync-over-async in any of our HttpContent classes.

{
throw _exnFactory();
}
}
}