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

Use curl for download on windows when proxy auth is required #1434

Merged
merged 2 commits into from
Jul 5, 2024

Conversation

sylvlecl
Copy link
Contributor

@sylvlecl sylvlecl commented Jun 14, 2024

This is a proposition to solve the issue reported in this microsoft/vcpkg#38827.

This replaces the first proposed implementation in #1410, following suggestions of @BillyONeal : curl is now used instead of winhttp for download as soon as we have credentials in the URL provided for HTTP proxy (through the HTTPS_PROXY environment variable).

Status:

  • Quick implementation to check that it is a better way to go than previous implementation.
    Just checking for "@" in the proxy URL, which might need some refinement ? not obvious though.
  • Successfully tested with squid proxy in test env
  • Question: should we use curl download as soon as a proxy url is defined ? that would eliminate some code from the winhttp download implementation.

Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
@sylvlecl sylvlecl marked this pull request as ready for review June 15, 2024 12:25
@BillyONeal
Copy link
Member

  • Question: should we use curl download as soon as a proxy url is defined ? that would eliminate some code from the winhttp download implementation.

I don't think so, there are environments like Server Core that don't come with curl.exe

@BillyONeal BillyONeal merged commit 968b4f4 into microsoft:main Jul 5, 2024
5 checks passed
@BillyONeal
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants