From 8b2484a413a5c9a216525dcf6858223addbf8362 Mon Sep 17 00:00:00 2001
From: Kristy Brambila <kristy@launchbadge.com>
Date: Tue, 24 Sep 2019 13:30:28 -0700
Subject: [PATCH] Improve hexDecode to not break on non-hex characters

---
 Sources/hedera/crypto/Hex.swift                | 14 ++++++++------
 .../crypto/ed25519/Ed25519PrivateKey.swift     | 18 +++++++-----------
 .../crypto/ed25519/Ed25519PublicKey.swift      | 15 ++++++++-------
 Tests/hederaTests/crypto/HexTests.swift        | 12 +++++++-----
 4 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/Sources/hedera/crypto/Hex.swift b/Sources/hedera/crypto/Hex.swift
index 48f3d653..388c6836 100644
--- a/Sources/hedera/crypto/Hex.swift
+++ b/Sources/hedera/crypto/Hex.swift
@@ -2,9 +2,9 @@ import Sodium
 
 struct HexDecodeError: Error {}
 
-func hexDecode<S>(_ hex: S) -> Result<Bytes, HexDecodeError> where S: StringProtocol {
-    if hex.count % 2 != 0 {
-        return .failure(HexDecodeError())
+func hexDecode<S>(_ hex: S) throws -> Bytes where S: StringProtocol {
+    if !hex.count.isMultiple(of: 2) {
+        throw HexDecodeError()
     }
 
     let bytesLen = hex.count / 2
@@ -13,13 +13,15 @@ func hexDecode<S>(_ hex: S) -> Result<Bytes, HexDecodeError> where S: StringProt
     for index in 0 ..< bytesLen {
         let start = hex.index(hex.startIndex, offsetBy: index * 2)
         let end = hex.index(start, offsetBy: 2)
-        bytes[index] = UInt8(hex[start ..< end], radix: 16)!
+        
+        guard let byte = UInt8(hex[start ..< end], radix: 16) else { throw HexDecodeError() }
+        bytes[index] = byte
     }
 
-    return .success(bytes)
+    return bytes
 }
 
-func hexEncode<S>(bytes: Bytes, prefixed with: S) -> String where S: StringProtocol {
+func hexEncode(bytes: Bytes, prefixed with: String = "") -> String {
     var result = String(with)
     for byte in bytes {
         result.append(String(format: "%02x", byte))
diff --git a/Sources/hedera/crypto/ed25519/Ed25519PrivateKey.swift b/Sources/hedera/crypto/ed25519/Ed25519PrivateKey.swift
index 3261020a..d90e8f93 100644
--- a/Sources/hedera/crypto/ed25519/Ed25519PrivateKey.swift
+++ b/Sources/hedera/crypto/ed25519/Ed25519PrivateKey.swift
@@ -48,19 +48,15 @@ extension Ed25519PrivateKey: LosslessStringConvertible {
     public init?(_ description: String) {
         switch description.count {
         case ed25519PrivateKeyLength * 2, combinedEd25519KeyLength * 2: // lone key, or combined key
-            // This cannot fail to decode
-            // swiftlint:disable:next force_try
-            self = Ed25519PrivateKey(bytes: try! hexDecode(description).get())!
+            guard let decoded = try? hexDecode(description) else { return nil }
+            inner = decoded
 
         case ed25519PrivateKeyLength * 2 + ed25519PrivateKeyPrefix.count: // DER encoded key
-            if description.hasPrefix(ed25519PrivateKeyPrefix) {
-                let range = description.index(description.startIndex, offsetBy: ed25519PrivateKeyPrefix.count)...
-                // This cannot fail to decode
-                // swiftlint:disable:next force_try
-                self = Ed25519PrivateKey(bytes: try! hexDecode(description[range]).get())!
-            } else {
-                return nil
-            }
+            guard description.hasPrefix(ed25519PrivateKeyPrefix) else { return nil }
+    
+            let range = description.index(description.startIndex, offsetBy: ed25519PrivateKeyPrefix.count)...
+            guard let decoded = try? hexDecode(description[range]) else { return nil }
+            inner = decoded
 
         default:
             return nil
diff --git a/Sources/hedera/crypto/ed25519/Ed25519PublicKey.swift b/Sources/hedera/crypto/ed25519/Ed25519PublicKey.swift
index d91b1e7b..836b8437 100644
--- a/Sources/hedera/crypto/ed25519/Ed25519PublicKey.swift
+++ b/Sources/hedera/crypto/ed25519/Ed25519PublicKey.swift
@@ -32,19 +32,20 @@ extension Ed25519PublicKey: CustomDebugStringConvertible {
     }
 }
 
-// TODO: Make this not explode on bad hex chars
 extension Ed25519PublicKey: LosslessStringConvertible {
     public init?(_ description: String) {
         switch description.count {
         case ed25519PublicKeyLength * 2:
-            // This cannot fail
-            // swiftlint:disable:next force_try
-            inner = try! hexDecode(description).get()
+            guard let decoded = try? hexDecode(description) else { return nil }
+            inner = decoded
+
         case ed25519PublicKeyLength * 2 + ed25519PublicKeyPrefix.count:
+            guard description.hasPrefix(ed25519PublicKeyPrefix) else { return nil }
+
             let start = description.index(description.startIndex, offsetBy: ed25519PublicKeyPrefix.count)
-            // This cannot fail
-            // swiftlint:disable:next force_try
-            inner = try! hexDecode(description[start...]).get()
+            guard let decoded = try? hexDecode(description[start...]) else { return nil }
+            inner = decoded
+
         default:
             return nil
         }
diff --git a/Tests/hederaTests/crypto/HexTests.swift b/Tests/hederaTests/crypto/HexTests.swift
index fc00cd40..4e92e46d 100644
--- a/Tests/hederaTests/crypto/HexTests.swift
+++ b/Tests/hederaTests/crypto/HexTests.swift
@@ -5,15 +5,17 @@ final class HexTests: XCTestCase {
     let testKeyStr = "302e020100300506032b657004220420db484b828e64b2d8f12ce3c0a0e93a0b8cce7af1bb8f39c97732394482538e10"
 
     func testDecodeEncode() {
-        let decoded = try! hexDecode(testKeyStr).get()
-        let encoded = hexEncode(bytes: decoded, prefixed: "")
+        let decoded = try? hexDecode(testKeyStr)
+        XCTAssertNotNil(decoded)
+
+        let encoded = hexEncode(bytes: decoded!)
         XCTAssertEqual(testKeyStr, encoded)
     }
 
     func testDecodeBadString() {
-        let badString = "a"
-        let decoded = hexDecode(badString)
-        XCTAssertThrowsError(try decoded.get())
+        XCTAssertThrowsError(try hexDecode("a"))
+
+        XCTAssertThrowsError(try hexDecode("4G"))
     }
 
     static var allTests = [