Remove Microsoft.AspNetCore.WebUtilities dependency (#419)

* Remove Microsoft.AspNetCore.WebUtilities dependency

* Fix more query-string handling

* Add unit test

* Merge with master

src/KubernetesClient/Kubernetes.Watch.cs

@@ -1,10 +1,10 @@
-using Microsoft.AspNetCore.WebUtilities;
 using Microsoft.Rest;
 using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Net;
 using System.Net.Http;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -43,53 +43,51 @@ namespace k8s
 
             uriBuilder.Path += path;
 
-            var query = string.Empty;
+            var query = new StringBuilder();
-
             // Don't sent watch, because setting that value will cause the WatcherDelegatingHandler to kick in. That class
             // "eats" the first line, which is something we don't want.
             // query = QueryHelpers.AddQueryString(query, "watch", "true");
-
+            if(@continue != null)
-            if (@continue != null)
             {
-                query = QueryHelpers.AddQueryString(query, "continue", Uri.EscapeDataString(@continue));
+                Utilities.AddQueryParameter(query, "continue", @continue);
             }
 
-            if (fieldSelector != null)
+            if (!string.IsNullOrEmpty(fieldSelector))
             {
-                query = QueryHelpers.AddQueryString(query, "fieldSelector", Uri.EscapeDataString(fieldSelector));
+                Utilities.AddQueryParameter(query, "fieldSelector", fieldSelector);
             }
 
             if (includeUninitialized != null)
             {
-                query = QueryHelpers.AddQueryString(query, "includeUninitialized", includeUninitialized.Value ? "true" : "false");
+                Utilities.AddQueryParameter(query, "includeUninitialized", includeUninitialized.Value ? "true" : "false");
             }
 
-            if (labelSelector != null)
+            if (!string.IsNullOrEmpty(labelSelector))
             {
-                query = QueryHelpers.AddQueryString(query, "labelSelector", Uri.EscapeDataString(labelSelector));
+                Utilities.AddQueryParameter(query, "labelSelector", labelSelector);
             }
 
             if (limit != null)
             {
-                query = QueryHelpers.AddQueryString(query, "limit", limit.Value.ToString());
+                Utilities.AddQueryParameter(query, "limit", limit.Value.ToString());
             }
 
             if (pretty != null)
             {
-                query = QueryHelpers.AddQueryString(query, "pretty", pretty.Value ? "true" : "false");
+                Utilities.AddQueryParameter(query, "pretty", pretty.Value ? "true" : "false");
             }
 
             if (timeoutSeconds != null)
             {
-                query = QueryHelpers.AddQueryString(query, "timeoutSeconds", timeoutSeconds.Value.ToString());
+                Utilities.AddQueryParameter(query, "timeoutSeconds", timeoutSeconds.Value.ToString());
             }
 
-            if (resourceVersion != null)
+            if (!string.IsNullOrEmpty(resourceVersion))
             {
-                query = QueryHelpers.AddQueryString(query, "resourceVersion", resourceVersion);
+                Utilities.AddQueryParameter(query, "resourceVersion", resourceVersion);
             }
 
-            uriBuilder.Query = query;
+            uriBuilder.Query = query.Length == 0 ? "" : query.ToString(1, query.Length-1); // UriBuilder.Query doesn't like leading '?' chars, so trim it
 
             // Create HTTP transport objects
             var httpRequest = new HttpRequestMessage(HttpMethod.Get, uriBuilder.ToString());

src/KubernetesClient/Kubernetes.WebSocket.cs

@@ -1,5 +1,4 @@
 using k8s.Models;
-using Microsoft.AspNetCore.WebUtilities;
 using Microsoft.Rest;
 using Microsoft.Rest.Serialization;
 using System;
@@ -14,6 +13,8 @@ using System.Net.Security;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading;
 using System.Threading.Tasks;
+using System.Text;
+using System.Globalization;
 
 namespace k8s
 {
@@ -102,27 +103,23 @@ namespace k8s
 
             uriBuilder.Path += $"api/v1/namespaces/{@namespace}/pods/{name}/exec";
 
-            var query = string.Empty;
+            var query = new StringBuilder();
 
             foreach (var c in command)
             {
-                query = QueryHelpers.AddQueryString(query, "command", c);
+                Utilities.AddQueryParameter(query, "command", c);
             }
 
-            if (container != null)
+            if (!string.IsNullOrEmpty(container))
             {
-                query = QueryHelpers.AddQueryString(query, "container", Uri.EscapeDataString(container));
+                Utilities.AddQueryParameter(query, "container", container);
             }
 
-            query = QueryHelpers.AddQueryString(query, new Dictionary<string, string>
+            query.Append("&stderr=").Append(stderr ? '1' : '0'); // the query string is guaranteed not to be empty here because it has a 'command' param
-            {
+            query.Append("&stdin=").Append(stdin ? '1' : '0');
-                {"stderr", stderr ? "1" : "0"},
+            query.Append("&stdout=").Append(stdout ? '1' : '0');
-                {"stdin", stdin ? "1" : "0"},
+            query.Append("&tty=").Append(tty ? '1' : '0');
-                {"stdout", stdout ? "1" : "0"},
+            uriBuilder.Query = query.ToString(1, query.Length-1); // UriBuilder.Query doesn't like leading '?' chars, so trim it
-                {"tty", tty ? "1" : "0"}
-            }).TrimStart('?');
-
-            uriBuilder.Query = query;
 
             return this.StreamConnectAsync(uriBuilder.Uri, _invocationId, webSocketSubProtol, customHeaders, cancellationToken);
         }
@@ -171,14 +168,13 @@ namespace k8s
 
             uriBuilder.Path += $"api/v1/namespaces/{@namespace}/pods/{name}/portforward";
 
-            var q = "";
+            var q = new StringBuilder();
             foreach (var port in ports)
             {
-                q = QueryHelpers.AddQueryString(q, "ports", $"{port}");
+                if (q.Length != 0) q.Append('&');
+                q.Append("ports=").Append(port.ToString(CultureInfo.InvariantCulture));
             }
-            uriBuilder.Query = q.TrimStart('?');
+            uriBuilder.Query = q.ToString();
-
-
 
             return StreamConnectAsync(uriBuilder.Uri, _invocationId, webSocketSubProtocol, customHeaders, cancellationToken);
         }
