Typo: compatibility

Fixed

Generally, you should not mix formatting changes with functional changes. Have them as separate PRs to make them more readable and reviewable.

You should put this in WebCmdletStrings.resx

I'll remove it. Not appropriate for release.

Don't we need to support other 3xx redirect codes?

https://en.wikipedia.org/wiki/URL_redirection#HTTP_status_codes_3xx

There's a race condition on Mac where stopping the httplistener and then starting a new one, the OS may not have cleaned up the actual underlying http service so the next start fails as the reservation still exists. In anycase, you should rebase against upstream/master as this script already starts and stops httplistener for all tests.

Seems like we should test multiple redirects to make sure it's stripped off the first one and subsequent redirects get handled correctly (3 may be sufficient)

Should also have a test case where -PreserveAuthorizationOnRedirect is specified, but no Authorization header was supplied

_cancelToken.Cancel(); [](start = 15, length = 23)

I am a little confused by this logic. It looks like an asynchronous call is being cancelled here. Is there a possibility of a race condition? Can you add a comment indicating what is being cancelled?

You need to make the corresponding doc change in PowerShell/PowerShell-Docs and mention it here to link the two

It's in progress. Will submit a PR shortly and update the description to reference it.

Also, do we need this capability with invoke-restmethod?

isn't 308 also valid?

Since you have this specific checks it seems you should have TestCases that go through the different status codes

308 isn't currently supported in CoreCLR - HttpStatusCode doesn't have a define for it so there's no way for the code to see it.

Refactored all tests to cover each redirect status code.

why isn't $response.content | convertfrom-json sufficient? this code seems unnecessary

Because I just want the json part of the raw content. The rest is extraneous for my purposes and it is also not valid json.
NOTE: $response.Content is a byte array.

Looking at your usage below, you pass in the output of Invoke-WebRequest to this helper. This helper isn't necessary. Line 189 below can just be:

$result.Content = $result.Output.Content | ConvertFrom-Json

This is assuming the output you create in httplistener is json (and if not, perhaps it should be).

The listener does add the output as json and I've even tried setting ContentType and ContentEncoding with no change in behavior. It appears it may be an issue on the client side. For now, I'll update the function to detect byte[] versus string and convert the byte[] to string before calling ConvertFrom-Json so it will work with arbitrary servers.

It would be better to have this as a -testcase where one of the parameters is $method and you can then pass Get and Post

This is only a choice between GET and POST. No other methods are being used.

lines 182 and 186 below only differ by use of -Method. You should either change $UsePost to $Method or use splatting so there's only a single Invoke-WebRequest line.

I'll update the function to use a ValidateSet and default to GET. I don't want to accept arbitrary methods since I'm explicitly testing the POST->GET logic on redirects.

Is there a specific reason you want to add a new Describe rather than having your tests part of the existing Invoke-WebRequest tests?

I placed the tests in a new describe so I can run them in isolation while developing them. it turns out appveyor doesn't allow extra tag so I'll be removing it shortly.

should validate the value as well

Will fix.

validate the value

Will fix

Actually, I can't fix it without fixing ConvertTo-Json. $request.Headers is a WebHeaderCollection which is a NamedValueCollection (ICollection). ConvertTo-Json (and Select-Object for that matter) treat it as a collection and would have to special case the type.

why not validate all the headers are there except authorization?

I don't know explicitly what other headers 'should' be there so I don't have a source of truth to compare against what is echoed in the response. I know the authorization header should or should not be in a specific echoed response so I can verify it.

lines 182 and 186 below only differ by use of -Method. You should either change $UsePost to $Method or use splatting so there's only a single Invoke-WebRequest line.

Looking at your usage below, you pass in the output of Invoke-WebRequest to this helper. This helper isn't necessary. Line 189 below can just be:

$result.Content = $result.Output.Content | ConvertFrom-Json

This is assuming the output you create in httplistener is json (and if not, perhaps it should be).

Not entirely sure about the Pester syntax, but do you have to have $redirectedMethod here since you don't use it? Or does Pester complain?

$response.Content is a byte array, not string ($result.Output.Content is this byte array). $response.RawContent has a few lines of text inserted by the underlying API followed by the json form of the original request; inserted by the httplistener. The purpose of the function is to extract the json from the RawContent and return it explicitly since it is needed to verify the incoming request.

I'll see if I can drop the unused $redirectedMethod params.

I'm remove the unused parameters.

I thought you were going to put these tests into an existing Describe?

Also, do we need this capability with invoke-restmethod?

We try to avoid == with string comparisons and instead use string.Equals(a,b, StringComparison.Ordinal) or the appropriate comparison - this makes it clearer that the author considered culture and case when writing the code.

Fixed

reqeusts [](start = 68, length = 8)

Spelling.

Fixed

false [](start = 76, length = 5)

Just a suggestion - we like to use named parameters when using bool constants - it makes it much easier to understand the code, so this could be GetResult(uri, stripAuthorization: false).

Updated.