Don't set CURL_NATIVE_CA if a cert path is provided#337
Open
freundTech wants to merge 1 commit into
Open
Conversation
Depending on the operating system if both a cert path is provided and CURL_NATIVE_CA is set, curl might first check against the provided cert path and fall back to the native ca if it can't verify the cert. However, when a user provides a custom cert path they want duckdb to only use that certificate store and not fall back to operating system certificates. This prevents `test/sql/httpfs/ca_cert_file.test` from breaking when curl is built with Apple SecTrust support. Related: duckdb#282
Author
|
The VCPKG maintainers decided to merge the version that always enables the curl build option, because they expect most users to want that behavior by default. This means that without this fix the duckdb-httpfs test suite will likely fail when built with the new vcpkg build manifest. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm working on exposing Apple SecTrust support in the
curlvcpkg package (microsoft/vcpkg#52150, microsoft/vcpkg#52158) to make it possible to solve #282. Depending on what the vcpkg maintainers decide this will be either an optional feature for duckdb-httpfs to enable or just enabled by default.Testing with this enabled surfaced a small bug in duckdb-httpfs: Settign
SET ca_cert_file = 'certificate_file';will still fall-back to the native CA store, if one is available, instead of erroring when the certificate can't be validated using the provided cert file, causingtest/sql/httpfs/ca_cert_file.testto fail.This commit fixes this behavior to prepare for adding SecTrust support and fixing #282 later.