@@ -226,14 +222,13 @@ namespace k8s
 
             uriBuilder.Path += $"api/v1/namespaces/{@namespace}/pods/{name}/attach";
 
-            uriBuilder.Query = QueryHelpers.AddQueryString(string.Empty, new Dictionary<string, string>
+            var query = new StringBuilder();
-            {
+            query.Append("?stderr=").Append(stderr ? '1' : '0');
-                { "container", container},
+            query.Append("&stdin=").Append(stdin ? '1' : '0');
-                { "stderr", stderr ? "1": "0"},
+            query.Append("&stdout=").Append(stdout ? '1' : '0');
-                { "stdin", stdin ? "1": "0"},
+            query.Append("&tty=").Append(tty ? '1' : '0');
-                { "stdout", stdout ? "1": "0"},
+            Utilities.AddQueryParameter(query, "container", container);
-                { "tty", tty ? "1": "0"}
+            uriBuilder.Query = query.ToString(1, query.Length-1); // UriBuilder.Query doesn't like leading '?' chars, so trim it
-            }).TrimStart('?');
 
             return StreamConnectAsync(uriBuilder.Uri, _invocationId, webSocketSubProtol, customHeaders, cancellationToken);
         }

src/KubernetesClient/KubernetesClient.csproj

@@ -32,12 +32,11 @@
     <PackageReference Include="Microsoft.AspNetCore.JsonPatch" Version="3.0.0" Condition="'$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'netcoreapp2.1'" />
     <PackageReference Include="Nerdbank.GitVersioning" Version="3.0.50" PrivateAssets="all" />
     <PackageReference Include="Portable.BouncyCastle" Version="1.8.1.3" />
-    <PackageReference Include="Microsoft.AspNetCore.WebUtilities" Version="1.1.2" Condition="'$(TargetFramework)' != 'netstandard2.0'" />
-    <PackageReference Include="Microsoft.AspNetCore.WebUtilities" Version="2.2.0" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
     <PackageReference Include="Microsoft.Rest.ClientRuntime" Version="2.3.10" />
     <PackageReference Include="Newtonsoft.Json" Version="9.0.1" Condition="'$(TargetFramework)' != 'netstandard2.0' and '$(TargetFramework)' != 'netcoreapp2.1'" />
     <PackageReference Include="Newtonsoft.Json" Version="12.0.2" Condition="'$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'netcoreapp2.1'" />
     <PackageReference Include="YamlDotNet" Version="6.0.0" />
+    <PackageReference Include="System.Buffers" Version="4.5.1" Condition="'$(TargetFramework)' != 'netcoreapp2.1'" />
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetFramework)' == 'net452'">

src/KubernetesClient/Utilities.cs

