From 761b5f11900becb1155abd4a9d0f03b564f227ed Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 27 Sep 2017 16:53:53 -0400 Subject: [PATCH] Several HttpClient/ManagedHandler fixes to better support ClientWebSocket - Allow ws:// and wss:// schemes - Add an HttpContentDuplexStream base class to parallel the existing HttpContentReadStream and HttpContentWriteStream base classes - Move more shared functionality down to the base HttpContentStream class - Make ConnectionCloseStream duplex instead of read-only Commit migrated from https://github.com/dotnet/corefx/commit/9438b0a760b1ef827729662186e107abccb7524d --- .../System.Net.Http/src/System.Net.Http.csproj | 5 ++- .../src/System/Net/Http/HttpUtilities.cs | 17 ++++++-- .../System/Net/Http/Managed/AutoRedirectHandler.cs | 6 +-- ...CloseReadStream.cs => ConnectionCloseStream.cs} | 34 +++++++++------ .../src/System/Net/Http/Managed/HttpConnection.cs | 4 +- .../Net/Http/Managed/HttpConnectionContent.cs | 10 ++--- .../Net/Http/Managed/HttpConnectionHandler.cs | 2 +- .../System/Net/Http/Managed/HttpConnectionKey.cs | 6 +-- .../Net/Http/Managed/HttpContentDuplexStream.cs | 31 ++++++++++++++ .../Net/Http/Managed/HttpContentReadStream.cs | 32 +------------- .../System/Net/Http/Managed/HttpContentStream.cs | 49 ++++++++++++++++++++++ .../Net/Http/Managed/HttpContentWriteStream.cs | 32 +------------- .../Net/Http/Managed/HttpProxyConnectionHandler.cs | 2 +- 13 files changed, 135 insertions(+), 95 deletions(-) rename src/libraries/System.Net.Http/src/System/Net/Http/Managed/{ConnectionCloseReadStream.cs => ConnectionCloseStream.cs} (57%) create mode 100644 src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentDuplexStream.cs diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index 0ffd402..a7c5d03 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -127,7 +127,7 @@ - + @@ -140,6 +140,7 @@ + @@ -461,4 +462,4 @@ - \ No newline at end of file + diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs index 252e1c3..c81754e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs @@ -25,12 +25,21 @@ namespace System.Net.Http internal static bool IsHttpUri(Uri uri) { Debug.Assert(uri != null); - - string scheme = uri.Scheme; - return string.Equals("http", scheme, StringComparison.OrdinalIgnoreCase) || - string.Equals("https", scheme, StringComparison.OrdinalIgnoreCase); + return IsSupportedScheme(uri.Scheme); } + internal static bool IsSupportedScheme(string scheme) => + IsSupportedNonSecureScheme(scheme) || + IsSupportedSecureScheme(scheme); + + internal static bool IsSupportedNonSecureScheme(string scheme) => + string.Equals(scheme, "http", StringComparison.OrdinalIgnoreCase) || + string.Equals(scheme, "ws", StringComparison.OrdinalIgnoreCase); + + internal static bool IsSupportedSecureScheme(string scheme) => + string.Equals(scheme, "https", StringComparison.OrdinalIgnoreCase) || + string.Equals(scheme, "wss", StringComparison.OrdinalIgnoreCase); + // Returns true if the task was faulted or canceled and sets tcs accordingly. internal static bool HandleFaultsAndCancelation(Task task, TaskCompletionSource tcs) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/AutoRedirectHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/AutoRedirectHandler.cs index 193add2..a35175a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/AutoRedirectHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/AutoRedirectHandler.cs @@ -80,10 +80,10 @@ namespace System.Net.Http location = new Uri(request.RequestUri, location); } - // Disallow automatic redirection from https to http + // Disallow automatic redirection from secure to non-secure schemes bool allowed = - (request.RequestUri.Scheme == UriScheme.Http && (location.Scheme == UriScheme.Http || location.Scheme == UriScheme.Https)) || - (request.RequestUri.Scheme == UriScheme.Https && location.Scheme == UriScheme.Https); + (HttpUtilities.IsSupportedNonSecureScheme(request.RequestUri.Scheme) && HttpUtilities.IsSupportedScheme(location.Scheme)) || + (HttpUtilities.IsSupportedSecureScheme(request.RequestUri.Scheme) && HttpUtilities.IsSupportedSecureScheme(location.Scheme)); if (!allowed) { break; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/ConnectionCloseReadStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/ConnectionCloseStream.cs similarity index 57% rename from src/libraries/System.Net.Http/src/System/Net/Http/Managed/ConnectionCloseReadStream.cs rename to src/libraries/System.Net.Http/src/System/Net/Http/Managed/ConnectionCloseStream.cs index 9cc75e9..5a3ad9a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/ConnectionCloseReadStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/ConnectionCloseStream.cs @@ -10,10 +10,9 @@ namespace System.Net.Http { internal sealed partial class HttpConnection : IDisposable { - private sealed class ConnectionCloseReadStream : HttpContentReadStream + private sealed class ConnectionCloseStream : HttpContentDuplexStream { - public ConnectionCloseReadStream(HttpConnection connection) - : base(connection) + public ConnectionCloseStream(HttpConnection connection) : base(connection) { } @@ -28,7 +27,6 @@ namespace System.Net.Http } int bytesRead = await _connection.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false); - if (bytesRead == 0) { // We cannot reuse this connection, so close it. @@ -40,6 +38,19 @@ namespace System.Net.Http return bytesRead; } + public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + ValidateBufferArgs(buffer, offset, count); + return + _connection == null ? Task.FromException(new IOException(SR.net_http_io_write)) : + count > 0 ? _connection.WriteWithoutBufferingAsync(buffer, offset, count, cancellationToken) : + Task.CompletedTask; + } + + public override Task FlushAsync(CancellationToken cancellationToken) => + _connection != null ? _connection.FlushAsync(cancellationToken) : + Task.CompletedTask; + public override async Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) { if (destination == null) @@ -47,17 +58,14 @@ namespace System.Net.Http throw new ArgumentNullException(nameof(destination)); } - if (_connection == null) + if (_connection != null) // null if response body fully consumed { - // Response body fully consumed - return; - } - - await _connection.CopyToAsync(destination, cancellationToken).ConfigureAwait(false); + await _connection.CopyToAsync(destination, cancellationToken).ConfigureAwait(false); - // We cannot reuse this connection, so close it. - _connection.Dispose(); - _connection = null; + // We cannot reuse this connection, so close it. + _connection.Dispose(); + _connection = null; + } } } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnection.cs index 077a004..56188e6 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnection.cs @@ -347,7 +347,7 @@ namespace System.Net.Http } // Create the response stream. - HttpContentReadStream responseStream; + HttpContentStream responseStream; if (request.Method == HttpMethod.Head || (int)response.StatusCode == 204 || (int)response.StatusCode == 304) { responseStream = EmptyReadStream.Instance; @@ -372,7 +372,7 @@ namespace System.Net.Http } else { - responseStream = new ConnectionCloseReadStream(this); + responseStream = new ConnectionCloseStream(this); } ((HttpConnectionContent)response.Content).SetStream(responseStream); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionContent.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionContent.cs index 6ce4fea..bf6bfb5 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionContent.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionContent.cs @@ -14,14 +14,14 @@ namespace System.Net.Http private sealed class HttpConnectionContent : HttpContent { private readonly CancellationToken _cancellationToken; - private HttpContentReadStream _stream; + private HttpContentStream _stream; public HttpConnectionContent(CancellationToken cancellationToken) { _cancellationToken = cancellationToken; } - public void SetStream(HttpContentReadStream stream) + public void SetStream(HttpContentStream stream) { Debug.Assert(stream != null); Debug.Assert(stream.CanRead); @@ -29,14 +29,14 @@ namespace System.Net.Http _stream = stream; } - private HttpContentReadStream ConsumeStream() + private HttpContentStream ConsumeStream() { if (_stream == null) { throw new InvalidOperationException(SR.net_http_content_stream_already_read); } - HttpContentReadStream stream = _stream; + HttpContentStream stream = _stream; _stream = null; return stream; } @@ -45,7 +45,7 @@ namespace System.Net.Http { Debug.Assert(stream != null); - using (HttpContentReadStream contentStream = ConsumeStream()) + using (HttpContentStream contentStream = ConsumeStream()) { const int BufferSize = 8192; await contentStream.CopyToAsync(stream, BufferSize, _cancellationToken).ConfigureAwait(false); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionHandler.cs index 67d9010..c063548 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionHandler.cs @@ -89,7 +89,7 @@ namespace System.Net.Http TransportContext transportContext = null; - if (uri.Scheme == UriScheme.Https) + if (HttpUtilities.IsSupportedSecureScheme(uri.Scheme)) { SslStream sslStream = await EstablishSslConnection(uri.IdnHost, request, stream).ConfigureAwait(false); stream = sslStream; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionKey.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionKey.cs index d4e4185..7d991dc 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionKey.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpConnectionKey.cs @@ -12,9 +12,9 @@ namespace System.Net.Http public HttpConnectionKey(Uri uri) { - UsingSSL = - uri.Scheme == UriScheme.Http ? false : - uri.Scheme == UriScheme.Https ? true : + UsingSSL = + HttpUtilities.IsSupportedNonSecureScheme(uri.Scheme) ? false : + HttpUtilities.IsSupportedSecureScheme(uri.Scheme) ? true : throw new ArgumentException(SR.net_http_client_http_baseaddress_required, nameof(uri)); Host = uri.Host; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentDuplexStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentDuplexStream.cs new file mode 100644 index 0000000..4fa5b26 --- /dev/null +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentDuplexStream.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Diagnostics; +using System.IO; +using System.Threading; + +namespace System.Net.Http +{ + internal abstract class HttpContentDuplexStream : HttpContentStream + { + public HttpContentDuplexStream(HttpConnection connection) : base(connection) + { + } + + public override bool CanRead => true; + public override bool CanWrite => true; + + public override void Flush() => FlushAsync().GetAwaiter().GetResult(); + + public override int Read(byte[] buffer, int offset, int count) => + ReadAsync(buffer, offset, count, CancellationToken.None).GetAwaiter().GetResult(); + + public override void Write(byte[] buffer, int offset, int count) => + WriteAsync(buffer, offset, count, CancellationToken.None).GetAwaiter().GetResult(); + + public override void CopyTo(Stream destination, int bufferSize) => + CopyToAsync(destination, bufferSize, CancellationToken.None).GetAwaiter().GetResult(); + } +} diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentReadStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentReadStream.cs index 795c143..62c48b3 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentReadStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentReadStream.cs @@ -9,30 +9,14 @@ namespace System.Net.Http { internal abstract class HttpContentReadStream : HttpContentStream { - protected HttpConnection _connection; - - public HttpContentReadStream(HttpConnection connection) + public HttpContentReadStream(HttpConnection connection) : base(connection) { - _connection = connection; } public override bool CanRead => true; - public override bool CanSeek => false; public override bool CanWrite => false; - public override long Length => throw new NotSupportedException(); - - public override long Position - { - get { throw new NotSupportedException(); } - set { throw new NotSupportedException(); } - } - - public override void Flush() => throw new NotSupportedException(); - - public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); - - public override void SetLength(long value) => throw new NotSupportedException(); + public override void Flush() { } public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); @@ -41,17 +25,5 @@ namespace System.Net.Http public override void CopyTo(Stream destination, int bufferSize) => CopyToAsync(destination, bufferSize, CancellationToken.None).GetAwaiter().GetResult(); - - protected override void Dispose(bool disposing) - { - if (_connection != null) - { - // We haven't finished reading the body, so close the connection. - _connection.Dispose(); - _connection = null; - } - - base.Dispose(disposing); - } } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentStream.cs index 991ea06..b54f6c4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentStream.cs @@ -3,11 +3,60 @@ // See the LICENSE file in the project root for more information. using System.IO; +using System.Threading; +using System.Threading.Tasks; namespace System.Net.Http { internal abstract class HttpContentStream : Stream { + protected HttpConnection _connection; + + public HttpContentStream(HttpConnection connection) + { + _connection = connection; + } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + if (_connection != null) + { + _connection.Dispose(); + _connection = null; + } + } + + base.Dispose(disposing); + } + + public override bool CanSeek => false; + + public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback callback, object state) => + TaskToApm.Begin(ReadAsync(buffer, offset, count, default(CancellationToken)), callback, state); + + public override int EndRead(IAsyncResult asyncResult) => + TaskToApm.End(asyncResult); + + public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback callback, object state) => + TaskToApm.Begin(WriteAsync(buffer, offset, count, default(CancellationToken)), callback, state); + + public override void EndWrite(IAsyncResult asyncResult) => + TaskToApm.End(asyncResult); + + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + public override void SetLength(long value) => throw new NotSupportedException(); + + public override long Length => throw new NotSupportedException(); + + public override long Position + { + get { throw new NotSupportedException(); } + set { throw new NotSupportedException(); } + } + protected static void ValidateBufferArgs(byte[] buffer, int offset, int count) { if (buffer == null) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentWriteStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentWriteStream.cs index a998366..54c68df4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentWriteStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpContentWriteStream.cs @@ -11,12 +11,9 @@ namespace System.Net.Http { internal abstract class HttpContentWriteStream : HttpContentStream { - protected HttpConnection _connection; - - public HttpContentWriteStream(HttpConnection connection, CancellationToken cancellationToken) + public HttpContentWriteStream(HttpConnection connection, CancellationToken cancellationToken) : base(connection) { Debug.Assert(connection != null); - _connection = connection; RequestCancellationToken = cancellationToken; } @@ -29,42 +26,15 @@ namespace System.Net.Http internal CancellationToken RequestCancellationToken { get; } public override bool CanRead => false; - public override bool CanSeek => false; public override bool CanWrite => true; - public override long Length => throw new NotSupportedException(); - - public override long Position - { - get { throw new NotSupportedException(); } - set { throw new NotSupportedException(); } - } - public override void Flush() => FlushAsync().GetAwaiter().GetResult(); - public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); - - public override void SetLength(long value) => throw new NotSupportedException(); - public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException(); public override void Write(byte[] buffer, int offset, int count) => WriteAsync(buffer, offset, count, CancellationToken.None).GetAwaiter().GetResult(); public abstract Task FinishAsync(); - - protected override void Dispose(bool disposing) - { - if (disposing) - { - if (_connection != null) - { - _connection.Dispose(); - _connection = null; - } - } - - base.Dispose(disposing); - } } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpProxyConnectionHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpProxyConnectionHandler.cs index 82374d3..db807d4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpProxyConnectionHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Managed/HttpProxyConnectionHandler.cs @@ -59,7 +59,7 @@ namespace System.Net.Http throw new InvalidOperationException(SR.net_http_invalid_proxy_scheme); } - if (request.RequestUri.Scheme == UriScheme.Https) + if (!HttpUtilities.IsSupportedNonSecureScheme(request.RequestUri.Scheme)) { // TODO #23136: Implement SSL tunneling through proxy throw new NotImplementedException("no support for SSL tunneling through proxy"); -- 2.7.4