Support for OpenSSL 1.1.X #61
Conversation
|
@djones6 @ianpartridge @pushkarnk Please review. |
Package.swift
Outdated
| @@ -29,7 +29,7 @@ var targetDependencies: [Target.Dependency] = [.byName(name: "Socket")] | |||
|
|
|||
| #if os(Linux) | |||
|
|
|||
| packageDependencies.append(.package(url: "https://github.com/IBM-Swift/OpenSSL.git", from: "2.0.0")) | |||
| packageDependencies.append(.package(url: "https://github.com/nethraravindran/OpenSSL.git", .branch("openSSL"))) | |||
There was a problem hiding this comment.
This is a temporary change until we a release for OpenSSL https://github.com/IBM-Swift/OpenSSL
There was a problem hiding this comment.
I'd hesitate to merge this until the change to OpenSSL is merged as well and passing all the necessary CI checks.
nethraravindran
force-pushed
the
openSSL
branch
2 times, most recently
from
a2ee480 to
e85d387
Compare
There was a problem hiding this comment.
Overall the changes look good. Although, I'd prefer to see the obsolete code deleted rather than commented out. It'd be nice if you could do that prior to the merge. Also, would like to see the new extensions that were added documented properly.
That and get it to pass CI. 👍?
nethraravindran
force-pushed
the
openSSL
branch
from
e85d387 to
861eb36
Compare
nethraravindran
force-pushed
the
openSSL
branch
from
861eb36 to
6b3884b
Compare
.travis.yml
Outdated
| @@ -17,6 +17,10 @@ matrix: | |||
| - os: linux | |||
| dist: trusty | |||
| sudo: required | |||
| - os: linux | |||
There was a problem hiding this comment.
This block you added for Swift 4.2 is not needed. The block right before this will default to the contents of .swift-version. BlueSSLService is already building on Swift 4.2.
There was a problem hiding this comment.
@billabt Thank you!
nethraravindran
force-pushed
the
openSSL
branch
from
e0a63b3 to
e9a299c
Compare
|
@nethraravindran looks like the build is failing on Linux 4.0.3 and Linux 4.1.3 |
|
@nethraravindran: Looks like everything is going to be fine now. However, can you document the extensions you added for Linux before I merge it? Thanks. |
|
Hi @billabt we have a little more work to do on this before it's ready, @nethraravindran will update soon :) |
nethraravindran
force-pushed
the
openSSL
branch
from
3c39a40 to
753d7a1
Compare
|
I have added support for Swift 4.0 and 4.1 @billabt @ianpartridge Please review. |
nethraravindran
force-pushed
the
openSSL
branch
from
753d7a1 to
3c3d12b
Compare
|
|
||
|
|
||
|
|
||
| #if swift(>=4.2) |
There was a problem hiding this comment.
If I am understanding correctly this means that on Swift 4.2 the cSSL property will be an OpaquePointer? but on earlier versions of Swift it will be UnsafeMutablePointer<SSL>?? So we would have different public API depending on the Swift version? That seems like a problem...
Could it be OpaquePointer? on all Swift versions?
There was a problem hiding this comment.
@ianpartridge This property as well as the method and context properties are only public so that they can be used by the verifyCallback if the developer desires. The use of this callback implies and expects that the developer have an in depth knowledge of how BlueSSLService is working. NONE of these properties are exposed by any public functions although they're used by a few private functions. The public API, IMHO, remains the same. The risk is very, very low and only for developers who are using the verifyCallback and those folks should be aware since the contents of the callback will in all probability be platform specific.
| // | ||
| // Hopefully, in time, these extensions can be removed. | ||
|
|
||
| extension UnsafePointer { |
There was a problem hiding this comment.
Please document these extensions...
There was a problem hiding this comment.
Let's leave it as is... After reading the above, it's pretty obvious what's going on.
| // | ||
| // Hopefully, in time, these extensions can be removed. | ||
|
|
||
| extension UnsafePointer { |
There was a problem hiding this comment.
Let's leave it as is... After reading the above, it's pretty obvious what's going on.
Description
Currently, BlueSSLService doesn't add support for openssl 1.1.X. This PR aims to add support for both openssl versions 1.0.X and 1.1.X
Resolves : Kitura/Kitura#1341
How Has This Been Tested?
Tested on both Ubuntu 16.04 and 18.04 with openssl 1.0.x and 1.1.x
Checklist: