8000 Added check for existence of Location HTTP header before using it by ffeldhaus · Pull Request #6560 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Added check for existence of Location HTTP header before using it #6560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 8, 2018
Merged

Added check for existence of Location HTTP header before using it #6560

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3f2bfb5
Check existence of Location header before using it
ffeldhaus Apr 3, 2018
6ecc705
Fixed Location header check to validate if it is not null
ffeldhaus Apr 4, 2018
d04be33
[Feature] Added tests to validate that WebCmdlets correctly handle re…
ffeldhaus Apr 4, 2018
f44f0db
Revert unrelated file about_PSReadline.help.txt
ffeldhaus Apr 4, 2018
bcd2588
[feature] Changed status code from string to value to allow WebListen…
ffeldhaus Apr 4, 2018
85c6c2c
[feature] Changed status code from string to int and using -Match ins…
ffeldhaus Apr 5, 2018
b61860b
[feature] Skipping relative test as it is not a valid response type a…
ffeldhaus Apr 5, 2018
33dba99
[feature] Clarified test description and added more information on wh…
ffeldhaus Apr 6, 2018
7e54d41
[feature] Shortened test description by one word and added whitespace
ffeldhaus Apr 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[feature] Changed status code from string to value to allow WebListen… 
…er to respond correctly
  • Loading branch information
@ffeldhaus
ffeldhaus committed Apr 4, 2018
commit bcd258830f8884e06d55382a9ac2e684c42721c9
6 changes: 2 additions & 4 deletions test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,11 +828,10 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {
# for a redirect response which does not contain a Location header
# the correct redirect status code should be included in the exception message
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should include a note about this requiring a Authorization header. Something like

# When an Authorization request header is present,
# and -PreserveAuthorizationOnRedirect is not present,
# PowerShell should throw an HTTP Response Exception
# for a redirect response which does not contain a Location response header.
# The correct redirect status code should be included in the exception.

It's a mouth full, but at least people will understand why we are doing what seems like a superfluous set of tests.

$StatusCode = [System.Net.HttpStatusCode]$redirectType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use this:

[int]$StatusCode = [System.Net.HttpStatusCode]$redirectType

Then you no longer need to do $StatusCode.value__ below. Makes it a bit cleaner, IMO.

$uri = Get-WebListenerUrl -Test Response -Query @{statuscode = $StatusCode}
$uri = Get-WebListenerUrl -Test Response -Query @{statuscode = $StatusCode.value__}
$command = "Invoke-WebRequest -Uri '$uri' -Headers @{Authorization = 'foo'}"
$response = ExecuteWebCommand -command $command

$response.Error | Should -Not -BeNullOrEmpty
$response.Error.Exception | Should -BeOfType 'Microsoft.PowerShell.Commands.HttpResponseException'
$response.Error.Exception.Message | Should -Be "Response status code does not indicate success: $(StatusCode.value__) ($StatusCode)."
$response.Error.Exception.Response.StatusCode | Should -Be $StatusCode
Expand Down Expand Up @@ -2160,11 +2159,10 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" {
# for a redirect response which does not contain a Location header
# the correct redirect status code should be included in the exception message
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

$StatusCode = [System.Net.HttpStatusCode]$redirectType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same.

$uri = Get-WebListenerUrl -Test Response -Query @{statuscode = $StatusCode}
$uri = Get-WebListenerUrl -Test Response -Query @{statuscode = $StatusCode.value__}
$command = "Invoke-RestMethod -Uri '$uri' -Headers @{Authorization = 'foo'}"
$response = ExecuteWebCommand -command $command

$response.Error | Should -Not -BeNullOrEmpty
$response.Error.Exception | Should -BeOfType 'Microsoft.PowerShell.Commands.HttpResponseException'
$response.Error.Exception.Message | Should -Be "Response status code does not indicate success: $(StatusCode.value__) ($StatusCode)."
$response.Error.Exception.Response.StatusCode | Should -Be $StatusCode
Expand Down
0