From 614f8c9f3ae08b32db47299073ac462210b35dd6 Mon Sep 17 00:00:00 2001 From: Joshua Myers Date: Fri, 22 May 2026 11:56:07 +1000 Subject: [PATCH] hash api keys in datafile handler --- .../DefaultDatafileHandler.swift | 37 ++++++++++---- Sources/ODP/OdpEventManager.swift | 2 +- .../DatafileHandlerTests.swift | 51 +++++++++++++++---- .../OdpEventManagerTests.swift | 8 +++ Tests/TestUtils/MockUrlSession.swift | 8 +-- Tests/TestUtils/OTUtils.swift | 31 +++++++++-- 6 files changed, 107 insertions(+), 30 deletions(-) diff --git a/Sources/Customization/DefaultDatafileHandler.swift b/Sources/Customization/DefaultDatafileHandler.swift index 1fd50548..a4631fa1 100644 --- a/Sources/Customization/DefaultDatafileHandler.swift +++ b/Sources/Customization/DefaultDatafileHandler.swift @@ -14,6 +14,7 @@ // limitations under the License. // +import CommonCrypto import Foundation open class DefaultDatafileHandler: OPTDatafileHandler { @@ -69,10 +70,10 @@ open class DefaultDatafileHandler: OPTDatafileHandler { } let session = self.getSession(resourceTimeoutInterval: resourceTimeoutInterval) - // without this the URLSession will leak, see docs on URLSession and https://stackoverflow.com/questions/67318867 - defer { session.finishTasksAndInvalidate() } - - guard let request = self.getRequest(sdkKey: sdkKey) else { return } + guard let request = self.getRequest(sdkKey: sdkKey) else { + session.finishTasksAndInvalidate() + return + } let task = session.downloadTask(with: request) { (url, response, error) in var result = OptimizelyResult.failure(.generic) @@ -104,6 +105,7 @@ open class DefaultDatafileHandler: OPTDatafileHandler { } self.reachability.updateNumContiguousFails(isError: (error != nil)) + session.finishTasksAndInvalidate() completionHandler(result) } @@ -212,23 +214,23 @@ open class DefaultDatafileHandler: OPTDatafileHandler { // MARK: - datafile store open func createDataStore(sdkKey: String) -> OPTDataStore { - return DataStoreFile(storeName: sdkKey) + return DataStoreFile(storeName: sdkKey.sha256Hash) } public func saveDatafile(sdkKey: String, dataFile: Data) { - getDatafileCache(sdkKey: sdkKey).saveItem(forKey: sdkKey, value: dataFile) + getDatafileCache(sdkKey: sdkKey).saveItem(forKey: sdkKey.sha256Hash, value: dataFile) } public func loadSavedDatafile(sdkKey: String) -> Data? { - return getDatafileCache(sdkKey: sdkKey).getItem(forKey: sdkKey) as? Data + return getDatafileCache(sdkKey: sdkKey).getItem(forKey: sdkKey.sha256Hash) as? Data } public func isDatafileSaved(sdkKey: String) -> Bool { - return getDatafileCache(sdkKey: sdkKey).getItem(forKey: sdkKey) as? Data != nil + return getDatafileCache(sdkKey: sdkKey).getItem(forKey: sdkKey.sha256Hash) as? Data != nil } public func removeSavedDatafile(sdkKey: String) { - getDatafileCache(sdkKey: sdkKey).removeItem(forKey: sdkKey) + getDatafileCache(sdkKey: sdkKey).removeItem(forKey: sdkKey.sha256Hash) } } @@ -336,11 +338,24 @@ extension DefaultDatafileHandler { extension DataStoreUserDefaults { func getLastModified(sdkKey: String) -> String? { - return getItem(forKey: "OPTLastModified-" + sdkKey) as? String + return getItem(forKey: "OPTLastModified-" + sdkKey.sha256Hash) as? String } func setLastModified(sdkKey: String, lastModified: String) { - saveItem(forKey: "OPTLastModified-" + sdkKey, value: lastModified) + saveItem(forKey: "OPTLastModified-" + sdkKey.sha256Hash, value: lastModified) + } +} + +extension String { + var sha256Hash: String { + let data = Data(utf8) + var digest = [UInt8](repeating: 0, count: Int(CC_SHA256_DIGEST_LENGTH)) + + data.withUnsafeBytes { buffer in + _ = CC_SHA256(buffer.baseAddress, CC_LONG(data.count), &digest) + } + + return digest.map { String(format: "%02x", $0) }.joined() } } diff --git a/Sources/ODP/OdpEventManager.swift b/Sources/ODP/OdpEventManager.swift index d136c6ea..b6b820ad 100644 --- a/Sources/ODP/OdpEventManager.swift +++ b/Sources/ODP/OdpEventManager.swift @@ -47,7 +47,7 @@ open class OdpEventManager { self.queueLock = DispatchQueue(label: "event") // a separate event queue for each sdkKey (which may have own ODP public key) - let storeName = "OPTEvent-ODP-\(sdkKey)" + let storeName = "OPTEvent-ODP-\(sdkKey.sha256Hash)" self.eventQueue = DataStoreQueueStackImpl(queueStackName: "odp", dataStore: DataStoreFile<[Data]>(storeName: storeName)) } diff --git a/Tests/OptimizelyTests-Common/DatafileHandlerTests.swift b/Tests/OptimizelyTests-Common/DatafileHandlerTests.swift index 6ec02195..b6249548 100644 --- a/Tests/OptimizelyTests-Common/DatafileHandlerTests.swift +++ b/Tests/OptimizelyTests-Common/DatafileHandlerTests.swift @@ -19,6 +19,7 @@ import XCTest class DatafileHandlerTests: XCTestCase { let sdkKey = "localcdnTestSDKKey" + let sdkKeyHash = "acd36624eecc323f29164c830871e59f91a9b604ac77794edfa060400750237c" override func setUp() { OTUtils.bindLoggerForTest(.info) @@ -416,9 +417,8 @@ class DatafileHandlerTests: XCTestCase { func testDownloadWithoutTimeout() { let handler = TimoutDatafileHandler() - // create a dummy file at a url to use as or datafile cdn location - let localUrl = OTUtils.saveAFile(name: "invalidKeyXXXXX", data: "{}".data(using: .utf8)!) - handler.localFileUrl = localUrl + handler.saveDatafile(sdkKey: "invalidKeyXXXXX", dataFile: "{}".data(using: .utf8)!) + XCTAssertTrue(handler.isDatafileSaved(sdkKey: "invalidKeyXXXXX")) let expectation = XCTestExpectation(description: "will wait for response.") handler.downloadDatafile(sdkKey: "invalidKeyXXXXX") { (result) in @@ -426,14 +426,48 @@ class DatafileHandlerTests: XCTestCase { print(data ?? "") XCTAssert(true) expectation.fulfill() - OTUtils.removeAFile(name: "invalidKeyXXXXX") + OTUtils.removeDatafileCache(sdkKey: "invalidKeyXXXXX") } } wait(for: [expectation], timeout: 10.0) } + + func testDatafileCacheUsesHashedPersistenceIdentifier() { + let handler = DefaultDatafileHandler() + let datafile = "{}".data(using: .utf8)! + + handler.saveDatafile(sdkKey: sdkKey, dataFile: datafile) + + XCTAssertTrue(handler.isDatafileSaved(sdkKey: sdkKey)) + + let store = handler.createDataStore(sdkKey: sdkKey) as! DataStoreFile + XCTAssertEqual(store.url.lastPathComponent, sdkKeyHash) + XCTAssertFalse(store.url.lastPathComponent.contains(sdkKey)) + } + + func testLastModifiedUsesHashedUserDefaultsKey() { + let handler = DefaultDatafileHandler() + + handler.sharedDataStore.setLastModified(sdkKey: sdkKey, lastModified: "1234") + + XCTAssertEqual(handler.sharedDataStore.getLastModified(sdkKey: sdkKey), "1234") + + let allKeys = UserDefaults.standard.dictionaryRepresentation().keys + XCTAssertTrue(allKeys.contains("OPTLastModified-\(sdkKeyHash)")) + XCTAssertFalse(allKeys.contains("OPTLastModified-\(sdkKey)")) + } + + func testLegacyRawLastModifiedKeyIsIgnored() { + let handler = DefaultDatafileHandler() + + OTUtils.createDatafileCache(sdkKey: sdkKey) + UserDefaults.standard.set("1234", forKey: "OPTLastModified-\(sdkKey)") + + XCTAssertNil(handler.getRequest(sdkKey: sdkKey)?.getLastModified()) + } - func testDatafileCacheFormatCompatibilty() { + func testLegacyRawNamedDatafileCacheIsIgnored() { // pre-store a datafile in a cache @@ -449,13 +483,10 @@ class DatafileHandlerTests: XCTestCase { url = url.appendingPathComponent(testSDKKey, isDirectory: false) try! datafileData.write(to: url, options: .atomic) - // verify that a new datafileHandler can read an existing datafile cache + // verify that hashed-only persistence causes a one-time cache miss for legacy raw-named files let datafileFromCache = DefaultDatafileHandler().loadSavedDatafile(sdkKey: testSDKKey) - XCTAssert(datafileFromCache == datafileData, "failed to support old datafile cached data format") - - let project = try! JSONDecoder().decode(Project.self, from: datafileFromCache!) - XCTAssert(project.revision == "241") + XCTAssertNil(datafileFromCache) } } diff --git a/Tests/OptimizelyTests-Common/OdpEventManagerTests.swift b/Tests/OptimizelyTests-Common/OdpEventManagerTests.swift index 2f989378..c420460d 100644 --- a/Tests/OptimizelyTests-Common/OdpEventManagerTests.swift +++ b/Tests/OptimizelyTests-Common/OdpEventManagerTests.swift @@ -20,6 +20,7 @@ class OdpEventManagerTests: XCTestCase { var manager: OdpEventManager! var odpConfig: OdpConfig! var apiManager = MockOdpEventApiManager() + let sdkKeyHash = "d6a7cd2a7371b1a15d543196979ff74fdb027023ebf187d5d329be11055c77fd" var options = [OptimizelySegmentOption]() @@ -56,6 +57,13 @@ class OdpEventManagerTests: XCTestCase { } // MARK: - save and restore events + + func testEventQueueUsesHashedPersistenceIdentifier() { + let dataStore = manager.eventQueue.dataStore as! DataStoreFile<[Data]> + + XCTAssertEqual(dataStore.url.lastPathComponent, "OPTEvent-ODP-\(sdkKeyHash)") + XCTAssertFalse(dataStore.url.lastPathComponent.contains("any")) + } func testSaveAndRestoreEvents() { manager.sendEvent(type: "t1", diff --git a/Tests/TestUtils/MockUrlSession.swift b/Tests/TestUtils/MockUrlSession.swift index 8845ff15..381a8a76 100644 --- a/Tests/TestUtils/MockUrlSession.swift +++ b/Tests/TestUtils/MockUrlSession.swift @@ -23,12 +23,12 @@ import Foundation // the cdn url is used to get the datafile if the datafile is not in cache class MockUrlSession: URLSession { static var validSessions = 0 + private static let validSessionsLock = DispatchQueue(label: "mock-session-counter-lock") var statusCode: Int var withError: Bool var localResponseData: String? var settingsMap: [String: (Int, Bool)]? var handler: MockDatafileHandler? - let lock = DispatchQueue(label: "mock-session-lock") class MockDownloadTask: URLSessionDownloadTask { var task: () -> Void @@ -55,7 +55,7 @@ class MockUrlSession: URLSession { } init(handler: MockDatafileHandler? = nil, statusCode: Int = 0, withError: Bool = false, localResponseData: String? = nil) { - lock.async { + Self.validSessionsLock.sync { Self.validSessions += 1 } self.handler = handler @@ -65,7 +65,7 @@ class MockUrlSession: URLSession { } init(handler: MockDatafileHandler? = nil, settingsMap: [String: (Int, Bool)]) { - lock.async { + Self.validSessionsLock.sync { Self.validSessions += 1 } self.handler = handler @@ -118,7 +118,7 @@ class MockUrlSession: URLSession { } override func finishTasksAndInvalidate() { - lock.async { + Self.validSessionsLock.sync { Self.validSessions -= 1 } } diff --git a/Tests/TestUtils/OTUtils.swift b/Tests/TestUtils/OTUtils.swift index b665a828..0298c427 100644 --- a/Tests/TestUtils/OTUtils.swift +++ b/Tests/TestUtils/OTUtils.swift @@ -254,16 +254,39 @@ class OTUtils { static func createDatafileCache(sdkKey: String, contents: String? = nil) { let data = (contents ?? "datafile-for-\(sdkKey)").data(using: .utf8)! - _ = saveAFile(name: sdkKey, data: data) + _ = saveAFile(name: sdkKey.sha256Hash, data: data) } static func removeDatafileCache(sdkKey: String, contents: String? = nil) { - removeAFile(name: sdkKey) + removeAFile(name: sdkKey.sha256Hash) } static func clearAllDataFiles(including: String? = nil) { - removeAllFiles(including: including, in: .documentDirectory) - removeAllFiles(including: including, in: .cachesDirectory) + removeAllDataFiles(matching: including.map { [$0, $0.sha256Hash] }, in: .documentDirectory) + removeAllDataFiles(matching: including.map { [$0, $0.sha256Hash] }, in: .cachesDirectory) + } + + static func removeAllDataFiles(matching namesToRemove: [String]?, in directory: FileManager.SearchPathDirectory) { + if let docUrl = FileManager.default.urls(for: directory, in: .userDomainMask).first { + if let names = try? FileManager.default.contentsOfDirectory(atPath: docUrl.path) { + names.forEach { name in + let shouldRemove: Bool + if let namesToRemove = namesToRemove { + shouldRemove = namesToRemove.contains(name) + } else { + shouldRemove = !name.contains("OPTEvent") + } + + if shouldRemove { + let fileUrl = docUrl.appendingPathComponent(name) + do { + try FileManager.default.removeItem(at: fileUrl) + } catch { + } + } + } + } + } } static func removeAllFiles(including: String? = nil, in directory: FileManager.SearchPathDirectory) {