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

Consider adding flag to fall back to natural libcurl proxy behavior #1583

Closed
rwolfson opened this issue Mar 4, 2021 · 4 comments
Closed
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue

Comments

@rwolfson
Copy link

rwolfson commented Mar 4, 2021

Hi AWS C++ SDK folks,
I recently hit similar behavior regarding libcurl environment variables as reported in #1049 . Although I understand the desire for certain customers not wanting this automatic behavior, the default behavior is very helpful in many other environments. Specifically, it allows developers to create more deployment-agnostic code that doesn't need to implement boilerplate proxy detection logic or flags.

Would it be possible to consider a CMake flag that skips the block that sets CURLOPT_PROXY to ""?
https://github.com/aws/aws-sdk-cpp/blob/1.8.155/aws-cpp-sdk-core/source/http/curl/CurlHttpClient.cpp#L613-L616

That would hopefully avoid breaking customers that rely on the default behavior, but provide an opportunity for customers that want the default libcurl behavior to have their applications function similarly to other libcurl applications.

An alternative implementation could be an automaticProxy flag in ClientConfiguration that skips that specific CURLOPT_PROXY option from being set. This is a little bit less transparent than the preprocessor option since it would require application code changes to utilize.

Thank you for your consideration. I'm happy to submit a PR if this seems like a worthwhile improvement.

@rwolfson rwolfson added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 4, 2021
@sahil1105
Copy link

I agree, using environment variables by default, and providing an option to ignore them for the special case would be ideal.

@jmklix
Copy link
Member

jmklix commented Mar 25, 2022

This behavior is something that we want to add, but I don't have a timeline for when it will get added

@jmklix jmklix removed the needs-triage This issue or PR still needs to be triaged. label Mar 25, 2022
@jmklix jmklix added the p3 This is a minor priority issue label Mar 8, 2023
@jmklix
Copy link
Member

jmklix commented Sep 22, 2023

This was added with this PR. Please update to the latest version of this sdk and let us know if you have any other features that could improve this sdk.

@jmklix jmklix closed this as completed Sep 22, 2023
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue
Projects
None yet
Development

No branches or pull requests

3 participants