Skip to content

Change Web Cmdlet Tests to Use 127.0.0.1 not Localhost #6069

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
iSazonov merged 3 commits into from
Feb 1, 2018
Merged

Change Web Cmdlet Tests to Use 127.0.0.1 not Localhost #6069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d92edb7
[feature] Change Web Cmdlet Tests to Use 127.0.0.1 not Localhost
markekraus Jan 30, 2018
40b431c
[feature] Replace localhost with 127.0.0.1 for proxy tests
markekraus Jan 31, 2018
56504d1
Add Comment about CoreFX issue
markekraus Jan 31, 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
12 changes: 6 additions & 6 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 @@ -496,15 +496,15 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {

It "Validate Invoke-WebRequest error with -Proxy and -NoProxy option" {
$uri = Get-WebListenerUrl -Test 'Delay' -TestValue '10'
$command = "Invoke-WebRequest -Uri '$uri' -Proxy 'http://localhost:8080' -NoProxy -TimeoutSec 2"
$command = "Invoke-WebRequest -Uri '$uri' -Proxy 'http://127.0.0.1:8080' -NoProxy -TimeoutSec 2"

$result = ExecuteWebCommand -command $command
$result.Error.FullyQualifiedErrorId | Should Be "AmbiguousParameterSet,Microsoft.PowerShell.Commands.InvokeWebRequestCommand"
}

$testCase = @(
@{ proxy_address = "http://localhost:9"; name = 'http_proxy'; protocol = 'http' }
@{ proxy_address = "http://localhost:9"; name = 'https_proxy'; protocol = 'https' }
@{ proxy_address = "http://127.0.0.1:9"; name = 'http_proxy'; protocol = 'http' }
@{ proxy_address = "http://127.0.0.1:9"; name = 'https_proxy'; protocol = 'https' }
)

It "Validate Invoke-WebRequest error with -Proxy option set - '<name>'" -TestCases $testCase {
Expand Down Expand Up @@ -1632,15 +1632,15 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" {

It "Validate Invoke-RestMethod error with -Proxy and -NoProxy option" {
$uri = Get-WebListenerUrl -Test 'Delay' -TestValue '10'
$command = "Invoke-RestMethod -Uri '$uri' -Proxy 'http://localhost:8080' -NoProxy -TimeoutSec 2"
$command = "Invoke-RestMethod -Uri '$uri' -Proxy 'http://127.0.0.1:8080' -NoProxy -TimeoutSec 2"

$result = ExecuteWebCommand -command $command
$result.Error.FullyQualifiedErrorId | Should Be "AmbiguousParameterSet,Microsoft.PowerShell.Commands.InvokeRestMethodCommand"
}

$testCase = @(
@{ proxy_address = "http://localhost:9"; name = 'http_proxy'; protocol = 'http' }
@{ proxy_address = "http://localhost:9"; name = 'https_proxy'; protocol = 'https' }
@{ proxy_address = "http://127.0.0.1:9"; name = 'http_proxy'; protocol = 'http' }
@{ proxy_address = "http://127.0.0.1:9"; name = 'https_proxy'; protocol = 'https' }
)

It "Validate Invoke-RestMethod error with -Proxy option - '<name>'" -TestCases $testCase {
Expand Down
3 changes: 2 additions & 1 deletion test/tools/Modules/WebListener/WebListener.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ function Get-WebListenerUrl {
return $null
}
$Uri = [System.UriBuilder]::new()
$Uri.Host = 'localhost'
# Use 127.0.0.1 and not localhost due to https://github.com/dotnet/corefx/issues/24104
Copy link
Collaborator

@iSazonov iSazonov Jan 31, 2018

Choose a reason for hiding this comment

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

I'd add "TODO: remove after fix https://github.com/dotnet/corefx/issues/24104"

Copy link
Contributor Author

@markekraus markekraus Jan 31, 2018

Choose a reason for hiding this comment

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

why would we ever change it back? we are binding WebListener to 127.0.0.1, not to 'localhost'. it's more correct to keep it at 127.0.0.1.

Copy link
Collaborator

@iSazonov iSazonov Jan 31, 2018

Choose a reason for hiding this comment

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

Good question! 😄 It seems IPv4 never die.

Copy link
Member

@TravisEz13 TravisEz13 Jan 31, 2018

Choose a reason for hiding this comment

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

At least we will have to make significant changes to the tests if we get rid of IPv4. I was thinking that perhaps we should define a function to get the value for localhost. Then, we only have one place to change, but this isn't a blocking comment.

Copy link
Contributor Author

@markekraus markekraus Jan 31, 2018

Choose a reason for hiding this comment

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

It could be done. for the most part.. the web cmdlets already do that since they get the url's from Get-WebListenerUrl the web listener module would just need to call whatever function we share with the rest of the tests.

The rest of the tests in the project may not even benefit from this change. I did a quick look this morning and they were mostly \\localhost\ (I'm not sure SMB is affected, the CoreFX issue seems to indicate it's specific to HTTP, some of those tests may need localhost regardless) or WSMan:\localhost\ (which wouldn't change).

Copy link
Collaborator

@iSazonov iSazonov Feb 1, 2018

Choose a reason for hiding this comment

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

In fact it's not worth.

$Uri.Host = '127.0.0.1'
$Uri.Port = $runningListener.HttpPort
$Uri.Scheme = 'Http'

Expand Down