Skip to content
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

Fix for Ubuntu 14.04 and OpenSSL 1.0.1 #17

Closed
wants to merge 12 commits into from
Closed

Conversation

KyeMaloy97
Copy link
Contributor

There is an issue where systems using an older version of OpenSSL are not able to build, due to a type mismatch. The PR links to a branch I have made of the OpenSSL module map which now has a wrapper around the method in question (EVP_DigestVerifyFinal) to stop Swift complaining due to a type mismatch.

When the OpenSSL branch is merged into master, I will update this PR to point to the release that contains the fix, rather than the branch. Opening this now so Travis can do some testing.

.swiftlint.yml Outdated
- trailing_newline
- force_cast
- function_body_length
- variable_name
- indentifier_name
Copy link
Contributor

Choose a reason for hiding this comment

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

Surely identifier_name?

@@ -467,7 +499,12 @@ class CryptorRSATests: XCTestCase {
// JWT also does base64url encoding, so make the proper replacements so its proper base64 encoding
sig = sig.replacingOccurrences(of: "-", with: "+")
sig = sig.replacingOccurrences(of: "_", with: "/")
let sigData = Data(base64Encoded: sig)!

guard let sigData: Data = Data(base64Encoded: sig) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to let the compiler infer the type I think.

@@ -445,11 +476,12 @@ class CryptorRSATests: XCTestCase {
-----END PUBLIC KEY-----
"""

let tokenPublicKey = try? CryptorRSA.createPublicKey(withPEM: certificatePEM)
let tokenPublicKey = try CryptorRSA.createPublicKey(withPEM: certificatePEM)
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to guard let this, XCTFail(); return in the else, and remove the XCTAssertNotNil() below.

@billabt
Copy link
Collaborator

billabt commented Jul 26, 2018

@KyeMaloy97: Are you still working on this? The CI checks are failing. Thanks.

@KyeMaloy97
Copy link
Contributor Author

@billabt Yea I am still working on it, I am working on removing force unwraps in the Test suite to get the Swift Linter to pass

@KyeMaloy97 KyeMaloy97 closed this Jul 30, 2018
@KyeMaloy97
Copy link
Contributor Author

Opening in two new PRs #20 and #19

@billabt
Copy link
Collaborator

billabt commented Jul 30, 2018

Cool, I’ll take a look either later today or first thing in the morning.

@djones6 djones6 deleted the issue.Fix1404 branch February 26, 2019 14:15
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.

None yet

3 participants