Skip to content

Implement HttpWebRequest AllowWriteStreamBuffering property #95001

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

Merged
merged 58 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
cf19c74
Implement HttpWebRequest WriteStreamBuffering
liveans Nov 20, 2023
6167c96
Fix tests
liveans Dec 4, 2023
f1e9a29
Fix tests
liveans Dec 4, 2023
5015280
Review feedback
liveans Dec 4, 2023
b7c62d1
Fix timeouts
liveans Dec 4, 2023
9ae4383
fix test build
liveans Dec 4, 2023
3114962
Hang test
liveans Dec 5, 2023
266b610
Merge branch 'main' of github.com:dotnet/runtime into http-write-stre…
liveans Dec 10, 2023
8e20ffa
Fix build
liveans Dec 10, 2023
98152a2
fix tests
liveans Dec 15, 2023
ff5e2bd
delete unnecessary line
liveans Dec 15, 2023
32832f7
fix tests
liveans Dec 15, 2023
69f3da7
refactor test
liveans Dec 15, 2023
dd0b860
Dispose http client
liveans Dec 15, 2023
31ef8a6
Fix buffer write in RequestStream.Flush() method
liveans Jan 2, 2024
c5efd40
Fix same length bug in FlushAsync
liveans Jan 2, 2024
9fb2978
Update ContentLength in HttpWebRequestTest.cs
liveans Jan 2, 2024
ea38e0e
Fix flushing and ending request stream
liveans Jan 3, 2024
a5511ea
Merge branch 'main' into http-write-stream-buffering-impl
liveans Jan 5, 2024
03455dc
Fix flushing and ending request stream
liveans Jan 5, 2024
2318eb5
Fix FlushAsync method to handle cancellation
liveans Jan 5, 2024
ce2f240
Merge branch 'main' into http-write-stream-buffering-impl
liveans Jan 9, 2024
251f6dd
Merge branch 'main' into http-write-stream-buffering-impl
liveans Jan 24, 2024
01e7cca
Update src/libraries/System.Net.Requests/src/System/Net/HttpClientCon…
liveans Jan 26, 2024
f6c3ad0
Review feedback
liveans Jan 31, 2024
cc8c351
Bound streamBuffer lifecycle to HttpClientContentStream
liveans Feb 1, 2024
a2b2b84
Merge branch 'main' into http-write-stream-buffering-impl
liveans Feb 1, 2024
7836a8f
Review feedback
liveans Feb 3, 2024
62067e7
Review feedback
liveans Feb 3, 2024
964d4c6
Change ??= to =
liveans Feb 4, 2024
cc28b79
change delay on test
liveans Feb 4, 2024
607991c
Merge branch 'main' into http-write-stream-buffering-impl
liveans Feb 5, 2024
7d4de22
Apply suggestions from code review
liveans Feb 6, 2024
864fbb7
Fix build
liveans Feb 6, 2024
3b0287a
Review feedback
liveans Feb 7, 2024
149a843
Apply suggestions from code review
liveans Feb 10, 2024
37f990b
Review feedback
liveans Feb 12, 2024
eb494f3
Apply suggestions from code review
liveans Feb 16, 2024
55b5016
Add ProtocolViolationException if we're not buffering and we didn't s…
liveans Feb 16, 2024
a93a60c
Merge branch 'http-write-stream-buffering-impl' of github.com:liveans…
liveans Feb 16, 2024
957ee2c
Review feedback
liveans Feb 16, 2024
6650652
Add test for not buffering and sending the content before we call `Ge…
liveans Feb 16, 2024
454752c
Update src/libraries/System.Net.Requests/src/System/Net/HttpWebReques…
liveans Feb 16, 2024
928eaa2
Remove exception
liveans Feb 16, 2024
4f76467
Merge branch 'http-write-stream-buffering-impl' of github.com:liveans…
liveans Feb 16, 2024
8ebaa78
Review feedback
liveans Feb 16, 2024
de7f101
Fix string resources
liveans Feb 19, 2024
6f2ba90
Add RequestStreamContent class for handling request stream serialization
liveans Feb 20, 2024
dd1bdde
Seperate Buffering and Non-Buffering Streams and Connect non-bufferin…
liveans Feb 23, 2024
c60f443
Remove unnecessary code in HttpWebRequestTest
liveans Feb 23, 2024
a699449
Merge branch 'main' into http-write-stream-buffering-impl
liveans Feb 23, 2024
dcac125
Use random data for testing
liveans Feb 26, 2024
c01da77
Update src/libraries/System.Net.Requests/src/System/Net/RequestBuffer…
liveans Feb 26, 2024
1b51e50
Merge branch 'main' into http-write-stream-buffering-impl
liveans Feb 26, 2024
38656d5
Remove default flushes and add flush on test
liveans Feb 27, 2024
796f1e9
Merge branch 'main' into http-write-stream-buffering-impl
liveans Mar 1, 2024
da86238
Review feedback
liveans Mar 4, 2024
804b80c
Removing unused code
liveans Mar 5, 2024
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 @@ -29,6 +29,7 @@
<Compile Include="System\Net\IWebRequestCreate.cs" />
<Compile Include="System\Net\ProtocolViolationException.cs" />
<Compile Include="System\Net\RequestStream.cs" />
<Compile Include="System\Net\RequestBufferingStream.cs" />
<Compile Include="System\Net\TaskExtensions.cs" />
<Compile Include="System\Net\WebException.cs" />
<Compile Include="System\Net\WebExceptionStatus.cs" />
Expand All @@ -48,6 +49,7 @@
<Compile Include="System\Net\NetRes.cs" />
<Compile Include="System\Net\NetworkStreamWrapper.cs" />
<Compile Include="System\Net\TimerThread.cs" />
<Compile Include="System\Net\RequestStreamContent.cs" />
<Compile Include="System\Net\Cache\HttpCacheAgeControl.cs" />
<Compile Include="System\Net\Cache\HttpRequestCacheLevel.cs" />
<Compile Include="System\Net\Cache\HttpRequestCachePolicy.cs" />
Expand Down
177 changes: 112 additions & 65 deletions src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
Expand All @@ -12,6 +13,7 @@
using System.Net.Http.Headers;
using System.Net.Security;
using System.Net.Sockets;
using System.Runtime.CompilerServices;
using System.Runtime.Serialization;
using System.Security.Authentication;
using System.Security.Cryptography.X509Certificates;
Expand Down Expand Up @@ -40,6 +42,7 @@ public class HttpWebRequest : WebRequest, ISerializable
private IWebProxy? _proxy = WebRequest.DefaultWebProxy;

private Task<HttpResponseMessage>? _sendRequestTask;
private HttpRequestMessage? _sendRequestMessage;

private static int _defaultMaxResponseHeadersLength = HttpHandlerDefaults.DefaultMaxResponseHeadersLength;
private static int _defaultMaximumErrorResponseLength = -1;
Expand All @@ -62,7 +65,7 @@ public class HttpWebRequest : WebRequest, ISerializable
private bool _hostHasPort;
private Uri? _hostUri;

private RequestStream? _requestStream;
private Stream? _requestStream;
private TaskCompletionSource<Stream>? _requestStreamOperation;
private TaskCompletionSource<WebResponse>? _responseOperation;
private AsyncCallback? _requestStreamCallback;
Expand All @@ -78,6 +81,8 @@ public class HttpWebRequest : WebRequest, ISerializable
private static readonly object s_syncRoot = new object();
private static volatile HttpClient? s_cachedHttpClient;
private static HttpClientParameters? s_cachedHttpClientParameters;
private bool _disposeRequired;
private HttpClient? _httpClient;

//these should be safe.
[Flags]
Expand Down Expand Up @@ -1003,17 +1008,17 @@ public override void Abort()
{
_responseCallback(_responseOperation.Task);
}

// Cancel the underlying send operation.
Debug.Assert(_sendRequestCts != null);
_sendRequestCts.Cancel();
}
else if (_requestStreamOperation != null)
if (_requestStreamOperation != null)
{
if (_requestStreamOperation.TrySetCanceled() && _requestStreamCallback != null)
{
_requestStreamCallback(_requestStreamOperation.Task);
}

// Cancel the underlying send operation.
Debug.Assert(_sendRequestCts != null);
_sendRequestCts.Cancel();
}
}

Expand Down Expand Up @@ -1041,8 +1046,7 @@ public override WebResponse GetResponse()
{
try
{
_sendRequestCts = new CancellationTokenSource();
return SendRequest(async: false).GetAwaiter().GetResult();
return HandleResponse(async: false).GetAwaiter().GetResult();
}
catch (Exception ex)
{
Expand All @@ -1052,10 +1056,11 @@ public override WebResponse GetResponse()

public override Stream GetRequestStream()
{
CheckRequestStream();
return InternalGetRequestStream().Result;
}

private Task<Stream> InternalGetRequestStream()
private void CheckRequestStream()
{
CheckAbort();

Expand All @@ -1073,10 +1078,28 @@ private Task<Stream> InternalGetRequestStream()
{
throw new InvalidOperationException(SR.net_reqsubmitted);
}
}

_requestStream = new RequestStream();
private async Task<Stream> InternalGetRequestStream()
{
// If we aren't buffering we need to open the connection right away.
// Because we need to send the data as soon as possible when it's available from the RequestStream.
// Making this allows us to keep the sync send request path for buffering cases.
if (AllowWriteStreamBuffering is false)
{
// We're calling SendRequest with async, because we need to open the connection and send the request
// Otherwise, sync path will block the current thread until the request is sent.
TaskCompletionSource<Stream> getStreamTcs = new();
TaskCompletionSource completeTcs = new();
_sendRequestTask = SendRequest(async: true, new RequestStreamContent(getStreamTcs, completeTcs));
_requestStream = new RequestStream(await getStreamTcs.Task.ConfigureAwait(false), completeTcs);
}
else
{
_requestStream = new RequestBufferingStream();
}

return Task.FromResult((Stream)_requestStream);
return _requestStream;
}

public Stream EndGetRequestStream(IAsyncResult asyncResult, out TransportContext? context)
Expand All @@ -1100,6 +1123,8 @@ public override IAsyncResult BeginGetRequestStream(AsyncCallback? callback, obje
throw new InvalidOperationException(SR.net_repcall);
}

CheckRequestStream();

_requestStreamCallback = callback;
_requestStreamOperation = InternalGetRequestStream().ToApm(callback, state);

Expand Down Expand Up @@ -1133,78 +1158,95 @@ public override Stream EndGetRequestStream(IAsyncResult asyncResult)
return stream;
}

private async Task<WebResponse> SendRequest(bool async)
private Task<HttpResponseMessage> SendRequest(bool async, HttpContent? content = null)
{
if (RequestSubmitted)
{
throw new InvalidOperationException(SR.net_reqsubmitted);
}

var request = new HttpRequestMessage(HttpMethod.Parse(_originVerb), _requestUri);
_sendRequestMessage = new HttpRequestMessage(HttpMethod.Parse(_originVerb), _requestUri);
_sendRequestCts = new CancellationTokenSource();
_httpClient = GetCachedOrCreateHttpClient(async, out _disposeRequired);

bool disposeRequired = false;
HttpClient? client = null;
try
if (content is not null)
{
_sendRequestMessage.Content = content;
}

if (_hostUri is not null)
{
client = GetCachedOrCreateHttpClient(async, out disposeRequired);
if (_requestStream != null)
_sendRequestMessage.Headers.Host = Host;
}

AddCacheControlHeaders(_sendRequestMessage);

// Copy the HttpWebRequest request headers from the WebHeaderCollection into HttpRequestMessage.Headers and
// HttpRequestMessage.Content.Headers.
foreach (string headerName in _webHeaderCollection)
{
// The System.Net.Http APIs require HttpRequestMessage headers to be properly divided between the request headers
// collection and the request content headers collection for all well-known header names. And custom headers
// are only allowed in the request headers collection and not in the request content headers collection.
if (IsWellKnownContentHeader(headerName))
{
ArraySegment<byte> bytes = _requestStream.GetBuffer();
request.Content = new ByteArrayContent(bytes.Array!, bytes.Offset, bytes.Count);
_sendRequestMessage.Content ??= new ByteArrayContent(Array.Empty<byte>());
_sendRequestMessage.Content.Headers.TryAddWithoutValidation(headerName, _webHeaderCollection[headerName!]);
}

if (_hostUri != null)
else
{
request.Headers.Host = Host;
_sendRequestMessage.Headers.TryAddWithoutValidation(headerName, _webHeaderCollection[headerName!]);
}
}

AddCacheControlHeaders(request);
if (_servicePoint?.Expect100Continue == true)
{
_sendRequestMessage.Headers.ExpectContinue = true;
}

// Copy the HttpWebRequest request headers from the WebHeaderCollection into HttpRequestMessage.Headers and
// HttpRequestMessage.Content.Headers.
foreach (string headerName in _webHeaderCollection)
{
// The System.Net.Http APIs require HttpRequestMessage headers to be properly divided between the request headers
// collection and the request content headers collection for all well-known header names. And custom headers
// are only allowed in the request headers collection and not in the request content headers collection.
if (IsWellKnownContentHeader(headerName))
{
// Create empty content so that we can send the entity-body header.
request.Content ??= new ByteArrayContent(Array.Empty<byte>());
_sendRequestMessage.Headers.TransferEncodingChunked = SendChunked;

request.Content.Headers.TryAddWithoutValidation(headerName, _webHeaderCollection[headerName!]);
}
else
{
request.Headers.TryAddWithoutValidation(headerName, _webHeaderCollection[headerName!]);
}
}
if (KeepAlive)
{
_sendRequestMessage.Headers.Connection.Add(HttpKnownHeaderNames.KeepAlive);
}
else
{
_sendRequestMessage.Headers.ConnectionClose = true;
}

request.Headers.TransferEncodingChunked = SendChunked;
_sendRequestMessage.Version = ProtocolVersion;
HttpCompletionOption completionOption = _allowReadStreamBuffering ? HttpCompletionOption.ResponseContentRead : HttpCompletionOption.ResponseHeadersRead;
// If we're not buffering, there is no way to open the connection and not send the request without async.
// So we should use Async, if we're not buffering.
_sendRequestTask = async || !AllowWriteStreamBuffering ?
_httpClient.SendAsync(_sendRequestMessage, completionOption, _sendRequestCts.Token) :
Task.FromResult(_httpClient.Send(_sendRequestMessage, completionOption, _sendRequestCts.Token));

if (KeepAlive)
{
request.Headers.Connection.Add(HttpKnownHeaderNames.KeepAlive);
}
else
{
request.Headers.ConnectionClose = true;
}
return _sendRequestTask!;
}

if (_servicePoint?.Expect100Continue == true)
{
request.Headers.ExpectContinue = true;
}
private async Task<WebResponse> HandleResponse(bool async)
{
// If user code used requestStream and didn't dispose it
// We're completing it here.
if (_requestStream is RequestStream requestStream)
{
requestStream.Complete();
}

request.Version = ProtocolVersion;
if (_sendRequestTask is null && _requestStream is RequestBufferingStream requestBufferingStream)
{
ArraySegment<byte> buffer = requestBufferingStream.GetBuffer();
_sendRequestTask = SendRequest(async, new ByteArrayContent(buffer.Array!, buffer.Offset, buffer.Count));
}

_sendRequestTask = async ?
client.SendAsync(request, _allowReadStreamBuffering ? HttpCompletionOption.ResponseContentRead : HttpCompletionOption.ResponseHeadersRead, _sendRequestCts!.Token) :
Task.FromResult(client.Send(request, _allowReadStreamBuffering ? HttpCompletionOption.ResponseContentRead : HttpCompletionOption.ResponseHeadersRead, _sendRequestCts!.Token));
_sendRequestTask ??= SendRequest(async);

try
{
HttpResponseMessage responseMessage = await _sendRequestTask.ConfigureAwait(false);

HttpWebResponse response = new HttpWebResponse(responseMessage, _requestUri, _cookieContainer);
HttpWebResponse response = new(responseMessage, _requestUri, _cookieContainer);

int maxSuccessStatusCode = AllowAutoRedirect ? 299 : 399;
if ((int)response.StatusCode > maxSuccessStatusCode || (int)response.StatusCode < 200)
Expand All @@ -1220,9 +1262,15 @@ private async Task<WebResponse> SendRequest(bool async)
}
finally
{
if (disposeRequired)
_sendRequestMessage?.Dispose();
if (_requestStream is RequestBufferingStream bufferStream)
{
client?.Dispose();
bufferStream.GetMemoryStream().Dispose();
}

if (_disposeRequired)
{
_httpClient?.Dispose();
}
}
}
Expand Down Expand Up @@ -1348,9 +1396,8 @@ public override IAsyncResult BeginGetResponse(AsyncCallback? callback, object? s
throw new InvalidOperationException(SR.net_repcall);
}

_sendRequestCts = new CancellationTokenSource();
_responseCallback = callback;
_responseOperation = SendRequest(async: true).ToApm(callback, state);
_responseOperation = HandleResponse(async: true).ToApm(callback, state);

return _responseOperation.Task;
}
Expand Down
Loading