Extend TLS Option #233
Closed
Extend TLS Option #233
Conversation
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
sbSteveK
reviewed
Mar 25, 2024
sbSteveK
reviewed
Mar 25, 2024
sbSteveK
approved these changes
Mar 25, 2024
waahm7
reviewed
Mar 25, 2024
waahm7
reviewed
Mar 28, 2024
waahm7
approved these changes
Apr 1, 2024
| /// Initializes TlsContextOptions for mutual TLS (mTLS), with client certificate and private key. These are in memory | ||
| /// buffers. These buffers must be in the PEM format. | ||
| /// | ||
| /// NOTE: This is unsupported on iOS. |
Contributor
There was a problem hiding this comment.
debatable: You can use the @available(iOS, unavailable) declaration so that it fails at compile time. However, that will probably require consumers to do an if else to call this API.
Contributor
Author
There was a problem hiding this comment.
Thinking about it, it's better to set restrictions early on instead of adding them later. I added the available here for the path and data API.
@available(iOS, unavailable)
@available(tvOS, unavailable)
@available(watchOS, unavailable)
public static func makeMTLS(
certificateData: String,
privateKeyData: String) throws -> TLSContextOptions {
try TLSContextOptions(certificateData: certificateData, privateKeyData: privateKeyData)
}
For the pkcs12 one, it seems that @available only works for Apple platforms, we dont have a way to disable the API for linux/windows. I will leave it there for now.
| path: String, | ||
| /// Initializes TlsContextOptions for mutual TLS (mTLS), with client certificate and private key in the PKCS#12 format. | ||
| /// | ||
| /// NOTE: This only works on Apple devices. |
Contributor
There was a problem hiding this comment.
@available directive?
Pros:
- Compile time safety instead of runtime error
Cons:
- Consumers of this API will have to do IF/ELSE at compile time, although they should probably do that anyhow if it is a cross-platform application.
Merged
Contributor
Author
|
The changes are brought in new PR: #246 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.