From a776098c0f87446bf78f733fe44d074fefe91121 Mon Sep 17 00:00:00 2001 From: Adam Milazzo Date: Thu, 16 Apr 2020 09:11:39 -0700 Subject: [PATCH 1/4] Remove Microsoft.AspNetCore.WebUtilities dependency --- src/KubernetesClient/Kubernetes.Watch.cs | 32 +++++++++---------- src/KubernetesClient/Kubernetes.WebSocket.cs | 24 ++++++-------- src/KubernetesClient/KubernetesClient.csproj | 3 +- src/KubernetesClient/Utilities.cs | 17 ++++++++++ .../WatcherDelegatingHandler.cs | 9 +++--- 5 files changed, 47 insertions(+), 38 deletions(-) create mode 100644 src/KubernetesClient/Utilities.cs diff --git a/src/KubernetesClient/Kubernetes.Watch.cs b/src/KubernetesClient/Kubernetes.Watch.cs index ca992cabe..3d36b3555 100644 --- a/src/KubernetesClient/Kubernetes.Watch.cs +++ b/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 @@ public partial class Kubernetes 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.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()); diff --git a/src/KubernetesClient/Kubernetes.WebSocket.cs b/src/KubernetesClient/Kubernetes.WebSocket.cs index 5da20c8c8..75dd37544 100644 --- a/src/KubernetesClient/Kubernetes.WebSocket.cs +++ b/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,7 @@ using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Tasks; +using System.Text; namespace k8s { @@ -102,27 +102,23 @@ public partial class Kubernetes 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 - { - {"stderr", stderr ? "1" : "0"}, - {"stdin", stdin ? "1" : "0"}, - {"stdout", stdout ? "1" : "0"}, - {"tty", tty ? "1" : "0"} - }).TrimStart('?'); - - uriBuilder.Query = query; + 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'); + query.Append("&stdout=").Append(stdout ? '1' : '0'); + query.Append("&tty=").Append(tty ? '1' : '0'); + uriBuilder.Query = query.ToString(1, query.Length-1); // UriBuilder.Query doesn't like leading '?' chars, so trim it return this.StreamConnectAsync(uriBuilder.Uri, _invocationId, webSocketSubProtol, customHeaders, cancellationToken); } diff --git a/src/KubernetesClient/KubernetesClient.csproj b/src/KubernetesClient/KubernetesClient.csproj index bc9cdd058..0d403ff66 100644 --- a/src/KubernetesClient/KubernetesClient.csproj +++ b/src/KubernetesClient/KubernetesClient.csproj @@ -32,12 +32,11 @@ - - + diff --git a/src/KubernetesClient/Utilities.cs b/src/KubernetesClient/Utilities.cs new file mode 100644 index 000000000..57e6d9148 --- /dev/null +++ b/src/KubernetesClient/Utilities.cs @@ -0,0 +1,17 @@ +using System; +using System.Text; + +namespace k8s +{ + internal static class Utilities + { + /// Given a that is building a query string, adds a parameter to it. + 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)); + } + } +} diff --git a/src/KubernetesClient/WatcherDelegatingHandler.cs b/src/KubernetesClient/WatcherDelegatingHandler.cs index 9fd753e56..cd553ac80 100644 --- a/src/KubernetesClient/WatcherDelegatingHandler.cs +++ b/src/KubernetesClient/WatcherDelegatingHandler.cs @@ -6,7 +6,6 @@ using System.Net.Http; using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.WebUtilities; namespace k8s { @@ -21,11 +20,11 @@ protected override async Task SendAsync(HttpRequestMessage { 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); - - if (query.TryGetValue("watch", out var values) && values.Any(v => v == "true")) + string query = request.RequestUri.Query; + int index = query.IndexOf("watch=true"); + if (index > 0 && (query[index-1] == '&' || query[index-1] == '?')) { originResponse.Content = new LineSeparatedHttpContent(originResponse.Content, cancellationToken); } From e2662ed4049523bb8af6fcd119ea2b29150ac3d1 Mon Sep 17 00:00:00 2001 From: Adam Milazzo Date: Thu, 16 Apr 2020 09:29:48 -0700 Subject: [PATCH 2/4] Fix more query-string handling --- src/KubernetesClient/Kubernetes.Watch.cs | 2 +- src/KubernetesClient/Kubernetes.WebSocket.cs | 25 +++++++++---------- .../Kubernetes.WebSockets.Tests.cs | 4 +-- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/KubernetesClient/Kubernetes.Watch.cs b/src/KubernetesClient/Kubernetes.Watch.cs index 3d36b3555..7cf801a0e 100644 --- a/src/KubernetesClient/Kubernetes.Watch.cs +++ b/src/KubernetesClient/Kubernetes.Watch.cs @@ -87,7 +87,7 @@ public partial class Kubernetes Utilities.AddQueryParameter(query, "resourceVersion", resourceVersion); } - uriBuilder.Query = query.ToString(1, query.Length-1); // UriBuilder.Query doesn't like leading '?' chars, so trim it + 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()); diff --git a/src/KubernetesClient/Kubernetes.WebSocket.cs b/src/KubernetesClient/Kubernetes.WebSocket.cs index 75dd37544..e3adb9de3 100644 --- a/src/KubernetesClient/Kubernetes.WebSocket.cs +++ b/src/KubernetesClient/Kubernetes.WebSocket.cs @@ -14,6 +14,7 @@ using System.Threading; using System.Threading.Tasks; using System.Text; +using System.Globalization; namespace k8s { @@ -167,14 +168,13 @@ public partial class Kubernetes 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); } @@ -222,14 +222,13 @@ public partial class Kubernetes uriBuilder.Path += $"api/v1/namespaces/{@namespace}/pods/{name}/attach"; - uriBuilder.Query = QueryHelpers.AddQueryString(string.Empty, new Dictionary - { - { "container", container}, - { "stderr", stderr ? "1": "0"}, - { "stdin", stdin ? "1": "0"}, - { "stdout", stdout ? "1": "0"}, - { "tty", tty ? "1": "0"} - }).TrimStart('?'); + var query = new StringBuilder(); + query.Append("?stderr=").Append(stderr ? '1' : '0'); + query.Append("&stdin=").Append(stdin ? '1' : '0'); + query.Append("&stdout=").Append(stdout ? '1' : '0'); + query.Append("&tty=").Append(tty ? '1' : '0'); + Utilities.AddQueryParameter(query, "container", container); + uriBuilder.Query = query.ToString(1, query.Length-1); // UriBuilder.Query doesn't like leading '?' chars, so trim it return StreamConnectAsync(uriBuilder.Uri, _invocationId, webSocketSubProtol, customHeaders, cancellationToken); } diff --git a/tests/KubernetesClient.Tests/Kubernetes.WebSockets.Tests.cs b/tests/KubernetesClient.Tests/Kubernetes.WebSockets.Tests.cs index 961ed1612..1c4bb8de0 100644 --- a/tests/KubernetesClient.Tests/Kubernetes.WebSockets.Tests.cs +++ b/tests/KubernetesClient.Tests/Kubernetes.WebSockets.Tests.cs @@ -58,7 +58,7 @@ public async Task WebSocketNamespacedPodExecAsync() }; 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 @@ public async Task WebSocketNamespacedPodAttachAsync() }; 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 } From 8d29761dc31fe1a7f9df8c216ab2a0ced1f779ad Mon Sep 17 00:00:00 2001 From: Adam Milazzo Date: Thu, 16 Apr 2020 14:09:33 -0700 Subject: [PATCH 3/4] Add unit test --- src/KubernetesClient/AssemblyInfo.cs | 3 +++ src/KubernetesClient/Utilities.cs | 6 ++--- .../KubernetesClient.Tests.csproj | 2 ++ tests/KubernetesClient.Tests/UtilityTests.cs | 24 +++++++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 src/KubernetesClient/AssemblyInfo.cs create mode 100644 tests/KubernetesClient.Tests/UtilityTests.cs diff --git a/src/KubernetesClient/AssemblyInfo.cs b/src/KubernetesClient/AssemblyInfo.cs new file mode 100644 index 000000000..f2764d2b3 --- /dev/null +++ b/src/KubernetesClient/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("KubernetesClient.Tests, PublicKey=00240000048000009400000006020000002400005253413100040000010001004917ad4e106c573cc5dbb3b7456de8b6c07128ae43de292752b339eb423de60f0db6a6c0cb21e6640fc672cc84df4a772db85df1505e5dd08c98d5d115eed7a7b59c67fe1f4b32fa716b7177743a417b3fcf88606861650a81f565ac6614abbf8b6b7710436edb497a83974165f9fe6995b70af13047a110bf63cdbfa45f89ac")] diff --git a/src/KubernetesClient/Utilities.cs b/src/KubernetesClient/Utilities.cs index 57e6d9148..1975ac381 100644 --- a/src/KubernetesClient/Utilities.cs +++ b/src/KubernetesClient/Utilities.cs @@ -8,10 +8,10 @@ internal static class Utilities /// Given a that is building a query string, adds a parameter to it. 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)); + 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)); + if (!string.IsNullOrEmpty(value)) sb.Append(Uri.EscapeDataString(value)); } } } diff --git a/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj b/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj index 452aa0be2..bf8a7f613 100755 --- a/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj +++ b/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj @@ -5,6 +5,8 @@ true ..\..\src\KubernetesClient\kubernetes-client.snk k8s.Tests + true + ../../src/KubernetesClient/kubernetes-client.snk netcoreapp2.1;netcoreapp2.0 diff --git a/tests/KubernetesClient.Tests/UtilityTests.cs b/tests/KubernetesClient.Tests/UtilityTests.cs new file mode 100644 index 000000000..02de7f998 --- /dev/null +++ b/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(() => Utilities.AddQueryParameter(null, "key", "value")); + Assert.Throws(() => Utilities.AddQueryParameter(sb, null, "value")); + Assert.Throws(() => 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()); + } + } +} From 4a440f2afe757f0190e6ae8d6be4826bc5cc2813 Mon Sep 17 00:00:00 2001 From: Adam Milazzo Date: Sat, 18 Apr 2020 00:51:01 -0700 Subject: [PATCH 4/4] Merge with master --- src/KubernetesClient/AssemblyInfo.cs | 3 --- tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj | 2 -- 2 files changed, 5 deletions(-) delete mode 100644 src/KubernetesClient/AssemblyInfo.cs diff --git a/src/KubernetesClient/AssemblyInfo.cs b/src/KubernetesClient/AssemblyInfo.cs deleted file mode 100644 index f2764d2b3..000000000 --- a/src/KubernetesClient/AssemblyInfo.cs +++ /dev/null @@ -1,3 +0,0 @@ -using System.Runtime.CompilerServices; - -[assembly: InternalsVisibleTo("KubernetesClient.Tests, PublicKey=00240000048000009400000006020000002400005253413100040000010001004917ad4e106c573cc5dbb3b7456de8b6c07128ae43de292752b339eb423de60f0db6a6c0cb21e6640fc672cc84df4a772db85df1505e5dd08c98d5d115eed7a7b59c67fe1f4b32fa716b7177743a417b3fcf88606861650a81f565ac6614abbf8b6b7710436edb497a83974165f9fe6995b70af13047a110bf63cdbfa45f89ac")] diff --git a/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj b/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj index bf8a7f613..452aa0be2 100755 --- a/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj +++ b/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj @@ -5,8 +5,6 @@ true ..\..\src\KubernetesClient\kubernetes-client.snk k8s.Tests - true - ../../src/KubernetesClient/kubernetes-client.snk netcoreapp2.1;netcoreapp2.0