Skip to content

Commit 525f593

Browse files
markekrausTravisEz13
markekraus
authored and
TravisEz13
committed
Fix Web Cmdlets for .NET Core 2.1 (#6806)
closes #6728 Breaking change approved in #6728 This PR switches the logic of when the Web Cmdlets handle redirects when the Authorization header is present. .NET Core 2.1 no longer sends the Authorization header by default (dotnet/corefx#26864). however, we introduced the ability to do so leveraging the previous default behavior through the use of the -PreserveAuthorizationOnRedirect switch. This PR also corrects a bug introduced 6.0.0 where certain redirect types redirect from POST to GET were set which should have passed through POST to POST and some were improperly passing through POST to POST which should have been doing POST to GET. This correction is a breaking change. It was made apparent as now the redirection behavior is being managed by CoreFX which is doing the correct behavior, tests were added for both when CoreFX and the Web Cmdlets manage redirection.
1 parent d07a3e7 commit 525f593

2 files changed

Lines changed: 58 additions & 40 deletions

File tree

‎src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs‎

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,7 @@ internal virtual HttpClient GetHttpClient(bool handleRedirect)
993993
return httpClient;
994994
}
995995

996-
internal virtual HttpRequestMessage GetRequest(Uri uri, bool stripAuthorization)
996+
internal virtual HttpRequestMessage GetRequest(Uri uri)
997997
{
998998
Uri requestUri = PrepareUri(uri);
999999
HttpMethod httpMethod = null;
@@ -1032,14 +1032,6 @@ internal virtual HttpRequestMessage GetRequest(Uri uri, bool stripAuthorization)
10321032
}
10331033
else
10341034
{
1035-
if (stripAuthorization
1036-
&&
1037-
String.Equals(entry.Key, HttpKnownHeaderNames.Authorization.ToString(), StringComparison.OrdinalIgnoreCase)
1038-
)
1039-
{
1040-
continue;
1041-
}
1042-
10431035
if (SkipHeaderValidation)
10441036
{
10451037
request.Headers.TryAddWithoutValidation(entry.Key, entry.Value);
@@ -1268,15 +1260,15 @@ static bool IsRedirectToGet(HttpStatusCode code)
12681260
||
12691261
code == HttpStatusCode.RedirectMethod
12701262
||
1271-
code == HttpStatusCode.TemporaryRedirect
1263+
code == HttpStatusCode.SeeOther
12721264
||
1273-
code == HttpStatusCode.RedirectKeepVerb
1265+
code == HttpStatusCode.Ambiguous
12741266
||
1275-
code == HttpStatusCode.SeeOther
1267+
code == HttpStatusCode.MultipleChoices
12761268
);
12771269
}
12781270

1279-
internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestMessage request, bool stripAuthorization)
1271+
internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestMessage request, bool keepAuthorization)
12801272
{
12811273
if (client == null) { throw new ArgumentNullException("client"); }
12821274
if (request == null) { throw new ArgumentNullException("request"); }
@@ -1287,7 +1279,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM
12871279
_cancelToken = new CancellationTokenSource();
12881280
HttpResponseMessage response = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, _cancelToken.Token).GetAwaiter().GetResult();
12891281

1290-
if (stripAuthorization && IsRedirectCode(response.StatusCode) && response.Headers.Location != null)
1282+
if (keepAuthorization && IsRedirectCode(response.StatusCode) && response.Headers.Location != null)
12911283
{
12921284
_cancelToken.Cancel();
12931285
_cancelToken = null;
@@ -1306,14 +1298,12 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM
13061298
Method = WebRequestMethod.Get;
13071299
}
13081300

1309-
// recreate the HttpClient with redirection enabled since the first call suppressed redirection
13101301
currentUri = new Uri(request.RequestUri, response.Headers.Location);
1311-
using (client = GetHttpClient(false))
1312-
using (HttpRequestMessage redirectRequest = GetRequest(currentUri, stripAuthorization:true))
1302+
// Continue to handle redirection
1303+
using (client = GetHttpClient(handleRedirect: true))
1304+
using (HttpRequestMessage redirectRequest = GetRequest(currentUri))
13131305
{
1314-
FillRequestStream(redirectRequest);
1315-
_cancelToken = new CancellationTokenSource();
1316-
response = client.SendAsync(redirectRequest, HttpCompletionOption.ResponseHeadersRead, _cancelToken.Token).GetAwaiter().GetResult();
1306+
response = GetResponse(client, redirectRequest, keepAuthorization);
13171307
}
13181308
}
13191309

