-
Notifications
You must be signed in to change notification settings - Fork 19
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
Updated to Blue Cryptor 1.0.0 #45
Conversation
@@ -40,12 +40,12 @@ class CookieCryptography { | |||
/// | |||
private let originalLength = 36 | |||
|
|||
init (secret: String) { | |||
init (secret: String) throws { |
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.
Isn't this a breaking change? All users of this API will have to do {} catch {}
it now?
I would rather not have to declare a new major version of this package, so I suggest that instead of marking this function as throws
we wrap the PBKDF.deriveKey()
calls in do {}
instead, and if they throw then we just fatalError()
(which is what Cryptor used to do anyway...)
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.
It is an internal initialize that is not part of the public API, so it shouldn't affect anyone other than ourselves. We avoid surfacing this exception in the constructor for Session by catching and logging it and then calling fatalError()
, which is what BlueCryptor previously did internally.
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.
Great, thanks for explaining. I missed that it's actually internal
even though the type is public
.
Travis is failing on Linux due to OpenSSL dependencies. In a nutshell, Session relies on Kitura which relies on Kitura-net which is relying on an outdated version of BlueSSLService, which is relying on an old version on OpenSSL. Moving BlueCryptor from Kitura-Net needs to use BlueSSLService 1.0.0+ to use OpenSSL 1.0.0 and fix the issue. |
Package.swift
Outdated
@@ -30,7 +30,7 @@ let package = Package( | |||
], | |||
dependencies: [ | |||
.package(url: "https://github.com/IBM-Swift/Kitura.git", from: "2.1.0"), | |||
.package(url: "https://github.com/IBM-Swift/BlueCryptor.git", .upToNextMinor(from: "0.8.0")), | |||
.package(url: "https://github.com/IBM-Swift/BlueCryptor.git", .upToNextMinor(from: "1.0.0")), |
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.
We should drop the .upToNextMinor
constraint here.
Also, as Kitura-net needs updating to BlueSocket / BlueSSLService 1.0.0 to be compatible with the updated BlueCryptor, you'll need to update the Kitura dependency to 2.3.0
(not yet tagged, but coming soon).
.travis.yml
Outdated
matrix: | ||
include: | ||
- os: linux | ||
dist: trusty | ||
sudo: required | ||
env: SWIFT_SNAPSHOT=4.1 |
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.
To be consistent with our other Travis files, we should remove the SWIFT_SNAPSHOT=4.1
which means we'll instead use the .swift-version
(which is now 4.1).
.travis.yml
Outdated
sudo: required | ||
env: SWIFT_SNAPSHOT=4.1 |
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.
ditto
@KyeMaloy97 Kitura-net |
This looks good now. It will need to be tagged with a new minor (due to the upgrade of BlueCryptor dependency). |
Codecov Report
@@ Coverage Diff @@
## master #45 +/- ##
==========================================
- Coverage 72.91% 70.54% -2.38%
==========================================
Files 6 6
Lines 240 258 +18
==========================================
+ Hits 175 182 +7
- Misses 65 76 +11
Continue to review full report at Codecov.
|
Updated to Blue Cryptor 1.0.0 and updated to Swift 4.1 by adding new Travis builds and by updating the .swift-version.
A few shuffles around of the code as the 1.0 release had some breaking changes coming from 0.8, mainly changing things to be inside do-catch blocks.
Passing tests and building with no warnings on my Mac on Swift 4.1 and Swift 4.0.3