-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Android certificate paths to CURL_CA_PATH #5283
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
base: main
Are you sure you want to change the base?
Conversation
| isDirectory: &isDirectory) | ||
| && isDirectory.boolValue { | ||
| path.withCString { pathPtr in | ||
| try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionCAPATH, UnsafeMutablePointer(mutating: pathPtr)).asError() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is try! safe here? Why is the result of .asError being ignored? In particular, it seems that we are losing failure information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's just the convention I observed from every other call to CFURLSesson_easy* functions in this file. They all follow the same pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should understand the error handling here, else we can do something like:
do {
try CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionCAPATH, UnsafeMutablePointer(mutating: pathPatr)).asError()
} catch let error {
fatalError(error.message)
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This exact pattern is followed 50 other times in this file, and is the documented way to handle errors:
swift-corelibs-foundation/Sources/FoundationNetworking/URLSession/URLSession.swift
Lines 109 to 125 in 8c60d6b
| /// ## Error Handling | |
| /// | |
| /// Most libcurl functions either return a `CURLcode` or `CURLMcode` which | |
| /// are represented in Swift as `CFURLSessionEasyCode` and | |
| /// `CFURLSessionMultiCode` respectively. We turn these functions into throwing | |
| /// functions by appending `.asError()` onto their calls. This turns the error | |
| /// code into `Void` but throws the error if it's not `.OK` / zero. | |
| /// | |
| /// This is combined with `try!` is almost all places, because such an error | |
| /// indicates a programming error. Hence the pattern used in this code is | |
| /// | |
| /// ``` | |
| /// try! someFunction().asError() | |
| /// ``` | |
| /// | |
| /// where `someFunction()` is a function that returns a `CFURLSessionEasyCode`. | |
| /// |
On Android devices, there is not single aggregate
.cerfile we can set withCFURLSessionInfoCAINFO, but rather a system managed folder that needs to be specified withCFURLSessionOptionCAPATH. This is similar to how swift-nio handles it (apple/swift-nio-ssl#453).This is a better solution than #5163, so I'll close that in favor of this.
It only affects Android codepaths, so this change should be low-risk.