From 87d5fb72c1e90521e08451e88ef9531b955060e6 Mon Sep 17 00:00:00 2001 From: Mark Kraus Date: Wed, 2 May 2018 19:56:02 -0500 Subject: [PATCH 1/2] [Feature] Fix Web Cmdlets for .NET Core 2.1 --- .../Common/WebRequestPSCmdlet.Common.cs | 44 ++++++--------- .../WebCmdlets.Tests.ps1 | 54 ++++++++++++++----- 2 files changed, 58 insertions(+), 40 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index 435eafb79b8..a64ae3f096d 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -993,7 +993,7 @@ internal virtual HttpClient GetHttpClient(bool handleRedirect) return httpClient; } - internal virtual HttpRequestMessage GetRequest(Uri uri, bool stripAuthorization) + internal virtual HttpRequestMessage GetRequest(Uri uri) { Uri requestUri = PrepareUri(uri); HttpMethod httpMethod = null; @@ -1032,14 +1032,6 @@ internal virtual HttpRequestMessage GetRequest(Uri uri, bool stripAuthorization) } else { - if (stripAuthorization - && - String.Equals(entry.Key, HttpKnownHeaderNames.Authorization.ToString(), StringComparison.OrdinalIgnoreCase) - ) - { - continue; - } - if (SkipHeaderValidation) { request.Headers.TryAddWithoutValidation(entry.Key, entry.Value); @@ -1268,15 +1260,15 @@ static bool IsRedirectToGet(HttpStatusCode code) || code == HttpStatusCode.RedirectMethod || - code == HttpStatusCode.TemporaryRedirect + code == HttpStatusCode.SeeOther || - code == HttpStatusCode.RedirectKeepVerb + code == HttpStatusCode.Ambiguous || - code == HttpStatusCode.SeeOther + code == HttpStatusCode.MultipleChoices ); } - internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestMessage request, bool stripAuthorization) + internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestMessage request, bool keepAuthorization) { if (client == null) { throw new ArgumentNullException("client"); } if (request == null) { throw new ArgumentNullException("request"); } @@ -1287,7 +1279,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM _cancelToken = new CancellationTokenSource(); HttpResponseMessage response = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, _cancelToken.Token).GetAwaiter().GetResult(); - if (stripAuthorization && IsRedirectCode(response.StatusCode) && response.Headers.Location != null) + if (keepAuthorization && IsRedirectCode(response.StatusCode) && response.Headers.Location != null) { _cancelToken.Cancel(); _cancelToken = null; @@ -1306,14 +1298,12 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM Method = WebRequestMethod.Get; } - // recreate the HttpClient with redirection enabled since the first call suppressed redirection currentUri = new Uri(request.RequestUri, response.Headers.Location); - using (client = GetHttpClient(false)) - using (HttpRequestMessage redirectRequest = GetRequest(currentUri, stripAuthorization:true)) + // Continue to handle redirection + using (client = GetHttpClient(true)) + using (HttpRequestMessage redirectRequest = GetRequest(currentUri)) { - FillRequestStream(redirectRequest); - _cancelToken = new CancellationTokenSource(); - response = client.SendAsync(redirectRequest, HttpCompletionOption.ResponseHeadersRead, _cancelToken.Token).GetAwaiter().GetResult(); + response = GetResponse(client, redirectRequest, keepAuthorization); } } @@ -1333,7 +1323,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM // are treated as a standard -OutFile request. This also disables appending local file. Resume = new SwitchParameter(false); - using (HttpRequestMessage requestWithoutRange = GetRequest(currentUri, stripAuthorization:false)) + using (HttpRequestMessage requestWithoutRange = GetRequest(currentUri)) { FillRequestStream(requestWithoutRange); long requestContentLength = 0; @@ -1347,7 +1337,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM requestContentLength); WriteVerbose(reqVerboseMsg); - return GetResponse(client, requestWithoutRange, stripAuthorization); + return GetResponse(client, requestWithoutRange, keepAuthorization); } } @@ -1377,15 +1367,15 @@ protected override void ProcessRecord() // if the request contains an authorization header and PreserveAuthorizationOnRedirect is not set, // it needs to be stripped on the first redirect. - bool stripAuthorization = null != WebSession + bool keepAuthorization = null != WebSession && null != WebSession.Headers && - !PreserveAuthorizationOnRedirect.IsPresent + PreserveAuthorizationOnRedirect.IsPresent && WebSession.Headers.ContainsKey(HttpKnownHeaderNames.Authorization.ToString()); - using (HttpClient client = GetHttpClient(stripAuthorization)) + using (HttpClient client = GetHttpClient(keepAuthorization)) { int followedRelLink = 0; Uri uri = Uri; @@ -1399,7 +1389,7 @@ protected override void ProcessRecord() WriteVerbose(linkVerboseMsg); } - using (HttpRequestMessage request = GetRequest(uri, stripAuthorization:false)) + using (HttpRequestMessage request = GetRequest(uri)) { FillRequestStream(request); try @@ -1415,7 +1405,7 @@ protected override void ProcessRecord() requestContentLength); WriteVerbose(reqVerboseMsg); - HttpResponseMessage response = GetResponse(client, request, stripAuthorization); + HttpResponseMessage response = GetResponse(client, request, keepAuthorization); string contentType = ContentHelper.GetContentType(response); string respVerboseMsg = string.Format(CultureInfo.CurrentCulture, diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 index a954656afb2..68f7925fed7 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 @@ -349,8 +349,8 @@ function GetMultipartBody { for additonal details. #> $redirectTests = @( - @{redirectType = 'MultipleChoices'; redirectedMethod = 'POST'} - @{redirectType = 'Ambiguous'; redirectedMethod = 'POST'} # Synonym for MultipleChoices + @{redirectType = 'MultipleChoices'; redirectedMethod = 'GET'} + @{redirectType = 'Ambiguous'; redirectedMethod = 'GET'} # Synonym for MultipleChoices @{redirectType = 'Moved'; redirectedMethod = 'GET'} @{redirectType = 'MovedPermanently'; redirectedMethod = 'GET'} # Synonym for Moved @@ -361,8 +361,8 @@ $redirectTests = @( @{redirectType = 'redirectMethod'; redirectedMethod = 'GET'} @{redirectType = 'SeeOther'; redirectedMethod = 'GET'} # Synonym for RedirectMethod - @{redirectType = 'TemporaryRedirect'; redirectedMethod = 'GET'} - @{redirectType = 'RedirectKeepVerb'; redirectedMethod = 'GET'} # Synonym for TemporaryRedirect + @{redirectType = 'TemporaryRedirect'; redirectedMethod = 'POST'} + @{redirectType = 'RedirectKeepVerb'; redirectedMethod = 'POST'} # Synonym for TemporaryRedirect @{redirectType = 'relative'; redirectedMethod = 'GET'} ) @@ -778,7 +778,7 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" { #region Redirect tests - It "Validates Invoke-WebRequest with -PreserveAuthorizationOnRedirect preserves the authorization header on redirect: " -Pending -TestCases $redirectTests { + It "Validates Invoke-WebRequest with -PreserveAuthorizationOnRedirect preserves the authorization header on redirect: " -TestCases $redirectTests { param($redirectType, $redirectedMethod) $uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType} $response = ExecuteRedirectRequest -Uri $uri -PreserveAuthorizationOnRedirect @@ -787,7 +787,7 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" { $response.Content.Headers."Authorization" | Should -BeExactly "test" } - It "Validates Invoke-WebRequest preserves the authorization header on multiple redirects: " -Pending -TestCases $redirectTests { + It "Validates Invoke-WebRequest preserves the authorization header on multiple redirects: " -TestCases $redirectTests { param($redirectType) $uri = Get-WebListenerUrl -Test 'Redirect' -TestValue 3 -Query @{type = $redirectType} $response = ExecuteRedirectRequest -Uri $uri -PreserveAuthorizationOnRedirect @@ -796,7 +796,7 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" { $response.Content.Headers."Authorization" | Should -BeExactly "test" } - It "Validates Invoke-WebRequest strips the authorization header on various redirects: " -Pending -TestCases $redirectTests { + It "Validates Invoke-WebRequest strips the authorization header on various redirects: " -TestCases $redirectTests { param($redirectType) $uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType} $response = ExecuteRedirectRequest -Uri $uri @@ -810,7 +810,7 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" { # NOTE: Only testing redirection of POST -> GET for unique underlying values of HttpStatusCode. # Some names overlap in underlying value. - It "Validates Invoke-WebRequest strips the authorization header redirects and switches from POST to GET when it handles the redirect: " -Pending -TestCases $redirectTests { + It "Validates Invoke-WebRequest strips the authorization header redirects and switches from POST to GET when it handles the redirect: " -TestCases $redirectTests { param($redirectType, $redirectedMethod) $uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType} $response = ExecuteRedirectRequest -Uri $uri -Method 'POST' @@ -824,6 +824,20 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" { $response.Content.Method | Should -Be $redirectedMethod } + It "Validates Invoke-WebRequest -PreserveAuthorizationOnRedirect keeps the authorization header redirects and switches from POST to GET when it handles the redirect: " -TestCases $redirectTests { + param($redirectType, $redirectedMethod) + $uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType} + $response = ExecuteRedirectRequest -PreserveAuthorizationOnRedirect -Uri $uri -Method 'POST' + + $response.Error | Should -BeNullOrEmpty + # ensure user-agent is present (i.e., no false positives ) + $response.Content.Headers."User-Agent" | Should -Not -BeNullOrEmpty + # ensure Authorization header has been removed. + $response.Content.Headers."Authorization" | Should -BeExactly 'test' + # ensure POST was changed to GET for selected redirections and remains as POST for others. + $response.Content.Method | Should -Be $redirectedMethod + } + It "Validates Invoke-WebRequest handles responses without Location header for requests with Authorization header and redirect: " -TestCases $redirectTests { param($redirectType, $redirectedMethod) # Skip relative test as it is not a valid response type. @@ -2113,7 +2127,7 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" { #region Redirect tests - It "Validates Invoke-RestMethod with -PreserveAuthorizationOnRedirect preserves the authorization header on redirect: " -Pending -TestCases $redirectTests { + It "Validates Invoke-RestMethod with -PreserveAuthorizationOnRedirect preserves the authorization header on redirect: " -TestCases $redirectTests { param($redirectType, $redirectedMethod) $uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType} $response = ExecuteRedirectRequest -Cmdlet 'Invoke-RestMethod' -Uri $uri -PreserveAuthorizationOnRedirect @@ -2123,7 +2137,7 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" { $response.Content.Headers."Authorization" | Should -BeExactly "test" } - It "Validates Invoke-RestMethod preserves the authorization header on multiple redirects: " -Pending -TestCases $redirectTests { + It "Validates Invoke-RestMethod preserves the authorization header on multiple redirects: " -TestCases $redirectTests { param($redirectType) $uri = Get-WebListenerUrl -Test 'Redirect' -TestValue 3 -Query @{type = $redirectType} $response = ExecuteRedirectRequest -Cmdlet 'Invoke-RestMethod' -Uri $uri -PreserveAuthorizationOnRedirect @@ -2133,7 +2147,7 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" { $response.Content.Headers."Authorization" | Should -BeExactly "test" } - It "Validates Invoke-RestMethod strips the authorization header on various redirects: " -Pending -TestCases $redirectTests { + It "Validates Invoke-RestMethod strips the authorization header on various redirects: " -TestCases $redirectTests { param($redirectType) $uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType} $response = ExecuteRedirectRequest -Cmdlet 'Invoke-RestMethod' -Uri $uri @@ -2147,7 +2161,7 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" { # NOTE: Only testing redirection of POST -> GET for unique underlying values of HttpStatusCode. # Some names overlap in underlying value. - It "Validates Invoke-RestMethod strips the authorization header redirects and switches from POST to GET when it handles the redirect: " -Pending -TestCases $redirectTests { + It "Validates Invoke-RestMethod strips the authorization header redirects and switches from POST to GET when it handles the redirect: " -TestCases $redirectTests { param($redirectType, $redirectedMethod) $uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType} $response = ExecuteRedirectRequest -Cmdlet 'Invoke-RestMethod' -Uri $uri -Method 'POST' @@ -2156,7 +2170,21 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" { # ensure user-agent is present (i.e., no false positives ) $response.Content.Headers."User-Agent" | Should -Not -BeNullOrEmpty # ensure Authorization header has been removed. - $response.Content."Authorization" | Should -BeNullOrEmpty + $response.Content.Headers."Authorization" | Should -BeNullOrEmpty + # ensure POST was changed to GET for selected redirections and remains as POST for others. + $response.Content.Method | Should -Be $redirectedMethod + } + + It "Validates Invoke-RestMethod -PreserveAuthorizationOnRedirect keeps the authorization header redirects and switches from POST to GET when it handles the redirect: " -TestCases $redirectTests { + param($redirectType, $redirectedMethod) + $uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType} + $response = ExecuteRedirectRequest -PreserveAuthorizationOnRedirect -Cmdlet 'Invoke-RestMethod' -Uri $uri -Method 'POST' + + $response.Error | Should -BeNullOrEmpty + # ensure user-agent is present (i.e., no false positives ) + $response.Content.Headers."User-Agent" | Should -Not -BeNullOrEmpty + # ensure Authorization header has been removed. + $response.Content.Headers."Authorization" | Should -BeExactly 'test' # ensure POST was changed to GET for selected redirections and remains as POST for others. $response.Content.Method | Should -Be $redirectedMethod } From 2f31e1a085cdc819a9604ae3d339fddc50ddfb95 Mon Sep 17 00:00:00 2001 From: Mark Kraus Date: Fri, 11 May 2018 03:28:21 -0500 Subject: [PATCH 2/2] [Feature] Add parameter name for readability --- .../utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index a64ae3f096d..15227211f0f 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -1300,7 +1300,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM currentUri = new Uri(request.RequestUri, response.Headers.Location); // Continue to handle redirection - using (client = GetHttpClient(true)) + using (client = GetHttpClient(handleRedirect: true)) using (HttpRequestMessage redirectRequest = GetRequest(currentUri)) { response = GetResponse(client, redirectRequest, keepAuthorization);