@CarloToso This change can be very sensitive. We need detailed description of the scenario(s) you are trying to fix.

The status codes 300,301,302,303,307,308 are redirect codes, and should have similar behaviour.
Before this PR when setting -MaximumRedirection 0 -SkipHttpErrorCheck in the WebCmdlets only 301 and 302 (HttpStatusCode.Moved, HttpStatusCode.MovedPermanently and HttpStatusCode.Found) threw this nonterminating error The maximum redirection count has been exceeded. To increase the number of redirections allowed, supply a higher value to the -MaximumRedirection parameter.
You can check it with this code
300,301,302,303,307,308 | ForEach-Object {Invoke-WebRequest http://mockbin.org/redirect/$($_)?to=https://mockbin.org/status/200 -MaximumRedirection 0 -SkipHttpErrorCheck}

Another option could be completely removing this non terminating error. So we have the same behaviour for -MaximumRedirection >= 0

@CarloToso While I agree that 308 is redirect code and should be handled in some way I have a concern that the PR changes not one scenario but same ones.

Supporting the HTTP client is a difficult task. We usually compare the behavior with wget. If you can create tests that work the same way for both clients, that would be convincing for reviewers.

@iSazonov I wrote this small test to compare curl, wget and invoke-webrequest

for ($maxredirs = 0; $maxredirs -le 2; $maxredirs++)
{
	Write-Host "MaximumRedirection == $maxredirs" -BackgroundColor Red
	Write-Host "-------------------Invoke-WebRequest-------------------"
	300,301,302,303,307,308 | ForEach-Object {Invoke-WebRequest "http://mockbin.org/redirect/$($_)?to=https://mockbin.org/redirect/302?to=https://mockbin.org/status/200" -Method HEAD -MaximumRedirection $maxredirs -SkipHttpErrorCheck}
	Write-Host "-------------------curl-------------------"
	300,301,302,303,307,308 | ForEach-Object {$x = curl.exe -LI --no-progress-meter "http://mockbin.org/redirect/$($_)?to=https://mockbin.org/redirect/302?to=https://mockbin.org/status/200" --max-redirs $maxredirs; ($x -join"`n").split("`n`n")[-1]}
	Write-Host "-------------------wget-------------------"
	300,301,302,303,307,308 | ForEach-Object {$x = .\wget.exe http://mockbin.org/redirect/$($_)?to=https://mockbin.org/redirect/302?to=https://mockbin.org/status/200 --max-redirect $maxredirs --spider --server-response 2>&1; ($x[0..($x.length-2)] -join"`n").split("Spider mode enabled. Check if remote file exists.")[-1]}
}

flags explained

curl -> -I == HEAD, -L == follow redirect
wget -> --spider == HEAD

It looks like they have similar behaviours:

$maxredirs = 0
Iwr ->error on 301 and 302 maximum redirections exceded
curl and wget -> always error maximum redirections exceded

$maxredirs = 1
Iwr ->no error
curl and wget -> always error maximum redirections exceded

I wasn't able to parse the maximum redirections exceded error correctly in wget, to visualize it use:

.\wget.exe http://mockbin.org/redirect/303?to=https://mockbin.org/redirect/302?to=https://mockbin.org/status/200 --max-redirect 0 --spider --server-response

I propose we remove the non terminating error (lines 1600-1614) from the webcmdlets, because it only appears for 2 status codes, and only for -MaximumRedirection 0

Maybe it might be useful in some edge cases, if that is the case adding the other redirect codes would be the right thing to do

@iSazonov I wrote this small test to compare curl, wget and invoke-webrequest

Thanks for your investigations! Can you share outputs (or screenshots)? For 308 particularly.

Invoke-WebRequest ($maxredirs -> 0, 1, max)

Curl ($maxredirs -> 0, 1, max)

Wget ($maxredirs -> 0, 1, max)

(different code to display the redirections exceeded error)

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

This PR has 44 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Label      : Extra Small
Size       : +18 -26
Percentile : 17.6%

Total files changed: 1

Change summary by file extension:
.cs : +18 -26

Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

Is something blocking this PR?

Is something blocking this PR?

Waiting review from @PaulHigin.