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

Add ca_bundle_file and ca_certs_dir config options #39

Merged
merged 2 commits into from
Sep 16, 2022

Conversation

funilrys
Copy link
Contributor

This patch (potentially) fixes #14.

Indeed, this patch introduces the "ca_bundle_file" and "ca_certs_dir" configuration keys.

From now on, "ca_bundle_file" will be plumbed into CURLOPT_CAINFO and "ca_certs_dir" into CURLOPT_CAPATH. However, when the value of "ca_certs_dir" is empty, we fallback to "ca_bundle_file" if it's not empty either.

This patch (potentially) fixes tarickb#14.

Indeed, this patch introduces the "ca_bundle_file" and "ca_certs_dir"
configuration keys.

From now on, "ca_bundle_file" will be plumbed into CURLOPT_CAINFO and
"ca_certs_dir" into CURLOPT_CAPATH. However, when the value of
"ca_certs_dir" is empty, we fallback to "ca_bundle_file" if it's not
emtpy either.
@funilrys
Copy link
Contributor Author

@tarickb I find it hard to find the right place to document this in the README file ... Can you take over that part?

Copy link
Owner

@tarickb tarickb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on! Yes, I can certainly update the README to describe use of these options.

src/http.cc Outdated Show resolved Hide resolved
src/http.cc Show resolved Hide resolved
@funilrys
Copy link
Contributor Author

@tarickb I took over the requested changes. Please review.

@funilrys funilrys requested a review from tarickb September 16, 2022 18:11
Copy link
Owner

@tarickb tarickb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@tarickb tarickb merged commit 9047a97 into tarickb:master Sep 16, 2022
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.

Config file should specify location of CA certs
2 participants