-
Notifications
You must be signed in to change notification settings - Fork 29
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
feat: add wrapper for checksums + unit tests #642
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1be11c8
add wrapper for checksums / hash functions + unit tests
dayaffe 90c0d1e
move protocol lower
dayaffe 252de0a
Merge branch 'main' into day/checksums-hash-functions-wrapper
dayaffe dcf2846
add initialize to try to fix failing linux CI
dayaffe 3b9e3d6
move initialize to setUp
dayaffe df1ad90
add HashResult enum with HexStringable + hardcoded test expectations
dayaffe cc134ad
add better error throwing
dayaffe c892662
make computeHash return a non-nullable result + clean up
dayaffe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0. | ||
*/ | ||
|
||
import AwsCommonRuntimeKit | ||
|
||
enum HashResult { | ||
case data(Data) | ||
case integer(UInt32) | ||
} | ||
|
||
enum HashError: Error { | ||
case invalidInput | ||
case hashingFailed(reason: String) | ||
} | ||
|
||
enum HashFunction { | ||
case crc32, crc32c, sha1, sha256, md5 | ||
|
||
static func from(string: String) -> HashFunction? { | ||
switch string.lowercased() { | ||
case "crc32": return .crc32 | ||
case "crc32c": return .crc32c | ||
case "sha1": return .sha1 | ||
case "sha256": return .sha256 | ||
case "md5": return .md5 // md5 is not a valid flexible checksum algorithm | ||
default: return nil | ||
} | ||
} | ||
|
||
var isSupported: Bool { | ||
switch self { | ||
case .crc32, .crc32c, .sha256, .sha1: | ||
return true | ||
default: | ||
return false | ||
} | ||
} | ||
|
||
func computeHash(of data: Data) throws -> HashResult { | ||
switch self { | ||
case .crc32: | ||
return .integer(data.computeCRC32()) | ||
case .crc32c: | ||
return .integer(data.computeCRC32C()) | ||
case .sha1: | ||
do { | ||
let hashed = try data.computeSHA1() | ||
return .data(hashed) | ||
} catch { | ||
throw HashError.hashingFailed(reason: "Error computing SHA1: \(error)") | ||
} | ||
case .sha256: | ||
do { | ||
let hashed = try data.computeSHA256() | ||
return .data(hashed) | ||
} catch { | ||
throw HashError.hashingFailed(reason: "Error computing SHA256: \(error)") | ||
} | ||
case .md5: | ||
do { | ||
let hashed = try data.computeMD5() | ||
return .data(hashed) | ||
} catch { | ||
throw HashError.hashingFailed(reason: "Error computing MD5: \(error)") | ||
} | ||
} | ||
} | ||
} | ||
|
||
extension HashResult { | ||
|
||
// Convert a HashResult to a hexadecimal String | ||
func toHexString() -> String { | ||
switch self { | ||
case .data(let data): | ||
return data.map { String(format: "%02x", $0) }.joined() | ||
case .integer(let integer): | ||
return String(format: "%08x", integer) | ||
} | ||
} | ||
} |
163 changes: 163 additions & 0 deletions
163
Tests/ClientRuntimeTests/NetworkingTests/HashFunctionTests.swift
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
// | ||
// Copyright Amazon.com Inc. or its affiliates. | ||
// All Rights Reserved. | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import XCTest | ||
@testable import ClientRuntime | ||
import AwsCommonRuntimeKit | ||
|
||
class HashFunctionTests: XCTestCase { | ||
|
||
override func setUp() { | ||
// Initialize function needs to be called before interacting with CRT | ||
CommonRuntimeKit.initialize() | ||
} | ||
|
||
func testCRC32NonUTF8Bytes() { | ||
guard let hashFunction = HashFunction.from(string: "crc32") else { | ||
XCTFail("CRC32 not found") | ||
return | ||
} | ||
|
||
// Create test data | ||
let testBytes: [UInt8] = [0xFF, 0xFE, 0xFD, 0xFC] // Bytes not valid in UTF-8 | ||
let testData = Data(testBytes) | ||
|
||
let computedHash = try? hashFunction.computeHash(of: testData) | ||
guard case let .integer(result)? = computedHash else { | ||
XCTFail("CRC32 computed hash is not an integer or is nil") | ||
return | ||
} | ||
let expected = UInt32(1426237168) | ||
XCTAssertEqual(result, expected, "CRC32 hash does not match expected value") | ||
} | ||
|
||
func testCRC32CNonUTF8Bytes() { | ||
guard let hashFunction = HashFunction.from(string: "crc32c") else { | ||
XCTFail("CRC32C not found") | ||
return | ||
} | ||
|
||
// Create test data | ||
let testBytes: [UInt8] = [0xFF, 0xFE, 0xFD, 0xFC] // Bytes not valid in UTF-8 | ||
let testData = Data(testBytes) | ||
|
||
let computedHash = try? hashFunction.computeHash(of: testData) | ||
guard case let .integer(result)? = computedHash else { | ||
XCTFail("CRC32C computed hash is not an integer or is nil") | ||
return | ||
} | ||
let expected = UInt32(1856745115) | ||
XCTAssertEqual(result, expected, "CRC32C hash does not match expected value") | ||
} | ||
|
||
func testSHA1NonUTF8Bytes() { | ||
guard let hashFunction = HashFunction.from(string: "sha1") else { | ||
XCTFail("SHA1 not found") | ||
return | ||
} | ||
|
||
// Create test data | ||
let testBytes: [UInt8] = [0xFF, 0xFE, 0xFD, 0xFC] // Bytes not valid in UTF-8 | ||
let testData = Data(testBytes) | ||
|
||
let computedHash = try? hashFunction.computeHash(of: testData) | ||
guard case let .data(result)? = computedHash else { | ||
XCTFail("SHA1 computed hash is not a data type or is nil") | ||
return | ||
} | ||
let expected = "ADfJtWg8Do2MpnFNsvFRmyMuEOI=" | ||
XCTAssertEqual(result.base64EncodedString(), expected, "SHA1 hash does not match expected value") | ||
} | ||
|
||
func testSHA256NonUTF8Bytes() { | ||
guard let hashFunction = HashFunction.from(string: "sha256") else { | ||
XCTFail("SHA256 not found") | ||
return | ||
} | ||
|
||
// Create test data | ||
let testBytes: [UInt8] = [0xFF, 0xFE, 0xFD, 0xFC] // Bytes not valid in UTF-8 | ||
let testData = Data(testBytes) | ||
|
||
let computedHash = try? hashFunction.computeHash(of: testData) | ||
guard case let .data(result)? = computedHash else { | ||
XCTFail("SHA256 computed hash is not a data type or is nil") | ||
return | ||
} | ||
let expected = "jCosV0rEcc6HWQwT8O/bQr0ssZuxhJM3nUW/zJBgtlc=" | ||
XCTAssertEqual(result.base64EncodedString(), expected, "SHA256 hash does not match expected value") | ||
} | ||
|
||
func testMD5NonUTF8Bytes() { | ||
guard let hashFunction = HashFunction.from(string: "md5") else { | ||
XCTFail("MD5 not found") | ||
return | ||
} | ||
|
||
// Create test data | ||
let testBytes: [UInt8] = [0xFF, 0xFE, 0xFD, 0xFC] // Bytes not valid in UTF-8 | ||
let testData = Data(testBytes) | ||
|
||
let computedHash = try? hashFunction.computeHash(of: testData) | ||
guard case let .data(result)? = computedHash else { | ||
XCTFail("MD5 computed hash is not a data type or is nil") | ||
return | ||
} | ||
let expected = "ilWq/WLcPzYHQ8fAzwCCLg==" | ||
XCTAssertEqual(result.base64EncodedString(), expected, "MD5 hash does not match expected value") | ||
} | ||
|
||
func testInvalidHashFunction() { | ||
let invalidHashFunction = HashFunction.from(string: "invalid") | ||
XCTAssertNil(invalidHashFunction, "Invalid hash function should return nil") | ||
} | ||
|
||
func testHashFunctionToHexString() { | ||
let testData = Data("Hello, world!".utf8) | ||
|
||
// CRC32 | ||
if let crc32Function = HashFunction.from(string: "crc32"), | ||
let crc32Result = try? crc32Function.computeHash(of: testData).toHexString() { | ||
XCTAssertEqual(crc32Result, "ebe6c6e6", "CRC32 hexadecimal representation does not match expected value") | ||
} else { | ||
XCTFail("CRC32 hash function not found or computation failed") | ||
} | ||
|
||
// CRC32C | ||
if let crc32cFunction = HashFunction.from(string: "crc32c"), | ||
let crc32cResult = try? crc32cFunction.computeHash(of: testData).toHexString() { | ||
XCTAssertEqual(crc32cResult, "c8a106e5", "CRC32C hexadecimal representation does not match expected value") | ||
} else { | ||
XCTFail("CRC32C hash function not found or computation failed") | ||
} | ||
|
||
// SHA1 | ||
if let sha1Function = HashFunction.from(string: "sha1"), | ||
let sha1Result = try? sha1Function.computeHash(of: testData).toHexString() { | ||
XCTAssertEqual(sha1Result, "943a702d06f34599aee1f8da8ef9f7296031d699", "SHA1 hexadecimal representation does not match expected value") | ||
} else { | ||
XCTFail("SHA1 hash function not found or computation failed") | ||
} | ||
|
||
// SHA256 | ||
if let sha256Function = HashFunction.from(string: "sha256"), | ||
let sha256Result = try? sha256Function.computeHash(of: testData).toHexString() { | ||
XCTAssertEqual(sha256Result, "315f5bdb76d078c43b8ac0064e4a0164612b1fce77c869345bfc94c75894edd3", "SHA256 hexadecimal representation does not match expected value") | ||
} else { | ||
XCTFail("SHA256 hash function not found or computation failed") | ||
} | ||
|
||
// MD5 | ||
if let md5Function = HashFunction.from(string: "md5"), | ||
let md5Result = try? md5Function.computeHash(of: testData).toHexString() { | ||
XCTAssertEqual(md5Result, "6cd3556deb0da54bca060b4c39479839", "MD5 hexadecimal representation does not match expected value") | ||
} else { | ||
XCTFail("MD5 hash function not found or computation failed") | ||
} | ||
} | ||
|
||
} |
Oops, something went wrong.
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.
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.
Debatable: Feel free to ignore.
I find it better and more concise to use
let hashFunction = HashFunction.from(string: "crc32")!
instead of a guard in tests. The test will fail anyway if we don't find it. Same comment for all the guards,.?
, andtry?
in these tests.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.
removing
try?
causes error unhandled errors, the guard statements I prefer to keep in for precise error messaging should it ever failThere 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.
Okay, by the way, you can replace
try?
withtry!
and.?
with.!
to avoid causing any errors.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.
Generally agreed, I am less of a stickler about force-unwrap in tests, though it can be annoying for the test suite to crash when it could have just failed a test & moved on.
If the
!
is expected to fail under any circumstance, I'd prefer to throw & fail versus crash the test suite.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.
Good to know! For now I am going to continue with the guard so that it is abundantly clear what the issue is if we get a failure here