From 611dd425ea0f377cdd889485c1778af6ed8f8f13 Mon Sep 17 00:00:00 2001 From: Eric Dahlvang Date: Mon, 26 Feb 2018 14:37:28 -0800 Subject: [PATCH 1/4] ServiceClient updates Check that .Product is not null during merge of ProductInfoHeaderValues. Do not add Product to UserAgent if already present. Dispose of root handler if created. --- .../ClientRuntime/ServiceClient.cs | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs b/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs index a585969dabfbe..59069a92dd145 100644 --- a/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs +++ b/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. See License.txt in the project root for license information. namespace Microsoft.Rest @@ -44,6 +44,11 @@ public abstract class ServiceClient : IDisposable /// private bool _disposeHttpClient; + /// + /// Flag to track if the root handler needs to disposed + /// + private bool _disposeRootHandler; + /// /// Field used for ClientVersion property /// @@ -384,8 +389,11 @@ protected virtual void Dispose(bool disposing) { HttpClient.Dispose(); HttpClient = null; - } - + } + + if (_disposeRootHandler) + HttpClientHandler.Dispose(); + FirstMessageHandler = null; HttpClientHandler = null; } @@ -419,6 +427,8 @@ protected void InitializeHttpClient(HttpClient httpClient, HttpClientHandler htt if(httpClientHandler == null) { httpClientHandler = CreateRootHandler(); + //set flag to dispose of handler, since it was not supplied and had to be created here + _disposeRootHandler = true; } HttpClientHandler = httpClientHandler; @@ -472,8 +482,8 @@ public bool SetUserAgent(string productName, string version) if (!_disposed && HttpClient != null) { MergeUserAgentInfo(DefaultUserAgentInfoList); - string cleanedProductName = CleanUserAgentInfoEntry(productName); - HttpClient.DefaultRequestHeaders.UserAgent.Add(new ProductInfoHeaderValue(cleanedProductName, version)); + string cleanedProductName = CleanUserAgentInfoEntry(productName); + MergeUserAgentInfo(new ProductInfoHeaderValue(cleanedProductName, version)); return true; } @@ -501,14 +511,17 @@ private string CleanUserAgentInfoEntry(string infoEntry) /// private void MergeUserAgentInfo(List defaultUserAgentInfoList) { - // If you want to log ProductName in userAgent, it has to be without spaces - foreach(ProductInfoHeaderValue piHv in defaultUserAgentInfoList) { - if(!HttpClient.DefaultRequestHeaders.UserAgent.Any((hv) => hv.Product.Name.Equals(piHv.Product.Name, StringComparison.OrdinalIgnoreCase))); - { - HttpClient.DefaultRequestHeaders.UserAgent.Add(piHv); - } + MergeUserAgentInfo(piHv); + } + } + + private void MergeUserAgentInfo(ProductInfoHeaderValue piHv) + { + if (!HttpClient.DefaultRequestHeaders.UserAgent.Any((hv) => hv.Product != null && hv.Product.Name.Equals(piHv.Product.Name, StringComparison.OrdinalIgnoreCase))) + { + HttpClient.DefaultRequestHeaders.UserAgent.Add(piHv); } } } From 6f95e660ac6c0060b02994333d970e4c16a03ada Mon Sep 17 00:00:00 2001 From: Eric Dahlvang Date: Mon, 26 Feb 2018 14:41:08 -0800 Subject: [PATCH 2/4] Update CustomClientWithHttpClientTests Add tests to ensure ProductInfoHeaderValues are not added to UserAgent twice, and those constructed with 'comment' constructor are ignored during merging. --- .../CustomClientWithHttpClientTests.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/CustomClientWithHttpClientTests.cs b/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/CustomClientWithHttpClientTests.cs index 6f8c4d788cca6..ecbebecf88791 100644 --- a/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/CustomClientWithHttpClientTests.cs +++ b/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/CustomClientWithHttpClientTests.cs @@ -207,6 +207,48 @@ public void AddFxVersionHeaderInformation() Assert.Equal(defaultVersion.ToString(), "1.0.0.0"); } + /// + /// This is to verify if a HttpClient is passed, we add default userAgent information + /// inside defaultheaders of the passed in HttpClient ONLY once + /// + [Fact] + public void AddFxVersionHeaderInformationOnce() + { + //Version defaultVersion = null; + HttpClient hc = new HttpClient(new ContosoMessageHandler()); + ContosoServiceClient contosoServiceClient = new ContosoServiceClient(); + string productName = contosoServiceClient.GetType().FullName; + contosoServiceClient.Dispose(); + hc.DefaultRequestHeaders.UserAgent.Add(new ProductInfoHeaderValue(productName, "1.0.0.0")); + + contosoServiceClient = new ContosoServiceClient(hc, false); + HttpResponseMessage response = contosoServiceClient.DoSyncWork(); + contosoServiceClient.Dispose(); + + HttpHeaderValueCollection userAgentValueCollection = contosoServiceClient.HttpClient.DefaultRequestHeaders.UserAgent; + var productInfos = userAgentValueCollection.Where((p) => p.Product.Name.Equals(productName, StringComparison.OrdinalIgnoreCase)); + Assert.Single(productInfos); + } + + /// + /// This is to verify if a HttpClient is passed that contains ProductInfoHeaderValues with comments in + /// userAgent information, no exception occurs while merging + /// + [Fact] + public void IgnoreProductInfoHeaderValueCommentsDuringMerge() + { + const string comment = "(comment)"; + HttpClient hc = new HttpClient(new ContosoMessageHandler()); + hc.DefaultRequestHeaders.UserAgent.Add(new ProductInfoHeaderValue(comment)); + ContosoServiceClient contosoClient = new ContosoServiceClient(hc, false); + HttpResponseMessage response = contosoClient.DoSyncWork(); + + HttpHeaderValueCollection userAgentValueCollection = contosoClient.HttpClient.DefaultRequestHeaders.UserAgent; + contosoClient.Dispose(); + var productInfos = userAgentValueCollection.Where((p) => p.Product == null && p.Comment.Equals(comment, StringComparison.OrdinalIgnoreCase)); + Assert.Single(productInfos); + } + private HttpResponseMessage SendAndReceiveResponse(HttpClient httpClient) { // Create HTTP transport objects From e13313279e108e350289dc7373414bcac4e4602a Mon Sep 17 00:00:00 2001 From: Eric Dahlvang Date: Tue, 27 Feb 2018 15:52:01 -0800 Subject: [PATCH 3/4] Update ServiceClient.cs --- .../ClientRuntime/ServiceClient.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs b/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs index 59069a92dd145..43bf42048d818 100644 --- a/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs +++ b/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs @@ -481,9 +481,8 @@ public bool SetUserAgent(string productName, string version) { if (!_disposed && HttpClient != null) { - MergeUserAgentInfo(DefaultUserAgentInfoList); string cleanedProductName = CleanUserAgentInfoEntry(productName); - MergeUserAgentInfo(new ProductInfoHeaderValue(cleanedProductName, version)); + MergeUserAgentInfo(DefaultUserAgentInfoList.Concat( new[] { new ProductInfoHeaderValue(cleanedProductName, version) })); return true; } @@ -509,19 +508,14 @@ private string CleanUserAgentInfoEntry(string infoEntry) /// We do this because, now we accept passed in HttpClient. /// So for any reason the passed HttpClient has our default UserAgent info (based on key name), we will not verify and check the values and will honor those values /// - private void MergeUserAgentInfo(List defaultUserAgentInfoList) + private void MergeUserAgentInfo(IEnumerable defaultUserAgentInfoList) { foreach(ProductInfoHeaderValue piHv in defaultUserAgentInfoList) { - MergeUserAgentInfo(piHv); - } - } - - private void MergeUserAgentInfo(ProductInfoHeaderValue piHv) - { - if (!HttpClient.DefaultRequestHeaders.UserAgent.Any((hv) => hv.Product != null && hv.Product.Name.Equals(piHv.Product.Name, StringComparison.OrdinalIgnoreCase))) - { - HttpClient.DefaultRequestHeaders.UserAgent.Add(piHv); + if (!HttpClient.DefaultRequestHeaders.UserAgent.Any((hv) => hv.Product != null && hv.Product.Name.Equals(piHv.Product.Name, StringComparison.OrdinalIgnoreCase))) + { + HttpClient.DefaultRequestHeaders.UserAgent.Add(piHv); + } } } } From 46ae1771f4e1e6a420d31a67475556aed01aa082 Mon Sep 17 00:00:00 2001 From: Eric Dahlvang Date: Mon, 5 Mar 2018 22:18:11 -0800 Subject: [PATCH 4/4] Dispose root handler in constructors that create it --- src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs b/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs index 43bf42048d818..0d3fe9f0c0831 100644 --- a/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs +++ b/src/SdkCommon/ClientRuntime/ClientRuntime/ServiceClient.cs @@ -250,6 +250,7 @@ private string FrameworkVersion protected ServiceClient() : this(CreateRootHandler()) { + _disposeRootHandler = true; } /// @@ -278,6 +279,7 @@ protected ServiceClient(HttpClient httpClient, bool disposeHttpClient = true) protected ServiceClient(params DelegatingHandler[] handlers) : this(CreateRootHandler(), handlers) { + _disposeRootHandler = true; } ///