From 73c3f7f4a1b0da207bf51a3984c2481a9894746e Mon Sep 17 00:00:00 2001 From: Timothy Byrd Date: Wed, 3 Mar 2021 15:48:38 -0800 Subject: [PATCH 1/7] make PUT redirects act like POST redirects for 30x reponses to conform to net Framework behaviour - issue 28998 --- .../tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs | 1 + .../src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs index ee41dbb716adfd..f97c99f31b8bc6 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs @@ -45,6 +45,7 @@ public static IEnumerable RedirectStatusCodesOldMethodsNewMethods() { yield return new object[] { statusCode, "GET", "GET" }; yield return new object[] { statusCode, "POST", statusCode <= 303 ? "GET" : "POST" }; + yield return new object[] { statusCode, "PUT", statusCode <= 303 ? "GET" : "PUT" }; yield return new object[] { statusCode, "HEAD", "HEAD" }; } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs index cf1693672b03e4..de17886f59576d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs @@ -144,7 +144,7 @@ private static bool RequestRequiresForceGet(HttpStatusCode statusCode, HttpMetho case HttpStatusCode.Found: case HttpStatusCode.SeeOther: case HttpStatusCode.MultipleChoices: - return requestMethod == HttpMethod.Post; + return requestMethod == HttpMethod.Post || requestMethod == HttpMethod.Put; default: return false; } From 8dafbc88e3b89155042c1821c6e78c1a775b5a1f Mon Sep 17 00:00:00 2001 From: Timothy Byrd Date: Wed, 3 Mar 2021 15:48:38 -0800 Subject: [PATCH 2/7] Make PUT redirects do GET like Net Framework In Net Framework, PUT redirects do a GET. Net 5.0 breaks compatibiltiy with this. See https://github.com/dotnet/runtime/issues/28998 This commit causes PUT redirects to act like POST redirects for 30x responses to conform to Net Framework behaviour. Fix #28998 --- .../tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs | 1 + .../src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs index ee41dbb716adfd..f97c99f31b8bc6 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs @@ -45,6 +45,7 @@ public static IEnumerable RedirectStatusCodesOldMethodsNewMethods() { yield return new object[] { statusCode, "GET", "GET" }; yield return new object[] { statusCode, "POST", statusCode <= 303 ? "GET" : "POST" }; + yield return new object[] { statusCode, "PUT", statusCode <= 303 ? "GET" : "PUT" }; yield return new object[] { statusCode, "HEAD", "HEAD" }; } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs index cf1693672b03e4..de17886f59576d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs @@ -144,7 +144,7 @@ private static bool RequestRequiresForceGet(HttpStatusCode statusCode, HttpMetho case HttpStatusCode.Found: case HttpStatusCode.SeeOther: case HttpStatusCode.MultipleChoices: - return requestMethod == HttpMethod.Post; + return requestMethod == HttpMethod.Post || requestMethod == HttpMethod.Put; default: return false; } From 66f59c46378acdef26d33ea01583d66a9d8a974e Mon Sep 17 00:00:00 2001 From: Timothy Byrd Date: Thu, 4 Mar 2021 15:34:09 -0800 Subject: [PATCH 3/7] Make 303 redirects do GET like Net Framework In Net Framework, PUT redirects on a 303 do a GET. Net 5.0 breaks compatibility with this. See https://github.com/dotnet/runtime/issues/28998 This commit causes redirects of a 303 to do a GET for POST (existing behaviour), PUT, DELETE, PATCH and OPTIONS Fix #28998 --- .../System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs | 5 ++++- .../System/Net/Http/SocketsHttpHandler/RedirectHandler.cs | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs index f97c99f31b8bc6..bac736f0d63c19 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs @@ -45,7 +45,10 @@ public static IEnumerable RedirectStatusCodesOldMethodsNewMethods() { yield return new object[] { statusCode, "GET", "GET" }; yield return new object[] { statusCode, "POST", statusCode <= 303 ? "GET" : "POST" }; - yield return new object[] { statusCode, "PUT", statusCode <= 303 ? "GET" : "PUT" }; + yield return new object[] { statusCode, "PUT", statusCode == 303 ? "GET" : "PUT" }; + yield return new object[] { statusCode, "DELETE", statusCode == 303 ? "GET" : "DELETE" }; + yield return new object[] { statusCode, "PATCH", statusCode == 303 ? "GET" : "PATCH" }; + yield return new object[] { statusCode, "OPTIONS", statusCode == 303 ? "GET" : "OPTIONS" }; yield return new object[] { statusCode, "HEAD", "HEAD" }; } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs index de17886f59576d..ecf79e0e188b51 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs @@ -142,9 +142,12 @@ private static bool RequestRequiresForceGet(HttpStatusCode statusCode, HttpMetho { case HttpStatusCode.Moved: case HttpStatusCode.Found: - case HttpStatusCode.SeeOther: case HttpStatusCode.MultipleChoices: - return requestMethod == HttpMethod.Post || requestMethod == HttpMethod.Put; + return requestMethod == HttpMethod.Post; + case HttpStatusCode.SeeOther: + return requestMethod == HttpMethod.Post || requestMethod == HttpMethod.Put + || requestMethod == HttpMethod.Delete || requestMethod == HttpMethod.Patch + || requestMethod == HttpMethod.Options; default: return false; } From 98b4652aa7ee1c17a6a7c79447090c12bed8ead0 Mon Sep 17 00:00:00 2001 From: Timothy Byrd Date: Sat, 6 Mar 2021 18:48:46 -0800 Subject: [PATCH 4/7] Make 303 redirects do GET like Net Framework In Net Framework, PUT redirects on a 303 do a GET. Net 5.0 breaks compatibility with this. See https://github.com/dotnet/runtime/issues/28998 This commit causes redirects of a 303 to do a GET for all methods except HEAD. Fix #28998 --- .../src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs index ecf79e0e188b51..c5d9e017a4c901 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs @@ -145,9 +145,7 @@ private static bool RequestRequiresForceGet(HttpStatusCode statusCode, HttpMetho case HttpStatusCode.MultipleChoices: return requestMethod == HttpMethod.Post; case HttpStatusCode.SeeOther: - return requestMethod == HttpMethod.Post || requestMethod == HttpMethod.Put - || requestMethod == HttpMethod.Delete || requestMethod == HttpMethod.Patch - || requestMethod == HttpMethod.Options; + return requestMethod != HttpMethod.Head; default: return false; } From ca834c0e975422049aaefebba8dbc29ab6e56c96 Mon Sep 17 00:00:00 2001 From: Timothy Byrd Date: Mon, 8 Mar 2021 20:22:35 -0800 Subject: [PATCH 5/7] Add test for MYCUSTOMMETHOD to check 303 redirect Fix #28998 --- .../tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs index bac736f0d63c19..096ed9e66f77a1 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs @@ -50,6 +50,7 @@ public static IEnumerable RedirectStatusCodesOldMethodsNewMethods() yield return new object[] { statusCode, "PATCH", statusCode == 303 ? "GET" : "PATCH" }; yield return new object[] { statusCode, "OPTIONS", statusCode == 303 ? "GET" : "OPTIONS" }; yield return new object[] { statusCode, "HEAD", "HEAD" }; + yield return new object[] { statusCode, "MYCUSTOMMETHOD", statusCode == 303 ? "GET" : "MYCUSTOMMETHOD" }; } } public HttpClientHandlerTest_AutoRedirect(ITestOutputHelper output) : base(output) { } From 3ec922c2364308ba9148485fc8969033254ffbe1 Mon Sep 17 00:00:00 2001 From: Timothy Byrd Date: Tue, 9 Mar 2021 04:48:31 -0800 Subject: [PATCH 6/7] Group test code by behavior Fix #28998 --- .../System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs index 096ed9e66f77a1..b7f911048dca84 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs @@ -44,12 +44,14 @@ public static IEnumerable RedirectStatusCodesOldMethodsNewMethods() foreach (int statusCode in new[] { 300, 301, 302, 303, 307, 308 }) { yield return new object[] { statusCode, "GET", "GET" }; + yield return new object[] { statusCode, "HEAD", "HEAD" }; + yield return new object[] { statusCode, "POST", statusCode <= 303 ? "GET" : "POST" }; - yield return new object[] { statusCode, "PUT", statusCode == 303 ? "GET" : "PUT" }; + yield return new object[] { statusCode, "DELETE", statusCode == 303 ? "GET" : "DELETE" }; - yield return new object[] { statusCode, "PATCH", statusCode == 303 ? "GET" : "PATCH" }; yield return new object[] { statusCode, "OPTIONS", statusCode == 303 ? "GET" : "OPTIONS" }; - yield return new object[] { statusCode, "HEAD", "HEAD" }; + yield return new object[] { statusCode, "PATCH", statusCode == 303 ? "GET" : "PATCH" }; + yield return new object[] { statusCode, "PUT", statusCode == 303 ? "GET" : "PUT" }; yield return new object[] { statusCode, "MYCUSTOMMETHOD", statusCode == 303 ? "GET" : "MYCUSTOMMETHOD" }; } } From 728a81fb887eb37e6aa13755e8cedb56ec6ba056 Mon Sep 17 00:00:00 2001 From: Timothy Byrd Date: Tue, 9 Mar 2021 11:27:11 -0800 Subject: [PATCH 7/7] Do not change GET to GET on 303 Fix #28998 --- .../src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs index c5d9e017a4c901..856b4e61da394f 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs @@ -145,7 +145,7 @@ private static bool RequestRequiresForceGet(HttpStatusCode statusCode, HttpMetho case HttpStatusCode.MultipleChoices: return requestMethod == HttpMethod.Post; case HttpStatusCode.SeeOther: - return requestMethod != HttpMethod.Head; + return requestMethod != HttpMethod.Get && requestMethod != HttpMethod.Head; default: return false; }