From edbc1ffaaf911b3d3bf2e498e684ad692a477fde Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 7 Apr 2025 12:11:19 +1000 Subject: [PATCH 1/2] Cleaned up some memory management, tweak MRR logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Reworked how we were passing some variables to libSession to more safely manage scope • Updated the message request logic to be based on the `isDraft` flag on a conversation (rather than `shouldBeVisible) --- Session.xcodeproj/project.pbxproj | 16 +- .../xcshareddata/xcschemes/Session.xcscheme | 64 ----- .../Session_CompileLibSession.xcscheme | 27 +- .../ConversationVC+Interaction.swift | 40 ++- .../Crypto/Crypto+SessionMessagingKit.swift | 59 ++--- .../LibSession+GroupKeys.swift | 48 ++-- .../LibSession/Types/Config.swift | 106 +++----- .../LibSession/LibSessionUtilSpec.swift | 134 ++++++---- .../LibSession/LibSession+Networking.swift | 248 ++++++++---------- .../Types/BatchRequestSpec.swift | 2 +- .../General/Collection+Utilities.swift | 120 ++++----- .../Utilities/TypeConversion+Utilities.swift | 75 ++---- .../TypeConversionUtilitiesSpec.swift | 60 ++--- 13 files changed, 418 insertions(+), 581 deletions(-) diff --git a/Session.xcodeproj/project.pbxproj b/Session.xcodeproj/project.pbxproj index 4caedb50a..b97983858 100644 --- a/Session.xcodeproj/project.pbxproj +++ b/Session.xcodeproj/project.pbxproj @@ -7810,7 +7810,7 @@ CODE_SIGN_IDENTITY = "iPhone Developer"; COMPILE_LIB_SESSION = ""; COPY_PHASE_STRIP = NO; - CURRENT_PROJECT_VERSION = 567; + CURRENT_PROJECT_VERSION = 569; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; ENABLE_TESTABILITY = YES; @@ -7890,7 +7890,7 @@ CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_IDENTITY = "iPhone Distribution"; COMPILE_LIB_SESSION = ""; - CURRENT_PROJECT_VERSION = 567; + CURRENT_PROJECT_VERSION = 569; ENABLE_BITCODE = NO; ENABLE_MODULE_VERIFIER = YES; ENABLE_STRICT_OBJC_MSGSEND = YES; @@ -8075,6 +8075,7 @@ GENERATE_INFOPLIST_FILE = YES; PRODUCT_BUNDLE_IDENTIFIER = io.oxen.SessionSnodeKitTests; PRODUCT_NAME = SessionSnodeKitTests; + SWIFT_VERSION = 5.0; }; name = Debug; }; @@ -8085,6 +8086,7 @@ GENERATE_INFOPLIST_FILE = YES; PRODUCT_BUNDLE_IDENTIFIER = io.oxen.SessionSnodeKitTests; PRODUCT_NAME = SessionSnodeKitTests; + SWIFT_VERSION = 5.0; }; name = App_Store_Release; }; @@ -8279,14 +8281,20 @@ FD860CBF2D6E981900BBE29C /* Debug_Compile_LibSession */ = { isa = XCBuildConfiguration; buildSettings = { + GENERATE_INFOPLIST_FILE = YES; + PRODUCT_BUNDLE_IDENTIFIER = io.oxen.SessionSnodeKitTests; PRODUCT_NAME = SessionSnodeKitTests; + SWIFT_VERSION = 5.0; }; name = Debug_Compile_LibSession; }; FD860CC02D6E981900BBE29C /* App_Store_Release_Compile_LibSession */ = { isa = XCBuildConfiguration; buildSettings = { + GENERATE_INFOPLIST_FILE = YES; + PRODUCT_BUNDLE_IDENTIFIER = io.oxen.SessionSnodeKitTests; PRODUCT_NAME = SessionSnodeKitTests; + SWIFT_VERSION = 5.0; }; name = App_Store_Release_Compile_LibSession; }; @@ -8413,7 +8421,7 @@ CODE_SIGN_IDENTITY = "iPhone Developer"; COMPILE_LIB_SESSION = YES; COPY_PHASE_STRIP = NO; - CURRENT_PROJECT_VERSION = 567; + CURRENT_PROJECT_VERSION = 569; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; ENABLE_TESTABILITY = YES; @@ -9051,7 +9059,7 @@ CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_IDENTITY = "iPhone Distribution"; COMPILE_LIB_SESSION = YES; - CURRENT_PROJECT_VERSION = 567; + CURRENT_PROJECT_VERSION = 569; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; GCC_NO_COMMON_BLOCKS = YES; diff --git a/Session.xcodeproj/xcshareddata/xcschemes/Session.xcscheme b/Session.xcodeproj/xcshareddata/xcschemes/Session.xcscheme index 9f0430f09..ad1576e05 100644 --- a/Session.xcodeproj/xcshareddata/xcschemes/Session.xcscheme +++ b/Session.xcodeproj/xcshareddata/xcschemes/Session.xcscheme @@ -20,34 +20,6 @@ ReferencedContainer = "container:Session.xcodeproj"> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + shouldUseLaunchSchemeArgsEnv = "YES"> + + + + AnyPublisher { let updateNavigationBackStack: () -> Void = { @@ -2610,11 +2607,11 @@ extension ConversationVC { else { return Just(()).eraseToAnyPublisher() } return viewModel.dependencies[singleton: .storage] - .writePublisher { [displayName = self.viewModel.threadData.displayName, dependencies = viewModel.dependencies] db in - // If we aren't creating a new thread (ie. sending a message request) then send a - // messageRequestResponse back to the sender (this allows the sender to know that - // they have been approved and can now use this contact in closed groups) - if !isNewThread { + .writePublisher { [dependencies = viewModel.dependencies] db in + /// If this isn't a draft thread (ie. sending a message request) then send a `messageRequestResponse` + /// back to the sender (this allows the sender to know that they have been approved and can now use this + /// contact in closed groups) + if !isDraft { _ = try? Interaction( threadId: threadId, threadVariant: threadVariant, @@ -2648,7 +2645,7 @@ extension ConversationVC { db, Contact.Columns.isApproved.set(to: true), Contact.Columns.didApproveMe - .set(to: contact.didApproveMe || !isNewThread), + .set(to: contact.didApproveMe || !isDraft), using: dependencies ) } @@ -2673,8 +2670,8 @@ extension ConversationVC { return viewModel.dependencies[singleton: .storage] .writePublisher { [dependencies = viewModel.dependencies] db in - /// Remove any existing `infoGroupInfoInvited` interactions from the group (don't want to have a duplicate one from - /// inside the group history) + /// Remove any existing `infoGroupInfoInvited` interactions from the group (don't want to have a + /// duplicate one from inside the group history) _ = try Interaction .filter(Interaction.Columns.threadId == group.id) .filter(Interaction.Columns.variant == Interaction.Variant.infoGroupInfoInvited) @@ -2691,10 +2688,10 @@ extension ConversationVC { isHidden: false ).upsert(db) - /// If we aren't creating a new thread (ie. sending a message request) and the user is not an admin - /// then schedule sending a `GroupUpdateInviteResponseMessage` to the group (this allows - /// other members to know that the user has joined the group) - if !isNewThread && group.groupIdentityPrivateKey == nil { + /// If this isn't a draft thread (ie. sending a message request) and the user is not an admin then schedule + /// sending a `GroupUpdateInviteResponseMessage` to the group (this allows other members to + /// know that the user has joined the group) + if !isDraft && group.groupIdentityPrivateKey == nil { try MessageSender.send( db, message: GroupUpdateInviteResponseMessage( @@ -2733,7 +2730,8 @@ extension ConversationVC { approveMessageRequestIfNeeded( for: self.viewModel.threadData.threadId, threadVariant: self.viewModel.threadData.threadVariant, - isNewThread: false, + displayName: self.viewModel.threadData.displayName, + isDraft: (self.viewModel.threadData.threadIsDraft == true), timestampMs: viewModel.dependencies[cache: .snodeAPI].currentOffsetTimestampMs() ).sinkUntilComplete() } diff --git a/SessionMessagingKit/Crypto/Crypto+SessionMessagingKit.swift b/SessionMessagingKit/Crypto/Crypto+SessionMessagingKit.swift index d8d090bf9..902c21ab4 100644 --- a/SessionMessagingKit/Crypto/Crypto+SessionMessagingKit.swift +++ b/SessionMessagingKit/Crypto/Crypto+SessionMessagingKit.swift @@ -76,36 +76,35 @@ public extension Crypto.Generator { args: [messages, recipients, ed25519PrivateKey, domain] ) { var outLen: Int = 0 - var cMessages: [UnsafePointer?] = (try? (messages - .map { message -> [UInt8] in Array(message) } - .unsafeCopyUInt8Array())) - .defaulting(to: []) - var messageSizes: [Int] = messages.map { $0.count } - var cRecipients: [UnsafePointer?] = (try? (recipients - .map { recipient -> [UInt8] in recipient.publicKey } - .unsafeCopyUInt8Array())) - .defaulting(to: []) - var secretKey: [UInt8] = ed25519PrivateKey - var cDomain: [CChar] = try domain.cString(using: .utf8) ?? { throw LibSessionError.invalidCConversion }() - let cEncryptedDataPtr: UnsafeMutablePointer? = session_encrypt_for_multiple_simple_ed25519( - &outLen, - &cMessages, - &messageSizes, - messages.count, - &cRecipients, - recipients.count, - &secretKey, - &cDomain, - nil, - 0 - ) - - let encryptedData: Data? = cEncryptedDataPtr.map { Data(bytes: $0, count: outLen) } - cMessages.forEach { $0?.deallocate() } - cRecipients.forEach { $0?.deallocate() } - cEncryptedDataPtr?.deallocate() - - return try encryptedData ?? { throw MessageSenderError.encryptionFailed }() + return try messages.map { Array($0) }.withUnsafeUInt8CArray { cMessages in + try recipients.map { Array($0.publicKey) }.withUnsafeUInt8CArray { cRecipients in + var messageSizes: [Int] = messages.map { $0.count } + var secretKey: [UInt8] = ed25519PrivateKey + var cDomain: [CChar] = try domain.cString(using: .utf8) ?? { + throw LibSessionError.invalidCConversion + }() + + let cEncryptedDataPtr: UnsafeMutablePointer? = session_encrypt_for_multiple_simple_ed25519( + &outLen, + cMessages.baseAddress, + &messageSizes, + messages.count, + cRecipients.baseAddress, + recipients.count, + &secretKey, + &cDomain, + nil, + 0 + ) + + let encryptedData: Data? = cEncryptedDataPtr.map { Data(bytes: $0, count: outLen) } + cMessages.forEach { $0?.deallocate() } + cRecipients.forEach { $0?.deallocate() } + cEncryptedDataPtr?.deallocate() + + return try encryptedData ?? { throw MessageSenderError.encryptionFailed }() + } + } } } } diff --git a/SessionMessagingKit/LibSession/Config Handling/LibSession+GroupKeys.swift b/SessionMessagingKit/LibSession/Config Handling/LibSession+GroupKeys.swift index b2e404070..5cba9d169 100644 --- a/SessionMessagingKit/LibSession/Config Handling/LibSession+GroupKeys.swift +++ b/SessionMessagingKit/LibSession/Config Handling/LibSession+GroupKeys.swift @@ -107,33 +107,27 @@ internal extension LibSession { throw LibSessionError.invalidConfigObject } - var cMemberIds: [UnsafePointer?] = ( try? (memberIds - .map { id in id.cString(using: .utf8) } - .unsafeCopyCStringArray())) - .defaulting(to: []) - - defer { cMemberIds.forEach { $0?.deallocate() } } - - // Performing a `key_supplement` returns the supplemental key changes, since our state doesn't care - // about the `GROUP_KEYS` needed for other members this change won't result in the `GROUP_KEYS` config - // going into a pending state or the `ConfigurationSyncJob` getting triggered so return the data so that - // the caller can push it directly - var cSupplementData: UnsafeMutablePointer! - var cSupplementDataLen: Int = 0 - - guard - groups_keys_key_supplement(conf, &cMemberIds, cMemberIds.count, &cSupplementData, &cSupplementDataLen), - let cSupplementData: UnsafeMutablePointer = cSupplementData - else { throw LibSessionError.failedToKeySupplementGroup } - - // Must deallocate on success - let supplementData: Data = Data( - bytes: cSupplementData, - count: cSupplementDataLen - ) - cSupplementData.deallocate() - - return supplementData + return try memberIds.withUnsafeCStrArray { cMemberIds in + /// Performing a `key_supplement` returns the supplemental key changes, since our state doesn't care about the + /// `GROUP_KEYS` needed for other members this change won't result in the `GROUP_KEYS` config going into a pending + /// state or the `ConfigurationSyncJob` getting triggered so return the data so that the caller can push it directly + var cSupplementData: UnsafeMutablePointer! + var cSupplementDataLen: Int = 0 + + guard + groups_keys_key_supplement(conf, cMemberIds.baseAddress, cMemberIds.count, &cSupplementData, &cSupplementDataLen), + let cSupplementData: UnsafeMutablePointer = cSupplementData + else { throw LibSessionError.failedToKeySupplementGroup } + + // Must deallocate on success + let supplementData: Data = Data( + bytes: cSupplementData, + count: cSupplementDataLen + ) + cSupplementData.deallocate() + + return supplementData + } } } diff --git a/SessionMessagingKit/LibSession/Types/Config.swift b/SessionMessagingKit/LibSession/Types/Config.swift index b9ee340d9..d6eee8374 100644 --- a/SessionMessagingKit/LibSession/Types/Config.swift +++ b/SessionMessagingKit/LibSession/Types/Config.swift @@ -216,10 +216,9 @@ public extension LibSession { } let result: [String] = [String]( - pointer: hashList.pointee.value, - count: hashList.pointee.len, - defaultValue: [] - ) + cStringArray: hashList.pointee.value, + count: hashList.pointee.len + ).defaulting(to: []) hashList.deallocate() return result @@ -230,10 +229,9 @@ public extension LibSession { } let result: [String] = [String]( - pointer: hashList.pointee.value, - count: hashList.pointee.len, - defaultValue: [] - ) + cStringArray: hashList.pointee.value, + count: hashList.pointee.len + ).defaulting(to: []) hashList.deallocate() return result @@ -251,10 +249,9 @@ public extension LibSession { } let result: [String] = [String]( - pointer: hashList.pointee.value, - count: hashList.pointee.len, - defaultValue: [] - ) + cStringArray: hashList.pointee.value, + count: hashList.pointee.len + ).defaulting(to: []) hashList.deallocate() return result @@ -266,64 +263,41 @@ public extension LibSession { case .userProfile(let conf), .contacts(let conf), .convoInfoVolatile(let conf), .userGroups(let conf), .groupInfo(let conf), .groupMembers(let conf): - var mergeHashes: [UnsafePointer?] = (try? (messages - .compactMap { message in message.serverHash.cString(using: .utf8) } - .unsafeCopyCStringArray())) - .defaulting(to: []) - var mergeData: [UnsafePointer?] = (try? (messages - .map { message -> [UInt8] in Array(message.data) } - .unsafeCopyUInt8Array())) - .defaulting(to: []) - defer { - mergeHashes.forEach { $0?.deallocate() } - mergeData.forEach { $0?.deallocate() } - } - - guard - mergeHashes.count == messages.count, - mergeData.count == messages.count, - mergeHashes.allSatisfy({ $0 != nil }), - mergeData.allSatisfy({ $0 != nil }) - else { - Log.error(.libSession, "Failed to correctly allocate merge data") - return nil - } - - var mergeSize: [size_t] = messages.map { size_t($0.data.count) } - let mergedHashesPtr: UnsafeMutablePointer? = config_merge( - conf, - &mergeHashes, - &mergeData, - &mergeSize, - messages.count - ) - - // If we got an error then throw it - try LibSessionError.throwIfNeeded(conf) - - // Get the list of hashes from the config (to determine which were successful) - let mergedHashes: [String] = mergedHashesPtr - .map { ptr in - [String]( - pointer: ptr.pointee.value, - count: ptr.pointee.len, - defaultValue: [] + return try messages.map { $0.serverHash }.withUnsafeCStrArray { cMergehashes in + try messages.map { Array($0.data) }.withUnsafeUInt8CArray { cMergeData in + var mergeSize: [size_t] = messages.map { size_t($0.data.count) } + let mergedHashesPtr: UnsafeMutablePointer? = config_merge( + conf, + cMergehashes.baseAddress, + cMergeData.baseAddress, + &mergeSize, + messages.count ) + + // If we got an error then throw it + try LibSessionError.throwIfNeeded(conf) + + // Get the list of hashes from the config (to determine which were successful) + let mergedHashes: [String] = mergedHashesPtr + .map { ptr in + [String](cStringArray: ptr.pointee.value, count: ptr.pointee.len) + .defaulting(to: []) + } + .defaulting(to: []) + mergedHashesPtr?.deallocate() + + if mergedHashes.count != messages.count { + Log.warn(.libSession, "Unable to merge \(messages[0].namespace) messages (\(mergedHashes.count)/\(messages.count))") + } + + return messages + .filter { mergedHashes.contains($0.serverHash) } + .map { $0.serverTimestampMs } + .sorted() + .last } - .defaulting(to: []) - mergedHashesPtr?.deallocate() - - if mergedHashes.count != messages.count { - Log.warn(.libSession, "Unable to merge \(messages[0].namespace) messages (\(mergedHashes.count)/\(messages.count))") } - return messages - .filter { mergedHashes.contains($0.serverHash) } - .map { $0.serverTimestampMs } - .sorted() - .last - - case .groupKeys(let conf, let infoConf, let membersConf): let successfulMergeTimestamps: [Int64] = try messages .map { message -> (Bool, Int64) in diff --git a/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift b/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift index 41baf5fb6..816888578 100644 --- a/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift +++ b/SessionMessagingKitTests/LibSession/LibSessionUtilSpec.swift @@ -402,7 +402,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData: [UnsafePointer?] = [UnsafePointer(pushData4.pointee.config)] var mergeSize: [Int] = [pushData4.pointee.config_len] let mergedHashes: UnsafeMutablePointer? = config_merge(conf, &mergeHashes, &mergeData, &mergeSize, 1) - expect([String](pointer: mergedHashes?.pointee.value, count: mergedHashes?.pointee.len)) + expect([String](cStringArray: mergedHashes?.pointee.value, count: mergedHashes?.pointee.len)) .to(equal(["fakehash2"])) config_confirm_pushed(conf2, pushData4.pointee.seqno, &cFakeHash2) mergeHashes.forEach { $0?.deallocate() } @@ -465,9 +465,9 @@ fileprivate extension LibSessionUtilSpec { let pushData6Data: Data = Data(bytes: pushData6.pointee.config, count: pushData6.pointee.config_len) let pushData7Data: Data = Data(bytes: pushData7.pointee.config, count: pushData7.pointee.config_len) expect(pushData6Data).toNot(equal(pushData7Data)) - expect([String](pointer: pushData6.pointee.obsolete, count: pushData6.pointee.obsolete_len)) + expect([String](cStringArray: pushData6.pointee.obsolete, count: pushData6.pointee.obsolete_len)) .to(equal([fakeHash2])) - expect([String](pointer: pushData7.pointee.obsolete, count: pushData7.pointee.obsolete_len)) + expect([String](cStringArray: pushData7.pointee.obsolete, count: pushData7.pointee.obsolete_len)) .to(equal([fakeHash2])) let fakeHash3a: String = "fakehash3a" @@ -481,7 +481,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData2: [UnsafePointer?] = [UnsafePointer(pushData7.pointee.config)] var mergeSize2: [Int] = [pushData7.pointee.config_len] let mergedHashes2: UnsafeMutablePointer? = config_merge(conf, &mergeHashes2, &mergeData2, &mergeSize2, 1) - expect([String](pointer: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) + expect([String](cStringArray: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) .to(equal(["fakehash3b"])) expect(config_needs_push(conf)).to(beTrue()) mergeHashes2.forEach { $0?.deallocate() } @@ -492,7 +492,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData3: [UnsafePointer?] = [UnsafePointer(pushData6.pointee.config)] var mergeSize3: [Int] = [pushData6.pointee.config_len] let mergedHashes3: UnsafeMutablePointer? = config_merge(conf2, &mergeHashes3, &mergeData3, &mergeSize3, 1) - expect([String](pointer: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) + expect([String](cStringArray: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) .to(equal(["fakehash3a"])) expect(config_needs_push(conf2)).to(beTrue()) mergeHashes3.forEach { $0?.deallocate() } @@ -508,9 +508,9 @@ fileprivate extension LibSessionUtilSpec { let pushData8Data: Data = Data(bytes: pushData8.pointee.config, count: pushData8.pointee.config_len) let pushData9Data: Data = Data(bytes: pushData9.pointee.config, count: pushData9.pointee.config_len) expect(pushData8Data).to(equal(pushData9Data)) - expect([String](pointer: pushData8.pointee.obsolete, count: pushData8.pointee.obsolete_len)) + expect([String](cStringArray: pushData8.pointee.obsolete, count: pushData8.pointee.obsolete_len)) .to(equal([fakeHash3b, fakeHash3a])) - expect([String](pointer: pushData9.pointee.obsolete, count: pushData9.pointee.obsolete_len)) + expect([String](cStringArray: pushData9.pointee.obsolete, count: pushData9.pointee.obsolete_len)) .to(equal([fakeHash3a, fakeHash3b])) let fakeHash4: String = "fakeHash4" @@ -712,7 +712,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData: [UnsafePointer?] = ((try? [expPush1Encrypted].unsafeCopyUInt8Array()) ?? []) var mergeSize: [Int] = [expPush1Encrypted.count] let mergedHashes: UnsafeMutablePointer? = config_merge(conf2, &mergeHashes, &mergeData, &mergeSize, 1) - expect([String](pointer: mergedHashes?.pointee.value, count: mergedHashes?.pointee.len)) + expect([String](cStringArray: mergedHashes?.pointee.value, count: mergedHashes?.pointee.len)) .to(equal(["fakehash1"])) mergeHashes.forEach { $0?.deallocate() } mergeData.forEach { $0?.deallocate() } @@ -795,7 +795,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData2: [UnsafePointer?] = [UnsafePointer(pushData3.pointee.config)] var mergeSize2: [Int] = [pushData3.pointee.config_len] let mergedHashes2: UnsafeMutablePointer? = config_merge(conf2, &mergeHashes2, &mergeData2, &mergeSize2, 1) - expect([String](pointer: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) + expect([String](cStringArray: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) .to(equal(["fakehash2"])) mergeHashes2.forEach { $0?.deallocate() } mergedHashes2?.deallocate() @@ -805,7 +805,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData3: [UnsafePointer?] = [UnsafePointer(pushData4.pointee.config)] var mergeSize3: [Int] = [pushData4.pointee.config_len] let mergedHashes3: UnsafeMutablePointer? = config_merge(conf, &mergeHashes3, &mergeData3, &mergeSize3, 1) - expect([String](pointer: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) + expect([String](cStringArray: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) .to(equal(["fakehash3"])) mergeHashes3.forEach { $0?.deallocate() } mergedHashes3?.deallocate() @@ -1021,7 +1021,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData: [UnsafePointer?] = [UnsafePointer(pushData2.pointee.config)] var mergeSize: [Int] = [pushData2.pointee.config_len] let mergedHashes: UnsafeMutablePointer? = config_merge(conf, &mergeHashes, &mergeData, &mergeSize, 1) - expect([String](pointer: mergedHashes?.pointee.value, count: mergedHashes?.pointee.len)) + expect([String](cStringArray: mergedHashes?.pointee.value, count: mergedHashes?.pointee.len)) .to(equal(["fakehash2"])) config_confirm_pushed(conf, pushData2.pointee.seqno, &cFakeHash2) mergeHashes.forEach { $0?.deallocate() } @@ -1188,7 +1188,7 @@ fileprivate extension LibSessionUtilSpec { // testing: let pushData1: UnsafeMutablePointer = config_push(conf) expect(pushData1.pointee.seqno).to(equal(0)) - expect([String](pointer: pushData1.pointee.obsolete, count: pushData1.pointee.obsolete_len)) + expect([String](cStringArray: pushData1.pointee.obsolete, count: pushData1.pointee.obsolete_len)) .to(beEmpty()) expect(pushData1.pointee.config_len).to(equal(432)) pushData1.deallocate() @@ -1286,7 +1286,7 @@ fileprivate extension LibSessionUtilSpec { // dumps; even though we changed two fields here). let pushData2: UnsafeMutablePointer = config_push(conf) expect(pushData2.pointee.seqno).to(equal(1)) - expect([String](pointer: pushData2.pointee.obsolete, count: pushData2.pointee.obsolete_len)) + expect([String](cStringArray: pushData2.pointee.obsolete, count: pushData2.pointee.obsolete_len)) .to(beEmpty()) // Pretend we uploaded it @@ -1310,12 +1310,12 @@ fileprivate extension LibSessionUtilSpec { let pushData3: UnsafeMutablePointer = config_push(conf) expect(pushData3.pointee.seqno).to(equal(1)) - expect([String](pointer: pushData3.pointee.obsolete, count: pushData3.pointee.obsolete_len)) + expect([String](cStringArray: pushData3.pointee.obsolete, count: pushData3.pointee.obsolete_len)) .to(beEmpty()) pushData3.deallocate() let currentHashes1: UnsafeMutablePointer? = config_current_hashes(conf) - expect([String](pointer: currentHashes1?.pointee.value, count: currentHashes1?.pointee.len)) + expect([String](cStringArray: currentHashes1?.pointee.value, count: currentHashes1?.pointee.len)) .to(equal(["fakehash1"])) currentHashes1?.deallocate() @@ -1325,12 +1325,12 @@ fileprivate extension LibSessionUtilSpec { let pushData4: UnsafeMutablePointer = config_push(conf2) expect(pushData4.pointee.seqno).to(equal(1)) expect(config_needs_dump(conf2)).to(beFalse()) - expect([String](pointer: pushData4.pointee.obsolete, count: pushData4.pointee.obsolete_len)) + expect([String](cStringArray: pushData4.pointee.obsolete, count: pushData4.pointee.obsolete_len)) .to(beEmpty()) pushData4.deallocate() let currentHashes2: UnsafeMutablePointer? = config_current_hashes(conf2) - expect([String](pointer: currentHashes2?.pointee.value, count: currentHashes2?.pointee.len)) + expect([String](cStringArray: currentHashes2?.pointee.value, count: currentHashes2?.pointee.len)) .to(equal(["fakehash1"])) currentHashes2?.deallocate() @@ -1440,11 +1440,11 @@ fileprivate extension LibSessionUtilSpec { let pushData7: UnsafeMutablePointer = config_push(conf2) expect(pushData7.pointee.seqno).to(equal(2)) config_confirm_pushed(conf2, pushData7.pointee.seqno, &cFakeHash2) - expect([String](pointer: pushData7.pointee.obsolete, count: pushData7.pointee.obsolete_len)) + expect([String](cStringArray: pushData7.pointee.obsolete, count: pushData7.pointee.obsolete_len)) .to(equal([fakeHash1])) let currentHashes3: UnsafeMutablePointer? = config_current_hashes(conf2) - expect([String](pointer: currentHashes3?.pointee.value, count: currentHashes3?.pointee.len)) + expect([String](cStringArray: currentHashes3?.pointee.value, count: currentHashes3?.pointee.len)) .to(equal([fakeHash2])) currentHashes3?.deallocate() @@ -1464,7 +1464,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData1: [UnsafePointer?] = [UnsafePointer(pushData8.pointee.config)] var mergeSize1: [Int] = [pushData8.pointee.config_len] let mergedHashes1: UnsafeMutablePointer? = config_merge(conf, &mergeHashes1, &mergeData1, &mergeSize1, 1) - expect([String](pointer: mergedHashes1?.pointee.value, count: mergedHashes1?.pointee.len)) + expect([String](cStringArray: mergedHashes1?.pointee.value, count: mergedHashes1?.pointee.len)) .to(equal(["fakehash2"])) mergeHashes1.forEach { $0?.deallocate() } mergedHashes1?.deallocate() @@ -1509,11 +1509,11 @@ fileprivate extension LibSessionUtilSpec { config_confirm_pushed(conf2, pushData10.pointee.seqno, &cFakeHash3) expect(pushData10.pointee.seqno).to(equal(3)) - expect([String](pointer: pushData10.pointee.obsolete, count: pushData10.pointee.obsolete_len)) + expect([String](cStringArray: pushData10.pointee.obsolete, count: pushData10.pointee.obsolete_len)) .to(equal([fakeHash2])) let currentHashes4: UnsafeMutablePointer? = config_current_hashes(conf2) - expect([String](pointer: currentHashes4?.pointee.value, count: currentHashes4?.pointee.len)) + expect([String](cStringArray: currentHashes4?.pointee.value, count: currentHashes4?.pointee.len)) .to(equal([fakeHash3])) currentHashes4?.deallocate() @@ -1521,7 +1521,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData2: [UnsafePointer?] = [UnsafePointer(pushData10.pointee.config)] var mergeSize2: [Int] = [pushData10.pointee.config_len] let mergedHashes2: UnsafeMutablePointer? = config_merge(conf, &mergeHashes2, &mergeData2, &mergeSize2, 1) - expect([String](pointer: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) + expect([String](cStringArray: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) .to(equal(["fakehash3"])) mergeHashes2.forEach { $0?.deallocate() } mergedHashes2?.deallocate() @@ -1556,7 +1556,7 @@ fileprivate extension LibSessionUtilSpec { let pushData11: UnsafeMutablePointer = config_push(conf) config_confirm_pushed(conf, pushData11.pointee.seqno, &cFakeHash4) expect(pushData11.pointee.seqno).to(equal(4)) - expect([String](pointer: pushData11.pointee.obsolete, count: pushData11.pointee.obsolete_len)) + expect([String](cStringArray: pushData11.pointee.obsolete, count: pushData11.pointee.obsolete_len)) .to(equal([fakeHash3, fakeHash2, fakeHash1])) // Load some obsolete ones in just to check that they get immediately obsoleted @@ -1580,7 +1580,7 @@ fileprivate extension LibSessionUtilSpec { pushData11.pointee.config_len ] let mergedHashes3: UnsafeMutablePointer? = config_merge(conf2, &mergeHashes3, &mergeData3, &mergeSize3, 4) - expect([String](pointer: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) + expect([String](cStringArray: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) .to(equal(["fakehash10", "fakehash11", "fakehash12", "fakehash4"])) expect(config_needs_dump(conf2)).to(beTrue()) expect(config_needs_push(conf2)).to(beFalse()) @@ -1592,13 +1592,13 @@ fileprivate extension LibSessionUtilSpec { pushData11.deallocate() let currentHashes5: UnsafeMutablePointer? = config_current_hashes(conf2) - expect([String](pointer: currentHashes5?.pointee.value, count: currentHashes5?.pointee.len)) + expect([String](cStringArray: currentHashes5?.pointee.value, count: currentHashes5?.pointee.len)) .to(equal([fakeHash4])) currentHashes5?.deallocate() let pushData12: UnsafeMutablePointer = config_push(conf2) expect(pushData12.pointee.seqno).to(equal(4)) - expect([String](pointer: pushData12.pointee.obsolete, count: pushData12.pointee.obsolete_len)) + expect([String](cStringArray: pushData12.pointee.obsolete, count: pushData12.pointee.obsolete_len)) .to(equal([fakeHash11, fakeHash12, fakeHash10, fakeHash3])) pushData12.deallocate() @@ -1716,7 +1716,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData1: [UnsafePointer?] = [UnsafePointer(pushData1.pointee.config)] var mergeSize1: [Int] = [pushData1.pointee.config_len] let mergedHashes1: UnsafeMutablePointer? = config_merge(conf2, &mergeHashes1, &mergeData1, &mergeSize1, 1) - expect([String](pointer: mergedHashes1?.pointee.value, count: mergedHashes1?.pointee.len)) + expect([String](cStringArray: mergedHashes1?.pointee.value, count: mergedHashes1?.pointee.len)) .to(equal(["fakehash1"])) expect(config_needs_push(conf2)).to(beFalse()) mergeHashes1.forEach { $0?.deallocate() } @@ -1745,10 +1745,9 @@ fileprivate extension LibSessionUtilSpec { groups_info_destroy_group(conf2) let pushData2: UnsafeMutablePointer = config_push(conf2) - let obsoleteHashes: [String] = [String]( - pointer: pushData2.pointee.obsolete, - count: pushData2.pointee.obsolete_len, - defaultValue: [] + let obsoleteHashes: [String]? = [String]( + cStringArray: pushData2.pointee.obsolete, + count: pushData2.pointee.obsolete_len ) expect(pushData2.pointee.seqno).to(equal(2)) expect(pushData2.pointee.config_len).to(equal(512)) @@ -1762,7 +1761,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData2: [UnsafePointer?] = [UnsafePointer(pushData2.pointee.config)] var mergeSize2: [Int] = [pushData2.pointee.config_len] let mergedHashes2: UnsafeMutablePointer? = config_merge(conf, &mergeHashes2, &mergeData2, &mergeSize2, 1) - expect([String](pointer: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) + expect([String](cStringArray: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) .to(equal(["fakehash2"])) mergeHashes2.forEach { $0?.deallocate() } mergedHashes2?.deallocate() @@ -1800,7 +1799,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData3: [UnsafePointer?] = [UnsafePointer(pushData3.pointee.config)] var mergeSize3: [Int] = [pushData3.pointee.config_len] let mergedHashes3: UnsafeMutablePointer? = config_merge(conf2, &mergeHashes3, &mergeData3, &mergeSize3, 1) - expect([String](pointer: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) + expect([String](cStringArray: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) .to(equal(["fakehash3"])) mergeHashes3.forEach { $0?.deallocate() } mergedHashes3?.deallocate() @@ -2069,7 +2068,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData1: [UnsafePointer?] = [UnsafePointer(pushData1.pointee.config)] var mergeSize1: [Int] = [pushData1.pointee.config_len] let mergedHashes1: UnsafeMutablePointer? = config_merge(conf2, &mergeHashes1, &mergeData1, &mergeSize1, 1) - expect([String](pointer: mergedHashes1?.pointee.value, count: mergedHashes1?.pointee.len)) + expect([String](cStringArray: mergedHashes1?.pointee.value, count: mergedHashes1?.pointee.len)) .to(equal(["fakehash1"])) expect(config_needs_push(conf2)).to(beFalse()) mergeHashes1.forEach { $0?.deallocate() } @@ -2160,10 +2159,9 @@ fileprivate extension LibSessionUtilSpec { groups_members_set(conf2, &member1) let pushData2: UnsafeMutablePointer = config_push(conf2) - let obsoleteHashes: [String] = [String]( - pointer: pushData2.pointee.obsolete, - count: pushData2.pointee.obsolete_len, - defaultValue: [] + let obsoleteHashes: [String]? = [String]( + cStringArray: pushData2.pointee.obsolete, + count: pushData2.pointee.obsolete_len ) expect(pushData2.pointee.seqno).to(equal(2)) expect(pushData2.pointee.config_len).to(equal(1024)) @@ -2177,7 +2175,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData2: [UnsafePointer?] = [UnsafePointer(pushData2.pointee.config)] var mergeSize2: [Int] = [pushData2.pointee.config_len] let mergedHashes2: UnsafeMutablePointer? = config_merge(conf, &mergeHashes2, &mergeData2, &mergeSize2, 1) - expect([String](pointer: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) + expect([String](cStringArray: mergedHashes2?.pointee.value, count: mergedHashes2?.pointee.len)) .to(equal(["fakehash2"])) mergeHashes2.forEach { $0?.deallocate() } mergedHashes2?.deallocate() @@ -2334,10 +2332,9 @@ fileprivate extension LibSessionUtilSpec { } let pushData3: UnsafeMutablePointer = config_push(conf) - let obsoleteHashes3: [String] = [String]( - pointer: pushData3.pointee.obsolete, - count: pushData3.pointee.obsolete_len, - defaultValue: [] + let obsoleteHashes3: [String]? = [String]( + cStringArray: pushData3.pointee.obsolete, + count: pushData3.pointee.obsolete_len ) expect(pushData3.pointee.seqno).to(equal(3)) expect(pushData3.pointee.config_len).to(equal(1024)) @@ -2351,7 +2348,7 @@ fileprivate extension LibSessionUtilSpec { var mergeData3: [UnsafePointer?] = [UnsafePointer(pushData3.pointee.config)] var mergeSize3: [Int] = [pushData3.pointee.config_len] let mergedHashes3: UnsafeMutablePointer? = config_merge(conf2, &mergeHashes3, &mergeData3, &mergeSize3, 1) - expect([String](pointer: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) + expect([String](cStringArray: mergedHashes3?.pointee.value, count: mergedHashes3?.pointee.len)) .to(equal(["fakehash3"])) mergeHashes3.forEach { $0?.deallocate() } mergedHashes3?.deallocate() @@ -2703,3 +2700,50 @@ private extension LibSessionUtilSpec { return false } } + +private extension Collection where Element == [CChar]? { + /// This creates an array of UnsafePointer types to access data of the C strings in memory. This array provides no automated + /// memory management of it's children so after use you are responsible for handling the life cycle of the child elements and + /// need to call `deallocate()` on each child. + func unsafeCopyCStringArray() throws -> [UnsafePointer?] { + return try self.map { value in + guard let value: [CChar] = value else { return nil } + + let copy = UnsafeMutableBufferPointer.allocate(capacity: value.underestimatedCount) + var remaining: (unwritten: Array.Iterator, index: Int) = copy.initialize(from: value) + guard remaining.unwritten.next() == nil else { throw LibSessionError.invalidCConversion } + + return UnsafePointer(copy.baseAddress) + } + } +} + +private extension Collection where Element == [CChar] { + /// This creates an array of UnsafePointer types to access data of the C strings in memory. This array provides no automated + /// memory management of it's children so after use you are responsible for handling the life cycle of the child elements and + /// need to call `deallocate()` on each child. + func unsafeCopyCStringArray() throws -> [UnsafePointer?] { + return try self.map { value in + let copy = UnsafeMutableBufferPointer.allocate(capacity: value.underestimatedCount) + var remaining: (unwritten: Array.Iterator, index: Int) = copy.initialize(from: value) + guard remaining.unwritten.next() == nil else { throw LibSessionError.invalidCConversion } + + return UnsafePointer(copy.baseAddress) + } + } +} + +private extension Collection where Element == [UInt8] { + /// This creates an array of UnsafePointer types to access data of the C strings in memory. This array provides no automated + /// memory management of it's children so after use you are responsible for handling the life cycle of the child elements and + /// need to call `deallocate()` on each child. + func unsafeCopyUInt8Array() throws -> [UnsafePointer?] { + return try self.map { value in + let copy = UnsafeMutableBufferPointer.allocate(capacity: value.underestimatedCount) + var remaining: (unwritten: Array.Iterator, index: Int) = copy.initialize(from: value) + guard remaining.unwritten.next() == nil else { throw LibSessionError.invalidCConversion } + + return UnsafePointer(copy.baseAddress) + } + } +} diff --git a/SessionSnodeKit/LibSession/LibSession+Networking.swift b/SessionSnodeKit/LibSession/LibSession+Networking.swift index fed40b519..3d314fc36 100644 --- a/SessionSnodeKit/LibSession/LibSession+Networking.swift +++ b/SessionSnodeKit/LibSession/LibSession+Networking.swift @@ -253,13 +253,12 @@ class LibSessionNetwork: NetworkType { throw NetworkError.invalidPreparedRequest case .snode(let snode, let swarmPublicKey): - let cSwarmPublicKey: UnsafePointer? = try swarmPublicKey.map { + let cSwarmPublicKey: [CChar]? = try swarmPublicKey.map { _ = try SessionId(from: $0) // Quick way to drop '05' prefix if present - return $0.suffix(64).cString(using: .utf8)?.unsafeCopy() - } - wrapper.addUnsafePointerToCleanup(cSwarmPublicKey) + return $0.suffix(64).cString(using: .utf8) + }.flatMap { $0 } network_send_onion_request_to_snode_destination( network, @@ -279,56 +278,62 @@ class LibSessionNetwork: NetworkType { ) case .server: - network_send_onion_request_to_server_destination( - network, - try wrapper.cServerDestination(destination), - cPayloadBytes, - cPayloadBytes.count, - Int64(floor(requestTimeout * 1000)), - Int64(floor((requestAndPathBuildTimeout ?? 0) * 1000)), - { success, timeout, statusCode, cHeaders, cHeaderVals, headerLen, dataPtr, dataLen, ctx in - let headers: [String: String] = CallbackWrapper - .headers(cHeaders, cHeaderVals, headerLen) - let data: Data? = dataPtr.map { Data(bytes: $0, count: dataLen) } - CallbackWrapper.run(ctx, (success, timeout, Int(statusCode), headers, data)) - }, - wrapper.unsafePointer() - ) + try destination.withUnsafePointer { cServerDestination in + network_send_onion_request_to_server_destination( + network, + cServerDestination, + cPayloadBytes, + cPayloadBytes.count, + Int64(floor(requestTimeout * 1000)), + Int64(floor((requestAndPathBuildTimeout ?? 0) * 1000)), + { success, timeout, statusCode, cHeaders, cHeaderVals, headerLen, dataPtr, dataLen, ctx in + let headers: [String: String] = CallbackWrapper + .headers(cHeaders, cHeaderVals, headerLen) + let data: Data? = dataPtr.map { Data(bytes: $0, count: dataLen) } + CallbackWrapper.run(ctx, (success, timeout, Int(statusCode), headers, data)) + }, + wrapper.unsafePointer() + ) + } case .serverUpload(_, let fileName): guard !cPayloadBytes.isEmpty else { throw NetworkError.invalidPreparedRequest } - network_upload_to_server( - network, - try wrapper.cServerDestination(destination), - cPayloadBytes, - cPayloadBytes.count, - fileName?.cString(using: .utf8), - Int64(floor(requestTimeout * 1000)), - Int64(floor((requestAndPathBuildTimeout ?? 0) * 1000)), - { success, timeout, statusCode, cHeaders, cHeaderVals, headerLen, dataPtr, dataLen, ctx in - let headers: [String: String] = CallbackWrapper - .headers(cHeaders, cHeaderVals, headerLen) - let data: Data? = dataPtr.map { Data(bytes: $0, count: dataLen) } - CallbackWrapper.run(ctx, (success, timeout, Int(statusCode), headers, data)) - }, - wrapper.unsafePointer() - ) + try destination.withUnsafePointer { cServerDestination in + network_upload_to_server( + network, + cServerDestination, + cPayloadBytes, + cPayloadBytes.count, + fileName?.cString(using: .utf8), + Int64(floor(requestTimeout * 1000)), + Int64(floor((requestAndPathBuildTimeout ?? 0) * 1000)), + { success, timeout, statusCode, cHeaders, cHeaderVals, headerLen, dataPtr, dataLen, ctx in + let headers: [String: String] = CallbackWrapper + .headers(cHeaders, cHeaderVals, headerLen) + let data: Data? = dataPtr.map { Data(bytes: $0, count: dataLen) } + CallbackWrapper.run(ctx, (success, timeout, Int(statusCode), headers, data)) + }, + wrapper.unsafePointer() + ) + } case .serverDownload: - network_download_from_server( - network, - try wrapper.cServerDestination(destination), - Int64(floor(requestTimeout * 1000)), - Int64(floor((requestAndPathBuildTimeout ?? 0) * 1000)), - { success, timeout, statusCode, cHeaders, cHeaderVals, headerLen, dataPtr, dataLen, ctx in - let headers: [String: String] = CallbackWrapper - .headers(cHeaders, cHeaderVals, headerLen) - let data: Data? = dataPtr.map { Data(bytes: $0, count: dataLen) } - CallbackWrapper.run(ctx, (success, timeout, Int(statusCode), headers, data)) - }, - wrapper.unsafePointer() - ) + try destination.withUnsafePointer { cServerDestination in + network_download_from_server( + network, + cServerDestination, + Int64(floor(requestTimeout * 1000)), + Int64(floor((requestAndPathBuildTimeout ?? 0) * 1000)), + { success, timeout, statusCode, cHeaders, cHeaderVals, headerLen, dataPtr, dataLen, ctx in + let headers: [String: String] = CallbackWrapper + .headers(cHeaders, cHeaderVals, headerLen) + let data: Data? = dataPtr.map { Data(bytes: $0, count: dataLen) } + CallbackWrapper.run(ctx, (success, timeout, Int(statusCode), headers, data)) + }, + wrapper.unsafePointer() + ) + } case .cached(let success, let timeout, let statusCode, let headers, let data): wrapper.run((success, timeout, statusCode, headers, data)) @@ -406,13 +411,6 @@ class LibSessionNetwork: NetworkType { private extension LibSessionNetwork { class CallbackWrapper { public let resultPublisher: CurrentValueSubject = CurrentValueSubject(nil) - private var pointersToDeallocate: [UnsafeRawPointer?] = [] - - // MARK: - Initialization - - deinit { - pointersToDeallocate.forEach { $0?.deallocate() } - } // MARK: - Functions @@ -430,10 +428,6 @@ private extension LibSessionNetwork { public func unsafePointer() -> UnsafeMutableRawPointer { Unmanaged.passRetained(self).toOpaque() } - public func addUnsafePointerToCleanup(_ pointer: UnsafePointer?) { - pointersToDeallocate.append(UnsafeRawPointer(pointer)) - } - public func run(_ output: Output) { resultPublisher.send(output) } @@ -554,32 +548,21 @@ extension network_service_node: @retroactive CAccessible, @retroactive CMutable // MARK: - Convenience -private extension LibSessionNetwork.CallbackWrapper { - static func headers( - _ cHeaders: UnsafeMutablePointer?>?, - _ cHeaderVals: UnsafeMutablePointer?>?, - _ count: Int - ) -> [String: String] { - let headers: [String] = [String](pointer: cHeaders, count: count, defaultValue: []) - let headerVals: [String] = [String](pointer: cHeaderVals, count: count, defaultValue: []) - - return zip(headers, headerVals) - .reduce(into: [:]) { result, next in result[next.0] = next.1 } - } - - func cServerDestination(_ destination: Network.Destination) throws -> network_server_destination { +private extension Network.Destination { + func withUnsafePointer(_ body: (network_server_destination) throws -> Result) throws -> Result { let method: HTTPMethod let url: URL let headers: [HTTPHeader: String]? let x25519PublicKey: String - switch destination { + switch self { case .snode, .randomSnode, .randomSnodeLatestNetworkTimeTarget, .cached: throw NetworkError.invalidPreparedRequest case .server(let info), .serverUpload(let info, _), .serverDownload(let info): method = info.method url = try info.url headers = info.headers - x25519PublicKey = info.x25519PublicKey + x25519PublicKey = String(info.x25519PublicKey + .suffix(64)) // Quick way to drop '05' prefix if present } guard let host: String = url.host else { throw NetworkError.invalidURL } @@ -587,75 +570,56 @@ private extension LibSessionNetwork.CallbackWrapper { throw LibSessionError.invalidCConversion } - let headerInfo: [(key: String, value: String)]? = headers?.map { ($0.key, $0.value) } - - // Handle the more complicated type conversions first - let cHeaderKeysContent: [UnsafePointer?] = (try? ((headerInfo ?? []) - .map { $0.key.cString(using: .utf8) } - .unsafeCopyCStringArray())) - .defaulting(to: []) - let cHeaderValuesContent: [UnsafePointer?] = (try? ((headerInfo ?? []) - .map { $0.value.cString(using: .utf8) } - .unsafeCopyCStringArray())) - .defaulting(to: []) - - guard - cHeaderKeysContent.count == cHeaderValuesContent.count, - cHeaderKeysContent.allSatisfy({ $0 != nil }), - cHeaderValuesContent.allSatisfy({ $0 != nil }) - else { - cHeaderKeysContent.forEach { $0?.deallocate() } - cHeaderValuesContent.forEach { $0?.deallocate() } - throw LibSessionError.invalidCConversion + let targetScheme: String = (url.scheme ?? "https") + let endpoint: String = url.path + .appending(url.query.map { value in "?\(value)" } ?? "") + let port: UInt16 = UInt16(url.port ?? (targetScheme == "https" ? 443 : 80)) + let headerKeys: [String] = (headers?.map { $0.key } ?? []) + let headerValues: [String] = (headers?.map { $0.value } ?? []) + let headersSize = headerKeys.count + + // Use scoped closure to avoid manual memory management (crazy nesting but it ends up safer) + return try method.rawValue.withCString { cMethodPtr in + try targetScheme.withCString { cTargetSchemePtr in + try host.withCString { cHostPtr in + try endpoint.withCString { cEndpointPtr in + try x25519PublicKey.withCString { cX25519PubkeyPtr in + try headerKeys.withUnsafeCStrArray { headerKeysPtr in + try headerValues.withUnsafeCStrArray { headerValuesPtr in + let cServerDest = network_server_destination( + method: cMethodPtr, + protocol: cTargetSchemePtr, + host: cHostPtr, + endpoint: cEndpointPtr, + port: port, + x25519_pubkey: cX25519PubkeyPtr, + headers: headerKeysPtr.baseAddress, + header_values: headerValuesPtr.baseAddress, + headers_size: headersSize + ) + + return try body(cServerDest) + } + } + } + } + } + } } + } +} + +private extension LibSessionNetwork.CallbackWrapper { + static func headers( + _ cHeaders: UnsafePointer?>?, + _ cHeaderVals: UnsafePointer?>?, + _ count: Int + ) -> [String: String] { + let headers: [String] = ([String](cStringArray: cHeaders, count: count) ?? []) + let headerVals: [String] = ([String](cStringArray: cHeaderVals, count: count) ?? []) - // Convert the other types - let targetScheme: String = (url.scheme ?? "https") - let cMethod: UnsafePointer? = method.rawValue - .cString(using: .utf8)? - .unsafeCopy() - let cTargetScheme: UnsafePointer? = targetScheme - .cString(using: .utf8)? - .unsafeCopy() - let cHost: UnsafePointer? = host - .cString(using: .utf8)? - .unsafeCopy() - let cEndpoint: UnsafePointer? = url.path - .appending(url.query.map { value in "?\(value)" }) - .cString(using: .utf8)? - .unsafeCopy() - let cX25519Pubkey: UnsafePointer? = x25519PublicKey - .suffix(64) // Quick way to drop '05' prefix if present - .cString(using: .utf8)? - .unsafeCopy() - let cHeaderKeys: UnsafeMutablePointer?>? = cHeaderKeysContent - .unsafeCopy() - let cHeaderValues: UnsafeMutablePointer?>? = cHeaderValuesContent - .unsafeCopy() - let cServerDestination = network_server_destination( - method: cMethod, - protocol: cTargetScheme, - host: cHost, - endpoint: cEndpoint, - port: UInt16(url.port ?? (targetScheme == "https" ? 443 : 80)), - x25519_pubkey: cX25519Pubkey, - headers: cHeaderKeys, - header_values: cHeaderValues, - headers_size: (headerInfo ?? []).count - ) - - // Add a cleanup callback to deallocate the header arrays - self.addUnsafePointerToCleanup(cMethod) - self.addUnsafePointerToCleanup(cTargetScheme) - self.addUnsafePointerToCleanup(cHost) - self.addUnsafePointerToCleanup(cEndpoint) - self.addUnsafePointerToCleanup(cX25519Pubkey) - cHeaderKeysContent.forEach { self.addUnsafePointerToCleanup($0) } - cHeaderValuesContent.forEach { self.addUnsafePointerToCleanup($0) } - self.addUnsafePointerToCleanup(cHeaderKeys) - self.addUnsafePointerToCleanup(cHeaderValues) - - return cServerDestination + return zip(headers, headerVals) + .reduce(into: [:]) { result, next in result[next.0] = next.1 } } } diff --git a/SessionSnodeKitTests/Types/BatchRequestSpec.swift b/SessionSnodeKitTests/Types/BatchRequestSpec.swift index 2f12ec774..36b33bd18 100644 --- a/SessionSnodeKitTests/Types/BatchRequestSpec.swift +++ b/SessionSnodeKitTests/Types/BatchRequestSpec.swift @@ -116,7 +116,7 @@ class BatchRequestSpec: QuickSpec { let requestData: Data? = try? JSONEncoder().encode(request) let requestJson: [[String: Any]]? = requestData - .map { try? JSONSerialization.jsonObject(with: $0) as? [[String: Any]] } + .map { (try? JSONSerialization.jsonObject(with: $0)) as? [[String: Any]] } expect(requestJson?.first?["path"] as? String).to(equal("/endpoint1")) expect(requestJson?.first?["method"] as? String).to(equal("GET")) expect(requestJson?.first?["b64"] as? String).to(equal("testBody")) diff --git a/SessionUtilitiesKit/General/Collection+Utilities.swift b/SessionUtilitiesKit/General/Collection+Utilities.swift index e34b16406..8ddefb4f7 100644 --- a/SessionUtilitiesKit/General/Collection+Utilities.swift +++ b/SessionUtilitiesKit/General/Collection+Utilities.swift @@ -2,81 +2,65 @@ import Foundation -extension Collection { - public subscript(ifValid index: Index) -> Iterator.Element? { - return self.indices.contains(index) ? self[index] : nil - } -} - -public extension Collection { - /// This creates an UnsafeMutableBufferPointer to access data in memory directly. This result pointer provides no automated - /// memory management so after use you are responsible for handling the life cycle and need to call `deallocate()`. - func unsafeCopy() -> UnsafeMutableBufferPointer { - let copy = UnsafeMutableBufferPointer.allocate(capacity: self.underestimatedCount) - _ = copy.initialize(from: self) - return copy - } - - /// This creates an UnsafePointer to access data in memory directly. This result pointer provides no automated - /// memory management so after use you are responsible for handling the life cycle and need to call `deallocate()`. - func unsafeCopy() -> UnsafePointer? { - let copy = UnsafeMutableBufferPointer.allocate(capacity: self.underestimatedCount) - _ = copy.initialize(from: self) - return UnsafePointer(copy.baseAddress) - } - - /// This creates an UnsafePointer to access data in memory directly. This result pointer provides no automated - /// memory management so after use you are responsible for handling the life cycle and need to call `deallocate()`. - func unsafeCopy() -> UnsafeMutablePointer? { - let copy = UnsafeMutableBufferPointer.allocate(capacity: self.underestimatedCount) - _ = copy.initialize(from: self) - return UnsafeMutablePointer(copy.baseAddress) - } -} - -public extension Collection where Element == [CChar]? { - /// This creates an array of UnsafePointer types to access data of the C strings in memory. This array provides no automated - /// memory management of it's children so after use you are responsible for handling the life cycle of the child elements and - /// need to call `deallocate()` on each child. - func unsafeCopyCStringArray() throws -> [UnsafePointer?] { - return try self.map { value in - guard let value: [CChar] = value else { return nil } - - let copy = UnsafeMutableBufferPointer.allocate(capacity: value.underestimatedCount) - var remaining: (unwritten: Array.Iterator, index: Int) = copy.initialize(from: value) - guard remaining.unwritten.next() == nil else { throw LibSessionError.invalidCConversion } +public extension Collection where Element == String { + func withUnsafeCStrArray( + _ body: (UnsafeBufferPointer?>) throws -> R + ) throws -> R { + let cCharArrays: [[CChar]] = try map { swiftStr in + guard let cChars = swiftStr.cString(using: .utf8) else { + throw LibSessionError.invalidCConversion + } - return UnsafePointer(copy.baseAddress) + return cChars } - } -} - -public extension Collection where Element == [CChar] { - /// This creates an array of UnsafePointer types to access data of the C strings in memory. This array provides no automated - /// memory management of it's children so after use you are responsible for handling the life cycle of the child elements and - /// need to call `deallocate()` on each child. - func unsafeCopyCStringArray() throws -> [UnsafePointer?] { - return try self.map { value in - let copy = UnsafeMutableBufferPointer.allocate(capacity: value.underestimatedCount) - var remaining: (unwritten: Array.Iterator, index: Int) = copy.initialize(from: value) - guard remaining.unwritten.next() == nil else { throw LibSessionError.invalidCConversion } + + func getPointers( + remainingArrays: ArraySlice<[CChar]>, + collectedPointers: [UnsafePointer?] + ) throws -> R { + guard let currentArray = remainingArrays.first else { + return try collectedPointers.withUnsafeBufferPointer { collectedPointersBuffer in + try body(collectedPointersBuffer) + } + } - return UnsafePointer(copy.baseAddress) + return try currentArray.withUnsafeBufferPointer { currentBufferPtr in + try getPointers( + remainingArrays: remainingArrays.dropFirst(), + collectedPointers: collectedPointers + [currentBufferPtr.baseAddress] + ) + } } + + return try getPointers(remainingArrays: ArraySlice(cCharArrays), collectedPointers: []) } } -public extension Collection where Element == [UInt8] { - /// This creates an array of UnsafePointer types to access data of the C strings in memory. This array provides no automated - /// memory management of it's children so after use you are responsible for handling the life cycle of the child elements and - /// need to call `deallocate()` on each child. - func unsafeCopyUInt8Array() throws -> [UnsafePointer?] { - return try self.map { value in - let copy = UnsafeMutableBufferPointer.allocate(capacity: value.underestimatedCount) - var remaining: (unwritten: Array.Iterator, index: Int) = copy.initialize(from: value) - guard remaining.unwritten.next() == nil else { throw LibSessionError.invalidCConversion } - - return UnsafePointer(copy.baseAddress) +public extension Collection where Element == [UInt8]? { + func withUnsafeUInt8CArray( + _ body: (UnsafeBufferPointer?>) throws -> R + ) throws -> R { + func processNext( + iterator: inout I, + collectedPointers: [UnsafePointer?] + ) throws -> R where I.Element == Element { + if let optionalElement: [UInt8]? = iterator.next() { + if let array: [UInt8] = optionalElement { + return try array.withUnsafeBufferPointer { bufferPtr in + try processNext(iterator: &iterator, collectedPointers: collectedPointers + [bufferPtr.baseAddress]) + } + } + + // It's a nil element in the input collection, add a nil pointer and recurse + return try processNext(iterator: &iterator, collectedPointers: collectedPointers + [nil]) + } else { + return try collectedPointers.withUnsafeBufferPointer { pointersBuffer in + try body(pointersBuffer) + } + } } + + var iterator = makeIterator() + return try processNext(iterator: &iterator, collectedPointers: []) } } diff --git a/SessionUtilitiesKit/LibSession/Utilities/TypeConversion+Utilities.swift b/SessionUtilitiesKit/LibSession/Utilities/TypeConversion+Utilities.swift index 4e87fbfd4..300b7f774 100644 --- a/SessionUtilitiesKit/LibSession/Utilities/TypeConversion+Utilities.swift +++ b/SessionUtilitiesKit/LibSession/Utilities/TypeConversion+Utilities.swift @@ -21,68 +21,37 @@ public extension String { // MARK: - Array public extension Array where Element == String { - init?(pointer: UnsafeMutablePointer?>?, count: Int?) { + init?(cStringArray: UnsafePointer?>?, count: Int?) { /// If `count` was provided but is `0` then accessing the pointer could crash (as it could be bad memory) so just return an empty array - guard let count: Int = count else { return nil } - guard count > 0 else { - self = [] - return - } + guard + let cStringArray: UnsafePointer?> = cStringArray, + let count: Int = count + else { return nil } - self.init(pointee: pointer.map { $0.pointee.map { UnsafePointer($0) } }, count: count) - } - - init?(pointer: UnsafeMutablePointer?>?, count: Int?) { - /// If `count` was provided but is `0` then accessing the pointer could crash (as it could be bad memory) so just return an empty array - guard let count: Int = count else { return nil } - guard count > 0 else { - self = [] - return - } + self.init() + self.reserveCapacity(count) - self.init(pointee: pointer.map { $0.pointee }, count: count) - } - - init( - pointer: UnsafeMutablePointer?>?, - count: Int?, - defaultValue: [String] - ) { - self = ([String](pointer: pointer, count: count) ?? defaultValue) - } - - init( - pointer: UnsafeMutablePointer?>?, - count: Int?, - defaultValue: [String] - ) { - self = ([String](pointer: pointer, count: count) ?? defaultValue) + for i in 0.. = cStringArray[i] { + self.append(String(cString: cStringPtr)) + } + } } - init?( - pointee: UnsafePointer?, - count: Int? - ) { + init?(cStringArray: UnsafeMutablePointer?>?, count: Int?) { + /// If `count` was provided but is `0` then accessing the pointer could crash (as it could be bad memory) so just return an empty array guard - let count: Int = count, - let pointee: UnsafePointer = pointee + let cStringArray: UnsafeMutablePointer?> = cStringArray, + let count: Int = count else { return nil } - /// If `count` was provided but is `0` then accessing the pointer could crash (as it could be bad memory) so just return an empty array - guard count > 0 else { - self = [] - return - } + self.init() + self.reserveCapacity(count) - self = (0.. = cStringArray[i] { + self.append(String(cString: cStringPtr)) + } } } } diff --git a/SessionUtilitiesKitTests/LibSession/Utilities/TypeConversionUtilitiesSpec.swift b/SessionUtilitiesKitTests/LibSession/Utilities/TypeConversionUtilitiesSpec.swift index 5a7324544..c331fb408 100644 --- a/SessionUtilitiesKitTests/LibSession/Utilities/TypeConversionUtilitiesSpec.swift +++ b/SessionUtilitiesKitTests/LibSession/Utilities/TypeConversionUtilitiesSpec.swift @@ -304,71 +304,51 @@ class TypeConversionUtilitiesSpec: QuickSpec { context("when initialised with a 2D C array") { // MARK: ---- returns the correct array it("returns the correct array") { - var test: [CChar] = ( - "Test1".cString(using: .utf8)! + - "Test2".cString(using: .utf8)! + - "Test3AndExtra".cString(using: .utf8)! - ) - let result = test.withUnsafeMutableBufferPointer { ptr in - var mutablePtr = UnsafePointer(ptr.baseAddress) - - return [String](pointer: &mutablePtr, count: 3) - } + var test: [String] = ["Test1", "Test2", "Test3AndExtra"] + let result = try! test.withUnsafeCStrArray { ptr in + return [String](cStringArray: ptr.baseAddress, count: 3) + } expect(result).to(equal(["Test1", "Test2", "Test3AndExtra"])) } // MARK: ---- returns an empty array if given one it("returns an empty array if given one") { - var test = [CChar]() - let result = test.withUnsafeMutableBufferPointer { ptr in - var mutablePtr = UnsafePointer(ptr.baseAddress) - - return [String](pointer: &mutablePtr, count: 0) + var test: [String] = [] + + let result = try! test.withUnsafeCStrArray { ptr in + return [String](cStringArray: ptr.baseAddress, count: 0) } - expect(result).to(equal([])) } // MARK: ---- handles empty strings without issues it("handles empty strings without issues") { - var test: [CChar] = ( - "Test1".cString(using: .utf8)! + - "".cString(using: .utf8)! + - "Test2".cString(using: .utf8)! - ) - let result = test.withUnsafeMutableBufferPointer { ptr in - var mutablePtr = UnsafePointer(ptr.baseAddress) - - return [String](pointer: &mutablePtr, count: 3) + var test: [String] = ["Test1", "", "Test2"] + + let result = try! test.withUnsafeCStrArray { ptr in + return [String](cStringArray: ptr.baseAddress, count: 3) } - expect(result).to(equal(["Test1", "", "Test2"])) } // MARK: ---- returns null when given a null pointer it("returns null when given a null pointer") { - expect([String](pointee: nil, count: 5)).to(beNil()) + expect([String]( + cStringArray: Optional?>>.none, + count: 5) + ).to(beNil()) } // MARK: ---- returns null when given a null count it("returns null when given a null count") { - var test: [CChar] = "Test1".cString(using: .utf8)! - let result = test.withUnsafeMutableBufferPointer { ptr in - var mutablePtr = UnsafeMutablePointer(ptr.baseAddress) - - return [String](pointer: &mutablePtr, count: nil) + var test: [String] = ["Test1"] + + let result = try! test.withUnsafeCStrArray { ptr in + return [String](cStringArray: ptr.baseAddress, count: nil) } - expect(result).to(beNil()) } - - // MARK: ---- returns the default value if given null values - it("returns the default value if given null values") { - let ptr: UnsafeMutablePointer?>? = nil - expect([String](pointer: ptr, count: 5, defaultValue: ["Test"])) - .to(equal(["Test"])) - } } } } From e2204963147ccbecfce98536c8987a71a0c01c0b Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Mon, 7 Apr 2025 12:40:52 +1000 Subject: [PATCH 2/2] Updated to libSession 1.3.1 --- Session.xcodeproj/project.pbxproj | 2 +- .../project.xcworkspace/xcshareddata/swiftpm/Package.resolved | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Session.xcodeproj/project.pbxproj b/Session.xcodeproj/project.pbxproj index b97983858..ea6ecd1a5 100644 --- a/Session.xcodeproj/project.pbxproj +++ b/Session.xcodeproj/project.pbxproj @@ -10063,7 +10063,7 @@ repositoryURL = "https://github.com/session-foundation/libsession-util-spm"; requirement = { kind = exactVersion; - version = 1.3.0; + version = 1.3.1; }; }; FD6A38E72C2A630E00762359 /* XCRemoteSwiftPackageReference "CocoaLumberjack" */ = { diff --git a/Session.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Session.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 8d90dc6d3..bd52b28ed 100644 --- a/Session.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Session.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -51,8 +51,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/session-foundation/libsession-util-spm", "state" : { - "revision" : "51580552aef35e52a3ae99ec72df2d70026ef415", - "version" : "1.3.0" + "revision" : "7c6665ffe8a458f57aa3cf9115873c7be9efe7cb", + "version" : "1.3.1" } }, {