@@ -0,0 +1,17 @@
+using System;
+using System.Text;
+
+namespace k8s
+{
+    internal static class Utilities
+    {
+        /// <summary>Given a <see cref="StringBuilder"/> that is building a query string, adds a parameter to it.</summary>
+        public static void AddQueryParameter(StringBuilder sb, string key, string value)
+        {
+            if (sb == null) throw new ArgumentNullException(nameof(sb));
+            if (string.IsNullOrEmpty(key)) throw new ArgumentNullException(nameof(key));
+            sb.Append(sb.Length != 0 ? '&' : '?').Append(Uri.EscapeDataString(key)).Append('=');
+            if (!string.IsNullOrEmpty(value)) sb.Append(Uri.EscapeDataString(value));
+        }
+    }
+}

src/KubernetesClient/WatcherDelegatingHandler.cs

@@ -6,7 +6,6 @@ using System.Net;
 using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.WebUtilities;
 
 namespace k8s
 {
@@ -21,11 +20,11 @@ namespace k8s
         {
             var originResponse = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
 
-            if (originResponse.IsSuccessStatusCode)
+            if (originResponse.IsSuccessStatusCode && request.Method == HttpMethod.Get) // all watches are GETs, so we can ignore others
             {
-                var query = QueryHelpers.ParseQuery(request.RequestUri.Query);
+                string query = request.RequestUri.Query;
-
+                int index = query.IndexOf("watch=true");
-                if (query.TryGetValue("watch", out var values) && values.Any(v => v == "true"))
+                if (index > 0 && (query[index-1] == '&' || query[index-1] == '?'))
                 {
                     originResponse.Content = new LineSeparatedHttpContent(originResponse.Content, cancellationToken);
                 }

tests/KubernetesClient.Tests/Kubernetes.WebSockets.Tests.cs

@@ -58,7 +58,7 @@ namespace k8s.Tests
             };
 
             Assert.Equal(mockWebSocketBuilder.PublicWebSocket, webSocket); // Did the method return the correct web socket?
-            Assert.Equal(new Uri("ws://localhost/api/v1/namespaces/mynamespace/pods/mypod/exec?command=%2Fbin%2Fbash&command=-c&command=echo%20Hello,%20World%0Aexit%200%0A&container=mycontainer&stderr=1&stdin=1&stdout=1&tty=1"), mockWebSocketBuilder.Uri); // Did we connect to the correct URL?
+            Assert.Equal(new Uri("ws://localhost/api/v1/namespaces/mynamespace/pods/mypod/exec?command=%2Fbin%2Fbash&command=-c&command=echo%20Hello%2C%20World%0Aexit%200%0A&container=mycontainer&stderr=1&stdin=1&stdout=1&tty=1"), mockWebSocketBuilder.Uri); // Did we connect to the correct URL?
             Assert.Empty(mockWebSocketBuilder.Certificates); // No certificates were used in this test
             Assert.Equal(expectedHeaders, mockWebSocketBuilder.RequestHeaders); // Did we use the expected headers
         }
@@ -136,7 +136,7 @@ namespace k8s.Tests
             };
 
             Assert.Equal(mockWebSocketBuilder.PublicWebSocket, webSocket); // Did the method return the correct web socket?
-            Assert.Equal(new Uri("ws://localhost:80/api/v1/namespaces/mynamespace/pods/mypod/attach?container=my-container&stderr=1&stdin=1&stdout=1&tty=1"), mockWebSocketBuilder.Uri); // Did we connect to the correct URL?
+            Assert.Equal(new Uri("ws://localhost:80/api/v1/namespaces/mynamespace/pods/mypod/attach?stderr=1&stdin=1&stdout=1&tty=1&container=my-container"), mockWebSocketBuilder.Uri); // Did we connect to the correct URL?
             Assert.Empty(mockWebSocketBuilder.Certificates); // No certificates were used in this test
             Assert.Equal(expectedHeaders, mockWebSocketBuilder.RequestHeaders); // Did we use the expected headers
         }

tests/KubernetesClient.Tests/UtilityTests.cs

@@ -0,0 +1,24 @@
+using System;
+using System.Text;
+using Xunit;
+
+namespace k8s.Tests
+{
+    public class UtilityTests
+    {
+        [Fact]
+        public void TestQueryStringUtilities()
+        {
+            var sb = new StringBuilder();
+            Assert.Throws<ArgumentNullException>(() => Utilities.AddQueryParameter(null, "key", "value"));
+            Assert.Throws<ArgumentNullException>(() => Utilities.AddQueryParameter(sb, null, "value"));
+            Assert.Throws<ArgumentNullException>(() => Utilities.AddQueryParameter(sb, "", "value"));
+
+            Utilities.AddQueryParameter(sb, "key", "value");
+            Utilities.AddQueryParameter(sb, "key", "a=b");
+            Utilities.AddQueryParameter(sb, "+key", null);
+            Utilities.AddQueryParameter(sb, "ekey", "");
+            Assert.Equal("?key=value&key=a%3Db&%2Bkey=&ekey=", sb.ToString());
+        }
+    }
+}