flofreud / posh-gvm

GVM for Powershell
Apache License 2.0
210 stars 60 forks source link

Added proxy abilities #9

Open santidhammo opened 8 years ago

santidhammo commented 8 years ago

Added proxy abilities to support proxying on URL operations.

flofreud commented 8 years ago

Thank you for the contribution. Its much appreciated.

I just tested your code against the current master and it works quite well without a defined proxy. I have no proxy to test it otherwise. I have to fix some unit tests because the mocks doesn't match anymore, but this is easy to fix.

Could you help me with the following questions?

  1. The [System.Net.WebRequest]::GetSystemWebProxy() call always returns a proxy, isn't it? If the system has no proxy-setup than this proxy seems to be empty and the call is made directly. I couldn't construct a cause where I didn't got a proxy. Did I miss anything? Is this only the behavior in my Windows version? If not, the code could be simplified, but we could keep it like this to be on the safe side.
  2. Why is there this $server in the line $proxyUri = $proxy.GetProxy("$server$url")? $server should be undefined in this context and always resolves to $null and therefore does the same as $proxyUri = $proxy.GetProxy("$URL"), correct?
  3. If no proxy is defined I get the $URL as return value from $proxy.GetProxy("$URL") therefore the Invoke-RestMethod call with the $URL also as proxy. This is probably correctly handles inside of Invoke-RestMethod but would it not be more clean to check this case?
santidhammo commented 8 years ago

1.) This was exactly my issue too, I have no machine available where it is not the case, however, I can check is on someone else's

2.) That appears to be a mistake

3.) Definetely

Met vriendelijke groet, Sjoerd

2015-12-13 12:46 GMT+01:00 Florian Freudenberg notifications@github.com:

Thank you for the contribution. Its much appreciated.

I just tested your code against the current master and it works quite well without a defined proxy. I have no proxy to test it otherwise. I have to fix some unit tests because the mocks doesn't match anymore, but this is easy to fix.

Could you help me with the following questions?

1.

The [System.Net.WebRequest]::GetSystemWebProxy() call always returns a proxy, isn't it? If the system has no proxy-setup than this proxy seems to be empty and the call is made directly. I couldn't construct a cause where I didn't got a proxy. Did I miss anything? Is this only the behavior in my Windows version? If not, the code could be simplified, but we could keep it like this to be on the safe side. 2.

Why is there this $server in the line $proxyUri = $proxy.GetProxy("$server$url")? $server should be undefined in this context and always resolves to $null and therefore does the same as $proxyUri = $proxy.GetProxy("$URL"), correct? 3.

If no proxy is defined I get the $URL as return value from $proxy.GetProxy("$URL") therefore the Invoke-RestMethod call with the $URL also as proxy. This is probably correctly handles inside of Invoke-RestMethod but would it not be more clean to check this case?

— Reply to this email directly or view it on GitHub https://github.com/flofreud/posh-gvm/pull/9#issuecomment-164251474.