From 278908cf64277c5d70d53a5001e2a172f2b00080 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 20 Jul 2023 14:50:41 -0400 Subject: [PATCH] Add HttpMethod.Parse --- .../System.Net.Http/ref/System.Net.Http.cs | 1 + .../src/System/Net/Http/HttpMethod.cs | 30 +++++++++-- .../HttpClientHandlerTest.Connect.cs | 4 +- .../tests/FunctionalTests/HttpMethodTest.cs | 50 +++++++++++++++++++ .../tests/FunctionalTests/MetricsTest.cs | 2 +- .../src/System/Net/HttpWebRequest.cs | 2 +- 6 files changed, 80 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Net.Http/ref/System.Net.Http.cs b/src/libraries/System.Net.Http/ref/System.Net.Http.cs index a31f5e69ad8cf0..f4794b5eee5d68 100644 --- a/src/libraries/System.Net.Http/ref/System.Net.Http.cs +++ b/src/libraries/System.Net.Http/ref/System.Net.Http.cs @@ -244,6 +244,7 @@ public HttpMethod(string method) { } public override int GetHashCode() { throw null; } public static bool operator ==(System.Net.Http.HttpMethod? left, System.Net.Http.HttpMethod? right) { throw null; } public static bool operator !=(System.Net.Http.HttpMethod? left, System.Net.Http.HttpMethod? right) { throw null; } + public static System.Net.Http.HttpMethod Parse(ReadOnlySpan method) { throw null; } public override string ToString() { throw null; } } public sealed class HttpProtocolException : System.Net.Http.HttpIOException diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs index 045d85fb97cd8c..1634453a9906de 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs @@ -146,6 +146,19 @@ public override string ToString() return !(left == right); } + /// Parses the provided into an instance. + /// The method to parse. + /// An instance for the provided . + /// + /// This method may return a singleton instance for known methods; for example, it may return + /// if "GET" is specified. The parsing is performed in a case-insensitive manner, so it may also return + /// if "get" is specified. For unknown methods, a new instance is returned, with the + /// same validation being performed as by the constructor. + /// + public static HttpMethod Parse(ReadOnlySpan method) => + GetKnownMethod(method) ?? + new HttpMethod(method.ToString()); + /// /// Returns a singleton method instance with a capitalized method name for the supplied method /// if it's known; otherwise, returns the original. @@ -158,17 +171,23 @@ internal static HttpMethod Normalize(HttpMethod method) // _http3Index is only set for the singleton instances, so if it's not null, // we can avoid the lookup. Otherwise, look up the method instance and return the // normalized instance if it's found. + return method._http3Index is null && GetKnownMethod(method._method) is HttpMethod match ? + match : + method; + } - if (method._http3Index is null && method._method.Length >= 3) // 3 == smallest known method + private static HttpMethod? GetKnownMethod(ReadOnlySpan method) + { + if (method.Length >= 3) // 3 == smallest known method { - HttpMethod? match = (method._method[0] | 0x20) switch + HttpMethod? match = (method[0] | 0x20) switch { 'c' => s_connectMethod, 'd' => s_deleteMethod, 'g' => s_getMethod, 'h' => s_headMethod, 'o' => s_optionsMethod, - 'p' => method._method.Length switch + 'p' => method.Length switch { 3 => s_putMethod, 4 => s_postMethod, @@ -178,13 +197,14 @@ internal static HttpMethod Normalize(HttpMethod method) _ => null, }; - if (match is not null && string.Equals(method._method, match._method, StringComparison.OrdinalIgnoreCase)) + if (match is not null && + method.Equals(match._method, StringComparison.OrdinalIgnoreCase)) { return match; } } - return method; + return null; } internal bool MustHaveRequestBody diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Connect.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Connect.cs index 16587b3de98b78..3f1f4b3102379c 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Connect.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Connect.cs @@ -21,7 +21,7 @@ await LoopbackServer.CreateServerAsync(async (server, url) => { using (HttpClient client = CreateHttpClient()) { - HttpRequestMessage request = new HttpRequestMessage(new HttpMethod("CONNECT"), url) { Version = UseVersion }; + HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Connect, url) { Version = UseVersion }; request.Headers.Host = "foo.com:345"; // We need to use ResponseHeadersRead here, otherwise we will hang trying to buffer the response body. @@ -80,7 +80,7 @@ await LoopbackServer.CreateServerAsync(async (server, url) => { using (HttpClient client = CreateHttpClient()) { - HttpRequestMessage request = new HttpRequestMessage(new HttpMethod("CONNECT"), url) { Version = UseVersion }; + HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Connect, url) { Version = UseVersion }; request.Headers.Host = "foo.com:345"; // We need to use ResponseHeadersRead here, otherwise we will hang trying to buffer the response body. Task responseTask = client.SendAsync(TestAsync, request, HttpCompletionOption.ResponseHeadersRead); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpMethodTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpMethodTest.cs index 2a804decd9fb2d..9911fa10bc0222 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpMethodTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpMethodTest.cs @@ -150,6 +150,56 @@ public void Patch_VerifyValue_PropertyNameMatchesHttpMethodName() Assert.Equal("PATCH", HttpMethod.Patch.Method); } + public static IEnumerable Parse_UsesKnownInstances_MemberData() + { + yield return new object[] { HttpMethod.Connect, nameof(HttpMethod.Connect) }; + yield return new object[] { HttpMethod.Delete, nameof(HttpMethod.Delete) }; + yield return new object[] { HttpMethod.Get, nameof(HttpMethod.Get) }; + yield return new object[] { HttpMethod.Head, nameof(HttpMethod.Head) }; + yield return new object[] { HttpMethod.Options, nameof(HttpMethod.Options) }; + yield return new object[] { HttpMethod.Patch, nameof(HttpMethod.Patch) }; + yield return new object[] { HttpMethod.Post, nameof(HttpMethod.Post) }; + yield return new object[] { HttpMethod.Put, nameof(HttpMethod.Put) }; + yield return new object[] { HttpMethod.Trace, nameof(HttpMethod.Trace) }; + } + + [Theory] + [MemberData(nameof(Parse_UsesKnownInstances_MemberData))] + public void Parse_KnownMethod_UsesKnownInstances(HttpMethod method, string methodName) + { + Assert.Same(method, HttpMethod.Parse(methodName)); + Assert.Same(method, HttpMethod.Parse(methodName.ToUpperInvariant())); + Assert.Same(method, HttpMethod.Parse(methodName.ToLowerInvariant())); + } + + [Theory] + [InlineData("Unknown")] + [InlineData("custom")] + public void Parse_UnknownMethod_UsesNewInstances(string method) + { + var h = HttpMethod.Parse(method); + Assert.NotNull(h); + Assert.NotSame(h, HttpMethod.Parse(method)); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void Parse_Whitespace_ThrowsArgumentException(string method) + { + AssertExtensions.Throws("method", () => HttpMethod.Parse(method)); + } + + [Theory] + [InlineData(" GET ")] + [InlineData(" Post")] + [InlineData("Put ")] + [InlineData("multiple things")] + public void Parse_InvalidToken_Throws(string method) + { + Assert.Throws(() => HttpMethod.Parse(method)); + } + private static void AddStaticHttpMethods(List staticHttpMethods) { staticHttpMethods.Add(new object[] { HttpMethod.Patch }); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index d57eec55d754f2..bb9441ae7c5155 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -263,7 +263,7 @@ public Task RequestDuration_Success_Recorded(string method, HttpStatusCode statu { using HttpMessageInvoker client = CreateHttpMessageInvoker(); using InstrumentRecorder recorder = SetupInstrumentRecorder(InstrumentNames.RequestDuration); - using HttpRequestMessage request = new(new HttpMethod(method), uri) { Version = UseVersion }; + using HttpRequestMessage request = new(HttpMethod.Parse(method), uri) { Version = UseVersion }; using HttpResponseMessage response = await SendAsync(client, request); diff --git a/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs b/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs index f99a052ff0ace9..5f39f3345132b5 100644 --- a/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs +++ b/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs @@ -1127,7 +1127,7 @@ private async Task SendRequest(bool async) throw new InvalidOperationException(SR.net_reqsubmitted); } - var request = new HttpRequestMessage(new HttpMethod(_originVerb), _requestUri); + var request = new HttpRequestMessage(HttpMethod.Parse(_originVerb), _requestUri); bool disposeRequired = false; HttpClient? client = null;