Skip to content
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.

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

hope to handle 308 statecode in rbenv-install.ps1/function dl() #18

Closed
Jump-Wang-111 opened this issue Apr 29, 2023 · 5 comments
Closed
Assignees

Comments

@Jump-Wang-111
Copy link

Jump-Wang-111 commented Apr 29, 2023

I am installing ruby for the first time. During the installation process, It need to visit https://s3.jcloud.sjtu.edu.cn/899a892efef34b1b944a19981040f55b-oss01/github-release/oneclick/rubyinstaller2/releases/download/RubyInstaller-3.2.2 -1/rubyinstaller-devkit-3.2.2-1-x64.exe to download, but the powershell script fails with 308, I use the browser to download from this URL succcessfully, because 308 returns a redirected address.

I think a good way to solve this is to add 308 in $handledCodes just like this:

$handledCodes = @(
            [System.Net.HttpStatusCode]::MovedPermanently,  # HTTP 301
            [System.Net.HttpStatusCode]::Found,             # HTTP 302
            [System.Net.HttpStatusCode]::SeeOther,          # HTTP 303
            [System.Net.HttpStatusCode]::TemporaryRedirect  # HTTP 307
	    [System.Net.HttpStatusCode]::PermanentRedirect	# HTTP 308
        )

however it doesn't work. Finally I change the condition of statecode to solve the problem like:

if ($handledCodes -notcontains $redirectRes.StatusCode -and $redirectRes.StatusCode -ne 308 ) {
            throw $exc
        }

I hope the support to 308 can be added and I still feel confused about why change $handledCodes don't work?

@ccmywish
Copy link
Collaborator

Hi @Jump-Wang-111

Thanks for your feedback and your patience in trying to solve it by yourself. 👍 In effect, the issue is reported several times, see #7

I speculate you're using PowerShell v5.x series. Please update to pwsh 7.x series as a solution. You don't need to change the source code.

I'll dig into the problem in detail later.

@Jump-Wang-111
Copy link
Author

ohhhh, I did not see that issue before. My PowerShell is indeed version 5.1. Thanks!

@ccmywish
Copy link
Collaborator

ccmywish commented May 19, 2023

@Jump-Wang-111

  • PowerShell v5.0 was released in 2016
  • PowerShell v5.1 was released in 2017

However, 308 status code is only published in 2015 (very young), many clients can't recognize it. I think PowerShell v5 series are so.

I'll close this issue. If you have any doubts, please re-open it. Thanks.

@ccmywish
Copy link
Collaborator

I've changed my mind, because I think it's easy to fix.

As I've said above, it's because PowerShell v5.x can't recognize [System.Net.HttpStatusCode]::PermanentRedirect, but we can just write the 308 directly to fix it.

@ccmywish ccmywish reopened this May 20, 2023
@ccmywish ccmywish self-assigned this May 20, 2023
@ccmywish
Copy link
Collaborator

ccmywish commented May 20, 2023

@Jump-Wang-111

I've patched the code.

I've tested it on my machine and it works. Please check it yourself If you have time. 🤝

Thanks for reporting it! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants