Skip to content

Conversation

@ymesika
Copy link
Contributor

@ymesika ymesika commented May 28, 2017

  1. Created a shim.h header file that holds the openssl includes. @shmuelk suggested this approach as it is platform-independent and does not use absolute paths to the included openssl header files.
  2. Added two inline wrapper functions for future ALPN support that only being invoked if the OpenSSL version is 1.0.2 and above.

@ymesika ymesika requested a review from billabt June 2, 2017 08:07
Copy link
Collaborator

@billabt billabt left a comment

Choose a reason for hiding this comment

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

The biggest question I have is how this was tested? A lot of other Blue frameworks are dependent on this and I can't afford to break them with merge unless I know how it was tested. Thanks.

@ymesika
Copy link
Contributor Author

ymesika commented Jun 5, 2017

We tested it by running the Kitura-Net tests which also test SSL connections.
It was tested both on Linux and Mac (although OpenSSL is not being fetched on Mac).

We've forked BlueSSLService and made only one change to it - depend on this ymesika/OpenSSL.
We then cloned Kitura-Net, updated it to depend on the forked BlueSSLService and ran the tests.
No failures.

@billabt
Copy link
Collaborator

billabt commented Jun 5, 2017

If you test it using BlueCryptor as well (it uses the OpenSSL modulemap), I think we'll be all set and I'll do the merge. Thanks.

@ymesika
Copy link
Contributor Author

ymesika commented Jun 6, 2017

Thanks for mentioning BlueCryptor.
I cloned it on a Linux machine, pointed to the updated OpenSSL fork and ran the tests.
No failures.

@billabt billabt merged commit 5d9a672 into Kitura:master Jun 6, 2017
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.

2 participants