@@ -1333,7 +1323,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM
13331323
// are treated as a standard -OutFile request. This also disables appending local file.
13341324
Resume = new SwitchParameter(false);
13351325

1336-
using (HttpRequestMessage requestWithoutRange = GetRequest(currentUri, stripAuthorization:false))
1326+
using (HttpRequestMessage requestWithoutRange = GetRequest(currentUri))
13371327
{
13381328
FillRequestStream(requestWithoutRange);
13391329
long requestContentLength = 0;
@@ -1347,7 +1337,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM
13471337
requestContentLength);
13481338
WriteVerbose(reqVerboseMsg);
13491339

1350-
return GetResponse(client, requestWithoutRange, stripAuthorization);
1340+
return GetResponse(client, requestWithoutRange, keepAuthorization);
13511341
}
13521342
}
13531343

@@ -1377,15 +1367,15 @@ protected override void ProcessRecord()
13771367

13781368
// if the request contains an authorization header and PreserveAuthorizationOnRedirect is not set,
13791369
// it needs to be stripped on the first redirect.
1380-
bool stripAuthorization = null != WebSession
1370+
bool keepAuthorization = null != WebSession
13811371
&&
13821372
null != WebSession.Headers
13831373
&&
1384-
!PreserveAuthorizationOnRedirect.IsPresent
1374+
PreserveAuthorizationOnRedirect.IsPresent
13851375
&&
13861376
WebSession.Headers.ContainsKey(HttpKnownHeaderNames.Authorization.ToString());
13871377

1388-
using (HttpClient client = GetHttpClient(stripAuthorization))
1378+
using (HttpClient client = GetHttpClient(keepAuthorization))
13891379
{
13901380
int followedRelLink = 0;
13911381
Uri uri = Uri;
@@ -1399,7 +1389,7 @@ protected override void ProcessRecord()
13991389
WriteVerbose(linkVerboseMsg);
14001390
}
14011391

1402-
using (HttpRequestMessage request = GetRequest(uri, stripAuthorization:false))
1392+
using (HttpRequestMessage request = GetRequest(uri))
14031393
{
14041394
FillRequestStream(request);
14051395
try
@@ -1415,7 +1405,7 @@ protected override void ProcessRecord()
14151405
requestContentLength);
14161406
WriteVerbose(reqVerboseMsg);
14171407

1418-
HttpResponseMessage response = GetResponse(client, request, stripAuthorization);
1408+
HttpResponseMessage response = GetResponse(client, request, keepAuthorization);
14191409

