diff --git a/common/src/service/HttpClientHelper.cs b/common/src/service/HttpClientHelper.cs index 1233933a61..b574e42c7b 100644 --- a/common/src/service/HttpClientHelper.cs +++ b/common/src/service/HttpClientHelper.cs @@ -10,8 +10,10 @@ using System.Net; using System.Net.Http; using System.Net.Http.Headers; +using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Tasks; +using Microsoft.Azure.Amqp.Transport; using Microsoft.Azure.Devices.Common; using Microsoft.Azure.Devices.Common.Exceptions; using Microsoft.Azure.Devices.Common.Extensions; @@ -885,26 +887,39 @@ internal static HttpClient CreateDefaultClient(IWebProxy webProxy, Uri baseUri, internal static HttpMessageHandler CreateDefaultHttpMessageHandler(IWebProxy webProxy, Uri baseUri, int connectionLeaseTimeoutMilliseconds) { -#pragma warning disable CA2000 // Dispose objects before losing scope (object is returned by this method, so the caller is responsible for disposing it) -#if NETCOREAPP && !NETCOREAPP2_0 && !NETCOREAPP1_0 && !NETCOREAPP1_1 - // SocketsHttpHandler is only available in netcoreapp2.1 and onwards - var httpMessageHandler = new SocketsHttpHandler(); - httpMessageHandler.SslOptions.EnabledSslProtocols = TlsVersions.Instance.Preferred; + HttpMessageHandler httpMessageHandler = null; + +#if NETCOREAPP2_1_OR_GREATER || NET5_0_OR_GREATER + var socketsHandler = new SocketsHttpHandler(); + socketsHandler.SslOptions.EnabledSslProtocols = TlsVersions.Instance.Preferred; + + if (!TlsVersions.Instance.CertificateRevocationCheck) + { + socketsHandler.SslOptions.CertificateRevocationCheckMode = X509RevocationMode.NoCheck; + } + + if (webProxy != DefaultWebProxySettings.Instance) + { + socketsHandler.UseProxy = webProxy != null; + socketsHandler.Proxy = webProxy; + } + + httpMessageHandler = socketsHandler; #else - var httpMessageHandler = new HttpClientHandler(); + var httpClientHandler = new HttpClientHandler(); #if !NET451 - httpMessageHandler.SslProtocols = TlsVersions.Instance.Preferred; - httpMessageHandler.CheckCertificateRevocationList = TlsVersions.Instance.CertificateRevocationCheck; + httpClientHandler.SslProtocols = TlsVersions.Instance.Preferred; + httpClientHandler.CheckCertificateRevocationList = TlsVersions.Instance.CertificateRevocationCheck; #endif -#endif -#pragma warning restore CA2000 // Dispose objects before losing scope - if (webProxy != DefaultWebProxySettings.Instance) { - httpMessageHandler.UseProxy = webProxy != null; - httpMessageHandler.Proxy = webProxy; + httpClientHandler.UseProxy = webProxy != null; + httpClientHandler.Proxy = webProxy; } + httpMessageHandler = httpClientHandler; +#endif + ServicePointHelpers.SetLimits(httpMessageHandler, baseUri, connectionLeaseTimeoutMilliseconds); return httpMessageHandler; diff --git a/e2e/test/iothub/FileUploadE2ETests.cs b/e2e/test/iothub/FileUploadE2ETests.cs index f522c26f11..dec61bb1cf 100644 --- a/e2e/test/iothub/FileUploadE2ETests.cs +++ b/e2e/test/iothub/FileUploadE2ETests.cs @@ -4,9 +4,13 @@ using System; using System.IO; using System.Net; +using System.Net.Http; using System.Security.Cryptography.X509Certificates; +using System.Threading; using System.Threading.Tasks; +using FluentAssertions; using Microsoft.Azure.Devices.Client; +using Microsoft.Azure.Devices.Client.Exceptions; using Microsoft.Azure.Devices.Client.Transport; using Microsoft.Azure.Devices.E2ETests.Helpers; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -127,6 +131,42 @@ public async Task FileUpload_SmallFile_Http_GranularSteps_Proxy() await UploadFileGranularAsync(fileStreamSource, filename, fileUploadTransportSettings).ConfigureAwait(false); } + // File upload requests can be configured to use a user-provided HttpClient + [TestMethod] + public async Task FileUpload_UsesCustomHttpClient() + { + using TestDevice testDevice = + await TestDevice.GetTestDeviceAsync(_devicePrefix, TestDeviceType.Sasl).ConfigureAwait(false); + + using var CustomHttpMessageHandler = new CustomHttpMessageHandler(); + var fileUploadSettings = new Http1TransportSettings() + { + // This HttpClient should throw a NotImplementedException whenever it makes an HTTP + // request + HttpClient = new HttpClient(CustomHttpMessageHandler), + }; + + var clientOptions = new ClientOptions() + { + FileUploadTransportSettings = fileUploadSettings, + }; + + using var deviceClient = + DeviceClient.CreateFromConnectionString(testDevice.ConnectionString, clientOptions); + + await deviceClient.OpenAsync().ConfigureAwait(false); + + var request = new FileUploadSasUriRequest() + { + BlobName = "someBlobName", + }; + var ex = await Assert.ThrowsExceptionAsync( + async () => await deviceClient.GetFileUploadSasUriAsync(request).ConfigureAwait(false)); + + ex.InnerException.Should().BeOfType( + "The provided custom HttpMessageHandler throws NotImplementedException when making any HTTP request"); + } + private async Task UploadFileGranularAsync(Stream source, string filename, Http1TransportSettings fileUploadTransportSettings, bool useX509auth = false) { using TestDevice testDevice = await TestDevice.GetTestDeviceAsync( @@ -265,6 +305,14 @@ private static async Task GetTestFileNameAsync(int fileSize) return filePath; } + private class CustomHttpMessageHandler : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + throw new NotImplementedException("Deliberately not implemented for test purposes"); + } + } + [ClassCleanup] public static void CleanupCertificates() { diff --git a/e2e/test/iothub/messaging/MessageSendE2ETests.cs b/e2e/test/iothub/messaging/MessageSendE2ETests.cs index bf4aea40f7..8c45dddad7 100644 --- a/e2e/test/iothub/messaging/MessageSendE2ETests.cs +++ b/e2e/test/iothub/messaging/MessageSendE2ETests.cs @@ -3,9 +3,12 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Net; +using System.Net.Http; using System.Text; +using System.Threading; using System.Threading.Tasks; using FluentAssertions; using Microsoft.Azure.Devices.Client; @@ -416,6 +419,31 @@ public async Task Message_DeviceSendMessageWayOverAllowedSize_Http() await SendSingleMessage(TestDeviceType.Sasl, Client.TransportType.Http1, OverlyExceedAllowedMessageSizeInBytes).ConfigureAwait(false); } + [TestMethod] + [Timeout(TestTimeoutMilliseconds)] + public async Task Message_DeviceSendSingleWithCustomHttpClient_Http() + { + using var httpMessageHandler = new CustomHttpMessageHandler(); + var httpTransportSettings = new Http1TransportSettings() + { + HttpClient = new HttpClient(httpMessageHandler) + }; + var transportSettings = new ITransportSettings[] { httpTransportSettings }; + + using TestDevice testDevice = + await TestDevice.GetTestDeviceAsync(_devicePrefix).ConfigureAwait(false); + + using DeviceClient deviceClient = testDevice.CreateDeviceClient(transportSettings); + + await deviceClient.OpenAsync().ConfigureAwait(false); + using var message = new Client.Message(); + var ex = await Assert.ThrowsExceptionAsync( + async () => await deviceClient.SendEventAsync(message).ConfigureAwait(false)); + + ex.InnerException.Should().BeOfType( + "The provided custom HttpMessageHandler throws NotImplementedException when making any HTTP request"); + } + private async Task SendSingleMessage(TestDeviceType type, Client.TransportType transport, int messageSize = 0) { using TestDevice testDevice = await TestDevice.GetTestDeviceAsync(_devicePrefix, type).ConfigureAwait(false); @@ -532,5 +560,13 @@ public static Client.Message ComposeD2cTestMessageOfSpecifiedSize(int messageSiz return message; } + + private class CustomHttpMessageHandler : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + throw new NotImplementedException("Deliberately not implemented for test purposes"); + } + } } } diff --git a/iothub/device/src/ClientSettings/Http1TransportSettings.cs b/iothub/device/src/ClientSettings/Http1TransportSettings.cs index dfec60ba74..1de6867f79 100644 --- a/iothub/device/src/ClientSettings/Http1TransportSettings.cs +++ b/iothub/device/src/ClientSettings/Http1TransportSettings.cs @@ -3,6 +3,7 @@ using System; using System.Net; +using System.Net.Http; using System.Security.Cryptography.X509Certificates; using Microsoft.Azure.Devices.Shared; @@ -47,5 +48,20 @@ public TransportType GetTransportType() /// public IWebProxy Proxy { get; set; } + + /// + /// The HTTP client to use for all HTTP operations. + /// + /// + /// If not provided, an HTTP client will be created for you based on the other settings provided. + /// + /// This HTTP client instance will be disposed when the device/module client using it is disposed. + /// + /// + /// If provided, all other HTTP-specific settings (such as proxy, SSL protocols, and certificate revocation check) + /// on this class will be ignored and must be specified on this HttpClient instance. + /// + /// + public HttpClient HttpClient { get; set; } } } diff --git a/iothub/device/src/ModuleClient.cs b/iothub/device/src/ModuleClient.cs index be97cced23..c2b4a815ab 100644 --- a/iothub/device/src/ModuleClient.cs +++ b/iothub/device/src/ModuleClient.cs @@ -36,6 +36,7 @@ public class ModuleClient : IDisposable private const string DeviceMethodUriFormat = "/twins/{0}/methods?" + ClientApiVersionHelper.ApiVersionQueryStringLatest; private readonly bool _isAnEdgeModule; private readonly ICertificateValidator _certValidator; + private HttpTransportHandler _httpTransportHandler; internal InternalClient InternalClient { get; private set; } @@ -514,6 +515,7 @@ public void Dispose() { Dispose(true); GC.SuppressFinalize(this); + _httpTransportHandler?.Dispose(); } #if !NET451 && !NET472 && !NETSTANDARD2_0 @@ -827,33 +829,53 @@ private async Task InvokeMethodAsync(Uri uri, MethodRequest meth { throw new ObjectDisposedException("IoT client", DefaultDelegatingHandler.ClientDisposedMessage); } - methodRequest.ThrowIfNull(nameof(methodRequest)); - HttpClientHandler httpClientHandler = null; - Func customCertificateValidation = _certValidator.GetCustomCertificateValidation(); + methodRequest.ThrowIfNull(nameof(methodRequest)); - try + if (_httpTransportHandler == null) { + Func customCertificateValidation = _certValidator.GetCustomCertificateValidation(); + + // The HTTP message handlers created in this block are disposed when this client is + // disposed. +#pragma warning disable CA2000 // Dispose objects before losing scope +#if !NET451 + var httpMessageHandler = new HttpClientHandler(); +#else + var httpMessageHandler = new WebRequestHandler(); +#endif +#pragma warning restore CA2000 // Dispose objects before losing scope + if (customCertificateValidation != null) { TlsVersions.Instance.SetLegacyAcceptableVersions(); #if !NET451 - httpClientHandler = new HttpClientHandler - { - ServerCertificateCustomValidationCallback = customCertificateValidation, - SslProtocols = TlsVersions.Instance.Preferred, - }; + httpMessageHandler.ServerCertificateCustomValidationCallback = customCertificateValidation; + httpMessageHandler.SslProtocols = TlsVersions.Instance.Preferred; #else - httpClientHandler = new WebRequestHandler(); - ((WebRequestHandler)httpClientHandler).ServerCertificateValidationCallback = (sender, certificate, chain, errors) => + httpMessageHandler.ServerCertificateValidationCallback = (sender, certificate, chain, errors) => { return customCertificateValidation(sender, certificate, chain, errors); }; #endif } - var context = new PipelineContext() + // We need to add the certificate to the httpTransport if DeviceAuthenticationWithX509Certificate + if (InternalClient.Certificate != null) + { + httpMessageHandler.ClientCertificates.Add(InternalClient.Certificate); + } + + // Note that this client is ignoring any HttpTransportSettings that the user may have + // provided. This is because the kinds of settings a user would want to override + // aren't as applicable for this particular operation. + var transportSettings = new Http1TransportSettings() + { + HttpClient = new HttpClient(httpMessageHandler, true) + }; + + var context = new PipelineContext { ProductInfo = new ProductInfo { @@ -861,23 +883,12 @@ private async Task InvokeMethodAsync(Uri uri, MethodRequest meth } }; - var transportSettings = new Http1TransportSettings(); - //We need to add the certificate to the httpTransport if DeviceAuthenticationWithX509Certificate - if (InternalClient.Certificate != null) - { - transportSettings.ClientCertificate = InternalClient.Certificate; - } - - using var httpTransport = new HttpTransportHandler(context, InternalClient.IotHubConnectionString, transportSettings, httpClientHandler); - var methodInvokeRequest = new MethodInvokeRequest(methodRequest.Name, methodRequest.DataAsJson, methodRequest.ResponseTimeout, methodRequest.ConnectionTimeout); - MethodInvokeResponse result = await httpTransport.InvokeMethodAsync(methodInvokeRequest, uri, cancellationToken).ConfigureAwait(false); - - return new MethodResponse(Encoding.UTF8.GetBytes(result.GetPayloadAsJson()), result.Status); - } - finally - { - httpClientHandler?.Dispose(); + _httpTransportHandler = new HttpTransportHandler(context, InternalClient.IotHubConnectionString, transportSettings); } + + var methodInvokeRequest = new MethodInvokeRequest(methodRequest.Name, methodRequest.DataAsJson, methodRequest.ResponseTimeout, methodRequest.ConnectionTimeout); + MethodInvokeResponse result = await _httpTransportHandler.InvokeMethodAsync(methodInvokeRequest, uri, cancellationToken).ConfigureAwait(false); + return new MethodResponse(Encoding.UTF8.GetBytes(result.GetPayloadAsJson()), result.Status); } private static Uri GetDeviceMethodUri(string deviceId) diff --git a/iothub/device/src/Transport/Http/HttpClientHelper.cs b/iothub/device/src/Transport/Http/HttpClientHelper.cs index edf4aef573..60748207f9 100644 --- a/iothub/device/src/Transport/Http/HttpClientHelper.cs +++ b/iothub/device/src/Transport/Http/HttpClientHelper.cs @@ -38,35 +38,16 @@ internal sealed class HttpClientHelper : IHttpClientHelper private readonly IReadOnlyDictionary>> _defaultErrorMapping; private readonly bool _usingX509ClientCert; private HttpClient _httpClientObj; - private HttpClientHandler _httpClientHandler; private bool _isDisposed; private readonly ProductInfo _productInfo; private readonly bool _isClientPrimaryTransportHandler; - // These default values are consistent with Azure.Core default values: - // https://github.com/Azure/azure-sdk-for-net/blob/7e3cf643977591e9041f4c628fd4d28237398e0b/sdk/core/Azure.Core/src/Pipeline/ServicePointHelpers.cs#L28 - private const int DefaultMaxConnectionsPerServer = 50; - - // How long, in milliseconds, a given cached TCP connection created by this client's HTTP layer will live before being closed. - // If this value is set to any negative value, the connection lease will be infinite. If this value is set to 0, then the TCP connection will close after - // each HTTP request and a new TCP connection will be opened upon the next request. - // - // By closing cached TCP connections and opening a new one upon the next request, the underlying HTTP client has a chance to do a DNS lookup - // to validate that it will send the requests to the correct IP address. While it is atypical for a given IoT hub to change its IP address, it does - // happen when a given IoT hub fails over into a different region. - // - // This default value is consistent with the default value used in Azure.Core - // https://github.com/Azure/azure-sdk-for-net/blob/7e3cf643977591e9041f4c628fd4d28237398e0b/sdk/core/Azure.Core/src/Pipeline/ServicePointHelpers.cs#L29 - private static readonly TimeSpan s_defaultConnectionLeaseTimeout = TimeSpan.FromMinutes(5); - public HttpClientHelper( Uri baseAddress, IAuthorizationProvider authenticationHeaderProvider, IDictionary>> defaultErrorMapping, TimeSpan timeout, - Action preRequestActionForAllRequests, - X509Certificate2 clientCert, - HttpClientHandler httpClientHandler, + Http1TransportSettings transportSettings, ProductInfo productInfo, IWebProxy proxy, bool isClientPrimaryTransportHandler = false) @@ -74,65 +55,94 @@ public HttpClientHelper( _baseAddress = baseAddress; _authenticationHeaderProvider = authenticationHeaderProvider; _defaultErrorMapping = new ReadOnlyDictionary>>(defaultErrorMapping); + _productInfo = productInfo; + _isClientPrimaryTransportHandler = isClientPrimaryTransportHandler; + + if (transportSettings.HttpClient != null) + { + _httpClientObj = transportSettings.HttpClient; + return; + } + HttpMessageHandler httpMessageHandler; + _usingX509ClientCert = transportSettings.ClientCertificate != null; #if NET451 TlsVersions.Instance.SetLegacyAcceptableVersions(); - _httpClientHandler = httpClientHandler; +#pragma warning disable CA2000 // Dispose objects before losing scope + // This disposable handler is disposed when the HttpClient it is given to is disposed + var webRequestHandler = new WebRequestHandler(); +#pragma warning restore CA2000 // Dispose objects before losing scope - if (clientCert != null) + if (_usingX509ClientCert) { - if (_httpClientHandler == null) - { - _httpClientHandler = new WebRequestHandler(); - } - - (_httpClientHandler as WebRequestHandler).ClientCertificates.Add(clientCert); - _usingX509ClientCert = true; + webRequestHandler.ClientCertificates.Add(transportSettings.ClientCertificate); } if (proxy != DefaultWebProxySettings.Instance) { - if (_httpClientHandler == null) - { - _httpClientHandler = new WebRequestHandler(); - } + webRequestHandler.UseProxy = proxy != null; + webRequestHandler.Proxy = proxy; + } + + httpMessageHandler = webRequestHandler; +#elif NET5_0_OR_GREATER +#pragma warning disable CA2000 // Dispose objects before losing scope + // This disposable handler is disposed when the HttpClient it is given to is disposed + var socketsHandler = new SocketsHttpHandler(); +#pragma warning restore CA2000 // Dispose objects before losing scope + socketsHandler.SslOptions.EnabledSslProtocols = TlsVersions.Instance.Preferred; - _httpClientHandler.UseProxy = (proxy != null); - _httpClientHandler.Proxy = proxy; + if (!TlsVersions.Instance.CertificateRevocationCheck) + { + socketsHandler.SslOptions.CertificateRevocationCheckMode = X509RevocationMode.NoCheck; } -#else - _httpClientHandler = httpClientHandler ?? new HttpClientHandler(); - _httpClientHandler.SslProtocols = TlsVersions.Instance.Preferred; - _httpClientHandler.CheckCertificateRevocationList = TlsVersions.Instance.CertificateRevocationCheck; + if (_usingX509ClientCert) + { + socketsHandler.SslOptions.ClientCertificates = + new X509CertificateCollection() { transportSettings.ClientCertificate }; + } + + if (proxy != DefaultWebProxySettings.Instance) + { + socketsHandler.UseProxy = proxy != null; + socketsHandler.Proxy = proxy; + } + + httpMessageHandler = socketsHandler; +#else // cases other than Net 5.0+ and net451 +#pragma warning disable CA2000 // Dispose objects before losing scope + // This disposable handler is disposed when the HttpClient it is given to is disposed + var httpClientHandler = new HttpClientHandler(); +#pragma warning restore CA2000 // Dispose objects before losing scope + + httpClientHandler.SslProtocols = TlsVersions.Instance.Preferred; + httpClientHandler.CheckCertificateRevocationList = TlsVersions.Instance.CertificateRevocationCheck; - if (clientCert != null) + if (_usingX509ClientCert) { - _httpClientHandler.ClientCertificates.Add(clientCert); - _usingX509ClientCert = true; + httpClientHandler.ClientCertificates.Add(transportSettings.ClientCertificate); } if (proxy != DefaultWebProxySettings.Instance) { - _httpClientHandler.UseProxy = proxy != null; - _httpClientHandler.Proxy = proxy; + httpClientHandler.UseProxy = proxy != null; + httpClientHandler.Proxy = proxy; } + + httpMessageHandler = httpClientHandler; #endif - ServicePointHelpers.SetLimits(_httpClientHandler, _baseAddress); + ServicePointHelpers.SetLimits(httpMessageHandler, _baseAddress); - _httpClientObj = _httpClientHandler != null ? new HttpClient(_httpClientHandler) : new HttpClient(); + _httpClientObj = new HttpClient(httpMessageHandler, true); _httpClientObj.BaseAddress = _baseAddress; _httpClientObj.Timeout = timeout; _httpClientObj.DefaultRequestHeaders.Accept.Add( new MediaTypeWithQualityHeaderValue(CommonConstants.MediaTypeForDeviceManagementApis)); _httpClientObj.DefaultRequestHeaders.ExpectContinue = false; - - preRequestActionForAllRequests?.Invoke(_httpClientObj); - _productInfo = productInfo; - _isClientPrimaryTransportHandler = isClientPrimaryTransportHandler; } public Task GetAsync( @@ -556,14 +566,6 @@ public void Dispose() _httpClientObj = null; } - // HttpClientHandler that is used to create HttpClient will automatically be disposed when HttpClient is disposed - // But in case the client handler didn't end up being used by the HttpClient, we explicitly dispose it here. - if (_httpClientHandler != null) - { - _httpClientHandler?.Dispose(); - _httpClientHandler = null; - } - // The associated TokenRefresher should be disposed by the http client helper only when the http client // is the primary transport handler. // For eg. we create HttpTransportHandler instances for file upload operations even though the client might be diff --git a/iothub/device/src/Transport/Http/HttpTransportHandler.cs b/iothub/device/src/Transport/Http/HttpTransportHandler.cs index 12bebef9fe..e54a3beb1c 100644 --- a/iothub/device/src/Transport/Http/HttpTransportHandler.cs +++ b/iothub/device/src/Transport/Http/HttpTransportHandler.cs @@ -64,7 +64,6 @@ internal HttpTransportHandler( PipelineContext context, IotHubConnectionString iotHubConnectionString, Http1TransportSettings transportSettings, - HttpClientHandler httpClientHandler = null, bool isClientPrimaryTransportHandler = false) : base(context, transportSettings) { @@ -76,9 +75,7 @@ internal HttpTransportHandler( iotHubConnectionString, ExceptionHandlingHelper.GetDefaultErrorMapping(), s_defaultOperationTimeout, - null, - transportSettings.ClientCertificate, - httpClientHandler, + transportSettings, productInfo, transportSettings.Proxy, isClientPrimaryTransportHandler); diff --git a/iothub/device/src/Transport/Http/ServicePointHelpers.cs b/iothub/device/src/Transport/Http/ServicePointHelpers.cs index d1ff818287..ca95defefa 100644 --- a/iothub/device/src/Transport/Http/ServicePointHelpers.cs +++ b/iothub/device/src/Transport/Http/ServicePointHelpers.cs @@ -42,7 +42,10 @@ public static void SetLimits(HttpMessageHandler messageHandler, Uri baseUri, int servicePoint.ConnectionLeaseTimeout = connectionLeaseTimeoutMilliseconds; break; #if NETCOREAPP2_1_OR_GREATER || NET5_0_OR_GREATER - // SocketsHttpHandler is only available in netcore2.1 and onwards + // SocketsHttpHandler is only available in netcore2.1 and onwards and .NET 5 and onwards. + // This library does not target netcore2.1+ though, so there is no way to set these timeouts + // within this library for netcore2.1+ cases though. Users will need to pass in this handler + // themselves in netcore2.1+ cases case SocketsHttpHandler socketsHttpHandler: socketsHttpHandler.MaxConnectionsPerServer = DefaultMaxConnectionsPerServer; socketsHttpHandler.PooledConnectionLifetime = TimeSpan.FromMilliseconds(connectionLeaseTimeoutMilliseconds);