Address comments

Author: mosherBT

Source/Core/EdgeAPI.swift

@@ -41,7 +41,7 @@ final class EdgeAPI {
     // MARK: Endpoints
     func identify(ids: [OptableIdentifier]) throws -> URLRequest? {
         guard let url = buildEdgeAPIURL(endpoint: "identify") else { return nil }
-        let jsonData = try jsonEncoder.encode(ids)
+        let jsonData = try jsonEncoder.encode(ids.filter({ $0.extendedIdentifier.isEmpty == false }))
         let request = try buildRequest(.POST, url: url, headers: resolveHeaders(), data: jsonData)
         return request
     }
@@ -67,7 +67,8 @@ final class EdgeAPI {
         guard var url = buildEdgeAPIURL(endpoint: "targeting") else { return nil }
 
         let queryItems = ids
-            .compactMap({ $0.extendedIdentifier })
+            .map({ $0.extendedIdentifier })
+            .filter({ $0.isEmpty == false })
             .compactMap({ URLQueryItem(name: "id", value: $0) })
         url.compatAppend(queryItems: queryItems)
 

Source/Core/OptableIdentifierEncoder.swift

@@ -31,15 +31,24 @@ enum OptableIdentifierEncoder {
         case let .utiq(value): utiq(prefix, value)
         case let .custom(idx, value): custom(prefix, idx: idx ?? 0, value)
         case let .optableVID(value): vid(prefix, value)
-        case let .hem(value): hashed(prefix, value)
-        case let .hashedPhoneNumber(value): hashed(prefix, value)
+        case let .hem(value): hem(prefix, value)
+        case let .hashedPhoneNumber(value): hashedPhoneNumber(prefix, value)
         }
         return eid
     }
 
-    /// Builds Extended Identifier from an already-hashed Email address or Phone number.
-    static func hashed(_ prefix: String, _ hash: String) -> String {
-        let identifier = hash.components(separatedBy: CharacterSet.whitespacesAndNewlines).joined().lowercased()
+    /// Builds Extended Identifier from an already-hashed Email address (HEM).
+    /// Returns an empty string when the value is not a SHA256 digest, so that a
+    /// plaintext Email is never sent over the wire.
+    static func hem(_ prefix: String, _ hash: String) -> String {
+        guard let identifier = validSHA256(hash) else { return "" }
+        return "\(prefix):\(identifier)"
+    }
+
+    /// Builds Extended Identifier from an already-hashed Phone number.
+    /// Returns an empty string when the value is not a SHA256 digest.
+    static func hashedPhoneNumber(_ prefix: String, _ hash: String) -> String {
+        guard let identifier = validSHA256(hash) else { return "" }
         return "\(prefix):\(identifier)"
     }
 
@@ -173,6 +182,19 @@ enum OptableIdentifierEncoder {
     }
 
     // MARK: - Private
+    private static let sha256HexCharacters = CharacterSet(charactersIn: "0123456789abcdef")
+
+    /// Normalizes an already-hashed value, returning nil unless it is a SHA256 hex digest.
+    private static func validSHA256(_ hash: String) -> String? {
+        let identifier = hash.components(separatedBy: CharacterSet.whitespacesAndNewlines).joined().lowercased()
+
+        guard identifier.count == 64,
+              identifier.rangeOfCharacter(from: sha256HexCharacters.inverted) == nil
+        else { return nil }
+
+        return identifier
+    }
+
     private static func sha256(data: Data) -> String {
         #if canImport(CryptoKit)
             if #available(iOS 13.0, *) {

Source/Public/ObjCSupport/OptableSDKIdentifier.m

@@ -7,6 +7,18 @@
 
 #import "OptableSDKIdentifier.h"
 
+/// An Email address always contains "@", which is not a hex character, so a
+/// plaintext value can never be mistaken for a SHA256 digest.
+static BOOL OptableIsSHA256Hex(NSString *value)
+{
+    if (value.length != 64) return NO;
+
+    NSCharacterSet *nonHex =
+    [[NSCharacterSet characterSetWithCharactersInString:@"0123456789abcdefABCDEF"] invertedSet];
+
+    return [value rangeOfCharacterFromSet:nonHex].location == NSNotFound;
+}
+
 @implementation OptableSDKIdentifier {
     OptableSDKIdentifierType _type;
     NSString *_value;
@@ -112,7 +124,22 @@ + (nullable instancetype)identifierWithString:(NSString *)string
     NSString *typeRaw = [string substringToIndex:range.location];
     NSString *value   = [string substringFromIndex:range.location + 1];
 
-    return [[self alloc] initWithTypeRawValue:typeRaw value:value];
+    OptableSDKIdentifier *identifier = [[self alloc] initWithTypeRawValue:typeRaw value:value];
+    if (identifier == nil) return nil;
+
+    // The packed form carries either a plaintext value or one that is already
+    // hashed, so the value itself decides which. Without this, a caller passing
+    // "e:<sha256>" would have it hashed a second time.
+    if (OptableIsSHA256Hex(value)) {
+        if (identifier.type == OptableSDKIdentifierType_EmailAddress) {
+            return [self identifierWithType:OptableSDKIdentifierType_HEM value:value];
+        }
+        if (identifier.type == OptableSDKIdentifierType_PhoneNumber) {
+            return [self identifierWithType:OptableSDKIdentifierType_HashedPhoneNumber value:value];
+        }
+    }
+
+    return identifier;
 }
 
 @end

Tests/Unit/OptableIdentifierEncoderTests.swift

@@ -146,17 +146,30 @@ class OptableIdentifierEncoderTests: XCTestCase {
         let prefix = OptableIdentifier.hem("").prefix
         let hem = "a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3"
 
-        XCTAssertEqual("e:\(hem)", SUT.hashed(prefix, hem))
-        XCTAssertEqual("e:\(hem)", SUT.hashed(prefix, "  \(hem)  "))
-        XCTAssertEqual("e:\(hem)", SUT.hashed(prefix, hem.uppercased()))
+        XCTAssertEqual("e:\(hem)", SUT.hem(prefix, hem))
+        XCTAssertEqual("e:\(hem)", SUT.hem(prefix, "  \(hem)  "))
+        XCTAssertEqual("e:\(hem)", SUT.hem(prefix, hem.uppercased()))
+    }
+
+    func test_hem_rejectsAnythingNotASHA256() {
+        let prefix = OptableIdentifier.hem("").prefix
+        let hem = "a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3"
+
+        // A plaintext Email must never reach the wire.
+        XCTAssertEqual("", SUT.hem(prefix, "test@foobarbaz.com"))
+        XCTAssertEqual("", SUT.hem(prefix, ""))
+        XCTAssertEqual("", SUT.hem(prefix, String(hem.dropLast()))) // too short
+        XCTAssertEqual("", SUT.hem(prefix, hem + "a")) // too long
+        XCTAssertEqual("", SUT.hem(prefix, String(hem.dropLast()) + "z")) // non-hex
     }
 
     func test_hashedPhoneNumber() {
         let prefix = OptableIdentifier.hashedPhoneNumber("").prefix
         let hash = "ebad3b64ae96005048fca1af2f15e5251ad3844d00fb80252711de9b651c8e46"
 
-        XCTAssertEqual("p:\(hash)", SUT.hashed(prefix, hash))
-        XCTAssertEqual("p:\(hash)", SUT.hashed(prefix, " \(hash) "))
+        XCTAssertEqual("p:\(hash)", SUT.hashedPhoneNumber(prefix, hash))
+        XCTAssertEqual("p:\(hash)", SUT.hashedPhoneNumber(prefix, " \(hash) "))
+        XCTAssertEqual("", SUT.hashedPhoneNumber(prefix, "+33555456789"))
     }
 
     func test_hem_isNotHashedAgain() {

Tests/Unit/OptableIdentifiersTests.swift

@@ -95,4 +95,24 @@ class OptableIdentifiersTests: XCTestCase {
 
         XCTAssertNil(OptableIdentifier(extendedIdentifier: "no-separator"))
     }
+
+    func test_objc_identifierWithString_detectsAlreadyHashedValues() throws {
+        let hem = "a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3"
+        let email = "test@foobarbaz.com"
+
+        func eid(_ string: String) throws -> String {
+            let objc = try XCTUnwrap(OptableSDKIdentifier(string: string), string)
+            return try XCTUnwrap(OptableIdentifier(objc: objc), string).extendedIdentifier
+        }
+
+        // A SHA256 is passed through, not hashed a second time.
+        XCTAssertEqual("e:\(hem)", try eid("e:\(hem)"))
+        XCTAssertEqual("p:\(hem)", try eid("p:\(hem)"))
+
+        // A plaintext value still gets hashed, as it did before.
+        XCTAssertEqual(OptableIdentifier.emailAddress(email).extendedIdentifier, try eid("e:\(email)"))
+
+        // Types that are never hashed are unaffected.
+        XCTAssertEqual("c9:custom-9-id", try eid("c9:custom-9-id"))
+    }
 }