14201410
string contentType = ContentHelper.GetContentType(response);
14211411
string respVerboseMsg = string.Format(CultureInfo.CurrentCulture,

‎test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1‎

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,8 @@ function GetMultipartBody {
349349
for additonal details.
350350
#>
351351
$redirectTests = @(
352-
@{redirectType = 'MultipleChoices'; redirectedMethod = 'POST'}
353-
@{redirectType = 'Ambiguous'; redirectedMethod = 'POST'} # Synonym for MultipleChoices
352+
@{redirectType = 'MultipleChoices'; redirectedMethod = 'GET'}
353+
@{redirectType = 'Ambiguous'; redirectedMethod = 'GET'} # Synonym for MultipleChoices
354354

355355
@{redirectType = 'Moved'; redirectedMethod = 'GET'}
356356
@{redirectType = 'MovedPermanently'; redirectedMethod = 'GET'} # Synonym for Moved
@@ -361,8 +361,8 @@ $redirectTests = @(
361361
@{redirectType = 'redirectMethod'; redirectedMethod = 'GET'}
362362
@{redirectType = 'SeeOther'; redirectedMethod = 'GET'} # Synonym for RedirectMethod
363363

364-
@{redirectType = 'TemporaryRedirect'; redirectedMethod = 'GET'}
365-
@{redirectType = 'RedirectKeepVerb'; redirectedMethod = 'GET'} # Synonym for TemporaryRedirect
364+
@{redirectType = 'TemporaryRedirect'; redirectedMethod = 'POST'}
365+
@{redirectType = 'RedirectKeepVerb'; redirectedMethod = 'POST'} # Synonym for TemporaryRedirect
366366

367367
@{redirectType = 'relative'; redirectedMethod = 'GET'}
368368
)
@@ -741,7 +741,7 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {
741741

742742
#region Redirect tests
743743

744-
It "Validates Invoke-WebRequest with -PreserveAuthorizationOnRedirect preserves the authorization header on redirect: <redirectType> <redirectedMethod>" -Pending -TestCases $redirectTests {
744+
It "Validates Invoke-WebRequest with -PreserveAuthorizationOnRedirect preserves the authorization header on redirect: <redirectType> <redirectedMethod>" -TestCases $redirectTests {
745745
param($redirectType, $redirectedMethod)
746746
$uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType}
747747
$response = ExecuteRedirectRequest -Uri $uri -PreserveAuthorizationOnRedirect
@@ -750,7 +750,7 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {
750750
$response.Content.Headers."Authorization" | Should -BeExactly "test"
751751
}
752752

753-
It "Validates Invoke-WebRequest preserves the authorization header on multiple redirects: <redirectType>" -Pending -TestCases $redirectTests {
753+
It "Validates Invoke-WebRequest preserves the authorization header on multiple redirects: <redirectType>" -TestCases $redirectTests {
754754
param($redirectType)
755755
$uri = Get-WebListenerUrl -Test 'Redirect' -TestValue 3 -Query @{type = $redirectType}
756756
$response = ExecuteRedirectRequest -Uri $uri -PreserveAuthorizationOnRedirect
@@ -759,7 +759,7 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {
759759
$response.Content.Headers."Authorization" | Should -BeExactly "test"
760760
}
761761

762-
It "Validates Invoke-WebRequest strips the authorization header on various redirects: <redirectType>" -Pending -TestCases $redirectTests {
762+
It "Validates Invoke-WebRequest strips the authorization header on various redirects: <redirectType>" -TestCases $redirectTests {
763763
param($redirectType)
764764
$uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType}
765765
$response = ExecuteRedirectRequest -Uri $uri
@@ -773,7 +773,7 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {
773773

774774
# NOTE: Only testing redirection of POST -> GET for unique underlying values of HttpStatusCode.
775775
# Some names overlap in underlying value.
776-
It "Validates Invoke-WebRequest strips the authorization header redirects and switches from POST to GET when it handles the redirect: <redirectType> <redirectedMethod>" -Pending -TestCases $redirectTests {
776+
It "Validates Invoke-WebRequest strips the authorization header redirects and switches from POST to GET when it handles the redirect: <redirectType> <redirectedMethod>" -TestCases $redirectTests {
777777
param($redirectType, $redirectedMethod)
778778
$uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType}
779779
$response = ExecuteRedirectRequest -Uri $uri -Method 'POST'
@@ -787,6 +787,20 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {
787787
$response.Content.Method | Should -Be $redirectedMethod
788788
}
789789

790+
It "Validates Invoke-WebRequest -PreserveAuthorizationOnRedirect keeps the authorization header redirects and switches from POST to GET when it handles the redirect: <redirectType> <redirectedMethod>" -TestCases $redirectTests {
791+
param($redirectType, $redirectedMethod)
792+
$uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType}
793+
$response = ExecuteRedirectRequest -PreserveAuthorizationOnRedirect -Uri $uri -Method 'POST'
794+
795+
$response.Error | Should -BeNullOrEmpty
796+
# ensure user-agent is present (i.e., no false positives )
797+
$response.Content.Headers."User-Agent" | Should -Not -BeNullOrEmpty
798+
# ensure Authorization header has been removed.
799+
$response.Content.Headers."Authorization" | Should -BeExactly 'test'
800+
# ensure POST was changed to GET for selected redirections and remains as POST for others.
801+
$response.Content.Method | Should -Be $redirectedMethod
802+
}
803+
790804
It "Validates Invoke-WebRequest handles responses without Location header for requests with Authorization header and redirect: <redirectType>" -TestCases $redirectTests {
791805
param($redirectType, $redirectedMethod)
792806
# Skip relative test as it is not a valid response type.
@@ -2017,7 +2031,7 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" {
20172031

20182032
#region Redirect tests
20192033

2020-
It "Validates Invoke-RestMethod with -PreserveAuthorizationOnRedirect preserves the authorization header on redirect: <redirectType> <redirectedMethod>" -Pending -TestCases $redirectTests {
2034+
It "Validates Invoke-RestMethod with -PreserveAuthorizationOnRedirect preserves the authorization header on redirect: <redirectType> <redirectedMethod>" -TestCases $redirectTests {
20212035
param($redirectType, $redirectedMethod)
20222036
$uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType}
20232037
$response = ExecuteRedirectRequest -Cmdlet 'Invoke-RestMethod' -Uri $uri -PreserveAuthorizationOnRedirect
@@ -2027,7 +2041,7 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" {
20272041
$response.Content.Headers."Authorization" | Should -BeExactly "test"
20282042
}
20292043

2030-
It "Validates Invoke-RestMethod preserves the authorization header on multiple redirects: <redirectType>" -Pending -TestCases $redirectTests {
2044+
It "Validates Invoke-RestMethod preserves the authorization header on multiple redirects: <redirectType>" -TestCases $redirectTests {
20312045
param($redirectType)
20322046
$uri = Get-WebListenerUrl -Test 'Redirect' -TestValue 3 -Query @{type = $redirectType}
20332047
$response = ExecuteRedirectRequest -Cmdlet 'Invoke-RestMethod' -Uri $uri -PreserveAuthorizationOnRedirect
@@ -2037,7 +2051,7 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" {
20372051
$response.Content.Headers."Authorization" | Should -BeExactly "test"
20382052
}
20392053

2040-
It "Validates Invoke-RestMethod strips the authorization header on various redirects: <redirectType>" -Pending -TestCases $redirectTests {
2054+
It "Validates Invoke-RestMethod strips the authorization header on various redirects: <redirectType>" -TestCases $redirectTests {
20412055
param($redirectType)
20422056
$uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType}
20432057
$response = ExecuteRedirectRequest -Cmdlet 'Invoke-RestMethod' -Uri $uri
@@ -2051,7 +2065,7 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" {
20512065

20522066
# NOTE: Only testing redirection of POST -> GET for unique underlying values of HttpStatusCode.
20532067
# Some names overlap in underlying value.
2054-
It "Validates Invoke-RestMethod strips the authorization header redirects and switches from POST to GET when it handles the redirect: <redirectType> <redirectedMethod>" -Pending -TestCases $redirectTests {
2068+
It "Validates Invoke-RestMethod strips the authorization header redirects and switches from POST to GET when it handles the redirect: <redirectType> <redirectedMethod>" -TestCases $redirectTests {
20552069
param($redirectType, $redirectedMethod)
20562070
$uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType}
20572071
$response = ExecuteRedirectRequest -Cmdlet 'Invoke-RestMethod' -Uri $uri -Method 'POST'
@@ -2060,7 +2074,21 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" {
20602074
# ensure user-agent is present (i.e., no false positives )
20612075
$response.Content.Headers."User-Agent" | Should -Not -BeNullOrEmpty
20622076
# ensure Authorization header has been removed.
2063-
$response.Content."Authorization" | Should -BeNullOrEmpty
2077+
$response.Content.Headers."Authorization" | Should -BeNullOrEmpty
2078+
# ensure POST was changed to GET for selected redirections and remains as POST for others.
2079+
$response.Content.Method | Should -Be $redirectedMethod
2080+
}
2081+
2082+
It "Validates Invoke-RestMethod -PreserveAuthorizationOnRedirect keeps the authorization header redirects and switches from POST to GET when it handles the redirect: <redirectType> <redirectedMethod>" -TestCases $redirectTests {
2083+
param($redirectType, $redirectedMethod)
2084+
$uri = Get-WebListenerUrl -Test 'Redirect' -Query @{type = $redirectType}
2085+
$response = ExecuteRedirectRequest -PreserveAuthorizationOnRedirect -Cmdlet 'Invoke-RestMethod' -Uri $uri -Method 'POST'
2086+
2087+
$response.Error | Should -BeNullOrEmpty
2088+
# ensure user-agent is present (i.e., no false positives )
2089+
$response.Content.Headers."User-Agent" | Should -Not -BeNullOrEmpty
2090+
# ensure Authorization header has been removed.
2091+
$response.Content.Headers."Authorization" | Should -BeExactly 'test'
20642092
# ensure POST was changed to GET for selected redirections and remains as POST for others.
20652093
$response.Content.Method | Should -Be $redirectedMethod
20662094
}

0 commit comments

Comments
 (0)