Skip to content

Fixes HTTP{S} auth when using wget #2764

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

Closed
wants to merge 1 commit into from

Conversation

eamsden
Copy link

@eamsden eamsden commented Aug 14, 2015

wget allows usernames and passwords to be encoded in URLs, and supports HTTPS. Thus, using http-transport: wget in the cabal configuration should permit the use of password-protected HTTPS hackage repositories. But because the arguments to wget are constructed using the Show instance for Network.URI.URI, password information in the repository URL is replaced with ..., resulting in authentication failures. This PR corrects that by using the Network.URI.uriToString function to construct the URL argument to wget.

It fixes issue #2763 and should be considered to solve issue #2761.

@23Skidoo
Copy link
Member

Do other transports (curl, PowerShell) also need to be updated?

@eamsden
Copy link
Author

eamsden commented Aug 17, 2015

@23Skidoo I have no idea about PowerShell. I don't believe curl takes auth information in the URL. It might be helpful as a separate PR to pull that out of the URI record and pass it to curl with the proper flags.

@DemiMarie
Copy link

One catch: one MUST NOT pass the password on the wget command line, because this is world-readable on many systems. The password must be passed over stdin.

@dcoutts
Copy link
Contributor

dcoutts commented Nov 2, 2015

So what's the conclusion here?

@23Skidoo
Copy link
Member

23Skidoo commented Nov 2, 2015

As @drbo says, passing the password on the command line is problematic from the security standpoint and we should use wget -i - instead.

@dcoutts
Copy link
Contributor

dcoutts commented Nov 2, 2015

@eamsden or @drbo would either of you be prepared to prepare a patch to do this?

23Skidoo referenced this pull request in bennofs/cabal Nov 3, 2015
This patch was tested with curl, wget and plain-http, but it's still untested on powershell.
The wget implementation will only work if the available wget supports the `--method` switch.
@23Skidoo
Copy link
Member

23Skidoo commented Nov 5, 2015

Superseded by #2890.

@23Skidoo 23Skidoo closed this Nov 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants