From 82062e2e21f880dfdd58db07997029b6de967c2b Mon Sep 17 00:00:00 2001 From: mojganii Date: Mon, 2 Dec 2024 15:49:28 +0100 Subject: [PATCH] Fix log redaction loading issue --- ios/CHANGELOG.md | 1 + ios/MullvadSettings/SettingsManager.swift | 21 +-- ios/MullvadTypes/StringConversionError.swift | 31 +++++ ios/MullvadVPN.xcodeproj/project.pbxproj | 20 +++ .../Classes/ConsolidatedApplicationLog.swift | 131 +++++++++++------- .../ProblemReportInteractor.swift | 84 +++++++---- .../ProblemReportReviewViewController.swift | 61 ++++---- .../ProblemReportViewController.swift | 4 +- .../Log/ConsolidatedApplicationLogTests.swift | 127 +++++++++++++++++ ios/Shared/LaunchArguments.swift | 5 +- 10 files changed, 362 insertions(+), 123 deletions(-) create mode 100644 ios/MullvadTypes/StringConversionError.swift create mode 100644 ios/MullvadVPNTests/MullvadVPN/Log/ConsolidatedApplicationLogTests.swift diff --git a/ios/CHANGELOG.md b/ios/CHANGELOG.md index 298c24dd7bce..f0f5d12ff53b 100644 --- a/ios/CHANGELOG.md +++ b/ios/CHANGELOG.md @@ -29,6 +29,7 @@ Line wrap the file at 100 chars. Th ## [2024.10 - 2024-11-20] ### Fixed - Removed deadlock when losing connectivity without entering offline state. +- Improved log reporting. ## [2024.9 - 2024-11-07] ### Added diff --git a/ios/MullvadSettings/SettingsManager.swift b/ios/MullvadSettings/SettingsManager.swift index 7a6d4ba2e98f..d414d7ed0aa7 100644 --- a/ios/MullvadSettings/SettingsManager.swift +++ b/ios/MullvadSettings/SettingsManager.swift @@ -47,7 +47,10 @@ public enum SettingsManager { public static func getLastUsedAccount() throws -> String { let data = try store.read(key: .lastUsedAccount) - return String(decoding: data, as: UTF8.self) + guard let result = String(bytes: data, encoding: .utf8) else { + throw StringDecodingError(data: data) + } + return result } public static func setLastUsedAccount(_ string: String?) throws { @@ -220,19 +223,3 @@ public struct UnsupportedSettingsVersionError: LocalizedError { """ } } - -public struct StringDecodingError: LocalizedError { - public let data: Data - - public var errorDescription: String? { - "Failed to decode string from data." - } -} - -public struct StringEncodingError: LocalizedError { - public let string: String - - public var errorDescription: String? { - "Failed to encode string into data." - } -} diff --git a/ios/MullvadTypes/StringConversionError.swift b/ios/MullvadTypes/StringConversionError.swift new file mode 100644 index 000000000000..1bf0bfdbb0f3 --- /dev/null +++ b/ios/MullvadTypes/StringConversionError.swift @@ -0,0 +1,31 @@ +// +// StringDecodingError.swift +// MullvadVPN +// +// Created by Mojgan on 2024-12-02. +// Copyright © 2024 Mullvad VPN AB. All rights reserved. +// + +public struct StringDecodingError: LocalizedError { + public let data: Data + + public init(data: Data) { + self.data = data + } + + public var errorDescription: String? { + "Failed to decode string from data." + } +} + +public struct StringEncodingError: LocalizedError { + public let string: String + + public init(string: String) { + self.string = string + } + + public var errorDescription: String? { + "Failed to encode string into data." + } +} diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj index b80591e9f86c..e1bcdb6dca32 100644 --- a/ios/MullvadVPN.xcodeproj/project.pbxproj +++ b/ios/MullvadVPN.xcodeproj/project.pbxproj @@ -871,6 +871,7 @@ F0164EC32B4C49D30020268D /* ShadowsocksLoaderStub.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0164EC22B4C49D30020268D /* ShadowsocksLoaderStub.swift */; }; F0164ED12B4F2DCB0020268D /* AccessMethodIterator.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0164ED02B4F2DCB0020268D /* AccessMethodIterator.swift */; }; F01DAE332C2B032A00521E46 /* RelaySelection.swift in Sources */ = {isa = PBXBuildFile; fileRef = F01DAE322C2B032A00521E46 /* RelaySelection.swift */; }; + F022EBA62CF0C6AE009484B9 /* ConsolidatedApplicationLog.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5871FB95254ADE4E0051A0A4 /* ConsolidatedApplicationLog.swift */; }; F028A56A2A34D4E700C0CAA3 /* RedeemVoucherViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F028A5692A34D4E700C0CAA3 /* RedeemVoucherViewController.swift */; }; F028A56C2A34D8E600C0CAA3 /* AddCreditSucceededViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F028A56B2A34D8E600C0CAA3 /* AddCreditSucceededViewController.swift */; }; F02F41A02B9723AF00625A4F /* AddLocationsViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F02F419A2B9723AE00625A4F /* AddLocationsViewController.swift */; }; @@ -946,6 +947,8 @@ F09D04C12AF39EA2003D4F89 /* OutgoingConnectionService.swift in Sources */ = {isa = PBXBuildFile; fileRef = F09D04BC2AEBB7C5003D4F89 /* OutgoingConnectionService.swift */; }; F0A086902C22D6A700BF83E7 /* TunnelSettingsStrategyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0A0868F2C22D6A700BF83E7 /* TunnelSettingsStrategyTests.swift */; }; F0A1638A2C47B77300592300 /* ServerRelaysResponse+Stubs.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ACE3342BE51745006D5333 /* ServerRelaysResponse+Stubs.swift */; }; + F0A7EBB22CEF6C79005BB671 /* ConsolidatedApplicationLogTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0A7EBB12CEF6C79005BB671 /* ConsolidatedApplicationLogTests.swift */; }; + F0A7EBB62CF092CC005BB671 /* ApplicationConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58BFA5CB22A7CE1F00A6173D /* ApplicationConfiguration.swift */; }; F0ACE30D2BE4E478006D5333 /* MullvadMockData.h in Headers */ = {isa = PBXBuildFile; fileRef = F0ACE30A2BE4E478006D5333 /* MullvadMockData.h */; settings = {ATTRIBUTES = (Public, ); }; }; F0ACE3102BE4E478006D5333 /* MullvadMockData.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = F0ACE3082BE4E478006D5333 /* MullvadMockData.framework */; }; F0ACE3112BE4E478006D5333 /* MullvadMockData.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = F0ACE3082BE4E478006D5333 /* MullvadMockData.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; @@ -964,6 +967,7 @@ F0ACE3372BE517F1006D5333 /* ServerRelaysResponse+Stubs.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ACE3342BE51745006D5333 /* ServerRelaysResponse+Stubs.swift */; }; F0ADC3722CD3AD1600A1AD97 /* ChipCollectionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ADC3712CD3AD1600A1AD97 /* ChipCollectionView.swift */; }; F0ADC3742CD3C47400A1AD97 /* ChipFlowLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ADC3732CD3C47400A1AD97 /* ChipFlowLayout.swift */; }; + F0ADF1CD2CFDFF3100299F09 /* StringConversionError.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0ADF1CC2CFDFF3100299F09 /* StringConversionError.swift */; }; F0B0E6972AFE6E7E001DC66B /* XCTest+Async.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0B0E6962AFE6E7E001DC66B /* XCTest+Async.swift */; }; F0B894EF2BF751C500817A42 /* RelayWithLocation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0B894EE2BF751C500817A42 /* RelayWithLocation.swift */; }; F0B894F12BF751E300817A42 /* RelayWithDistance.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0B894F02BF751E300817A42 /* RelayWithDistance.swift */; }; @@ -2166,12 +2170,14 @@ F09D04BF2AF39D63003D4F89 /* OutgoingConnectionServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OutgoingConnectionServiceTests.swift; sourceTree = ""; }; F0A0868F2C22D6A700BF83E7 /* TunnelSettingsStrategyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelSettingsStrategyTests.swift; sourceTree = ""; }; F0A163882C47B46300592300 /* SingleHopEphemeralPeerExchangerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SingleHopEphemeralPeerExchangerTests.swift; sourceTree = ""; }; + F0A7EBB12CEF6C79005BB671 /* ConsolidatedApplicationLogTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConsolidatedApplicationLogTests.swift; sourceTree = ""; }; F0ACE3082BE4E478006D5333 /* MullvadMockData.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = MullvadMockData.framework; sourceTree = BUILT_PRODUCTS_DIR; }; F0ACE30A2BE4E478006D5333 /* MullvadMockData.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MullvadMockData.h; sourceTree = ""; }; F0ACE32E2BE4EA8B006D5333 /* MockProxyFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockProxyFactory.swift; sourceTree = ""; }; F0ACE3342BE51745006D5333 /* ServerRelaysResponse+Stubs.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "ServerRelaysResponse+Stubs.swift"; sourceTree = ""; }; F0ADC3712CD3AD1600A1AD97 /* ChipCollectionView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChipCollectionView.swift; sourceTree = ""; }; F0ADC3732CD3C47400A1AD97 /* ChipFlowLayout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChipFlowLayout.swift; sourceTree = ""; }; + F0ADF1CC2CFDFF3100299F09 /* StringConversionError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StringConversionError.swift; sourceTree = ""; }; F0B0E6962AFE6E7E001DC66B /* XCTest+Async.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "XCTest+Async.swift"; sourceTree = ""; }; F0B894EE2BF751C500817A42 /* RelayWithLocation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayWithLocation.swift; sourceTree = ""; }; F0B894F02BF751E300817A42 /* RelayWithDistance.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayWithDistance.swift; sourceTree = ""; }; @@ -2425,6 +2431,7 @@ 440E9EF02BDA93CB00B1FD11 /* MullvadVPN */ = { isa = PBXGroup; children = ( + F0A7EBB02CEF6C5F005BB671 /* Log */, 440E9EF62BDA957300B1FD11 /* Classes */, 440E9F002BDA997C00B1FD11 /* Extensions */, 440E9EFB2BDA97C600B1FD11 /* GeneralAPIs */, @@ -2704,6 +2711,7 @@ 5898D2AF2902A67C00EB5EBA /* RelayLocation.swift */, 581DA2722A1E227D0046ED47 /* RESTTypes.swift */, 58F1311427E0B2AB007AC5BC /* Result+Extensions.swift */, + F0ADF1CC2CFDFF3100299F09 /* StringConversionError.swift */, A91614D02B108D1B00F416EB /* TransportLayer.swift */, 58E511E028DDB7F100B0BCDE /* WrappingError.swift */, ); @@ -4205,6 +4213,14 @@ path = GeneralAPIs; sourceTree = ""; }; + F0A7EBB02CEF6C5F005BB671 /* Log */ = { + isa = PBXGroup; + children = ( + F0A7EBB12CEF6C79005BB671 /* ConsolidatedApplicationLogTests.swift */, + ); + path = Log; + sourceTree = ""; + }; F0ACE3092BE4E478006D5333 /* MullvadMockData */ = { isa = PBXGroup; children = ( @@ -5378,6 +5394,7 @@ A9A5FA3B2ACB05910083449F /* UIMetrics.swift in Sources */, 58B07C182AEFDD6C00A09625 /* StoreTransactionLog.swift in Sources */, A9A5FA382ACB05600083449F /* InputTextFormatter.swift in Sources */, + F022EBA62CF0C6AE009484B9 /* ConsolidatedApplicationLog.swift in Sources */, A9A5FA372ACB052D0083449F /* ApplicationTarget.swift in Sources */, A9A5F9E12ACB05160083449F /* AddressCacheTracker.swift in Sources */, A9A5F9E22ACB05160083449F /* BackgroundTask.swift in Sources */, @@ -5488,6 +5505,7 @@ A9A5FA282ACB05160083449F /* WgKeyRotation.swift in Sources */, 449872E42B7CB96300094DDC /* TunnelSettingsUpdateTests.swift in Sources */, A9A5FA292ACB05160083449F /* AddressCacheTests.swift in Sources */, + F0A7EBB22CEF6C79005BB671 /* ConsolidatedApplicationLogTests.swift in Sources */, A9B6AC182ADE8F4300F7802A /* MigrationManagerTests.swift in Sources */, 7A9BE5AB2B909A1700E2A7D0 /* LocationDataSourceProtocol.swift in Sources */, A9A5FA2A2ACB05160083449F /* CoordinatesTests.swift in Sources */, @@ -5515,6 +5533,7 @@ A9A5FA342ACB05160083449F /* StringTests.swift in Sources */, 7A52F96C2C17450C00B133B9 /* RelaySelectorWrapperTests.swift in Sources */, A9A5FA352ACB05160083449F /* WgKeyRotationTests.swift in Sources */, + F0A7EBB62CF092CC005BB671 /* ApplicationConfiguration.swift in Sources */, 7AB4CCB92B69097E006037F5 /* IPOverrideTests.swift in Sources */, A9A5FA362ACB05160083449F /* TunnelManagerTests.swift in Sources */, ); @@ -6120,6 +6139,7 @@ A9E031782ACB09930095D843 /* BackgroundTaskProvider.swift in Sources */, 58D2240B294C90210029F5F8 /* Cancellable.swift in Sources */, 58D2240C294C90210029F5F8 /* WrappingError.swift in Sources */, + F0ADF1CD2CFDFF3100299F09 /* StringConversionError.swift in Sources */, A9A8A8EB2A262AB30086D569 /* FileCache.swift in Sources */, A90C48692C36BF3900DCB94C /* TunnelProvider.swift in Sources */, 58D2240D294C90210029F5F8 /* CustomErrorDescriptionProtocol.swift in Sources */, diff --git a/ios/MullvadVPN/Classes/ConsolidatedApplicationLog.swift b/ios/MullvadVPN/Classes/ConsolidatedApplicationLog.swift index 48dbf6a72349..6e96912c5856 100644 --- a/ios/MullvadVPN/Classes/ConsolidatedApplicationLog.swift +++ b/ios/MullvadVPN/Classes/ConsolidatedApplicationLog.swift @@ -15,6 +15,7 @@ private let kRedactedContainerPlaceholder = "[REDACTED CONTAINER PATH]" class ConsolidatedApplicationLog: TextOutputStreamable { typealias Metadata = KeyValuePairs + private let bufferSize: UInt64 enum MetadataKey: String { case id, os @@ -26,18 +27,21 @@ class ConsolidatedApplicationLog: TextOutputStreamable { let content: String } - let redactCustomStrings: [String] + let redactCustomStrings: [String]? let applicationGroupContainers: [URL] let metadata: Metadata + private let logQueue = DispatchQueue(label: "com.mullvad.consolidation.logs.queue") private var logs: [LogAttachment] = [] init( - redactCustomStrings: [String], - redactContainerPathsForSecurityGroupIdentifiers securityGroupIdentifiers: [String] + redactCustomStrings: [String]? = nil, + redactContainerPathsForSecurityGroupIdentifiers securityGroupIdentifiers: [String], + bufferSize: UInt64 ) { metadata = Self.makeMetadata() self.redactCustomStrings = redactCustomStrings + self.bufferSize = bufferSize applicationGroupContainers = securityGroupIdentifiers .compactMap { securityGroupIdentifier -> URL? in @@ -46,38 +50,62 @@ class ConsolidatedApplicationLog: TextOutputStreamable { } } - func addLogFiles(fileURLs: [URL]) { - for fileURL in fileURLs { - addSingleLogFile(fileURL) + func addLogFiles(fileURLs: [URL], completion: (() -> Void)? = nil) { + logQueue.async(flags: .barrier) { + for fileURL in fileURLs { + self.addSingleLogFile(fileURL) + } + DispatchQueue.main.async { + completion?() + } } } - func addError(message: String, error: String) { + func addError(message: String, error: String, completion: (() -> Void)? = nil) { let redactedError = redact(string: error) - - logs.append(LogAttachment(label: message, content: redactedError)) + logQueue.async(flags: .barrier) { + self.logs.append(LogAttachment(label: message, content: redactedError)) + DispatchQueue.main.async { + completion?() + } + } } var string: String { - var body = "" - write(to: &body) - return body + var logsCopy: [LogAttachment] = [] + var metadataCopy: Metadata = [:] + logQueue.sync { + logsCopy = logs + metadataCopy = metadata + } + guard !logsCopy.isEmpty else { return "" } + return formatLog(logs: logsCopy, metadata: metadataCopy) } func write(to stream: inout some TextOutputStream) { - print("System information:", to: &stream) - for (key, value) in metadata { - print("\(key.rawValue): \(value)", to: &stream) + var logsCopy: [LogAttachment] = [] + var metadataCopy: Metadata = [:] + logQueue.sync { + logsCopy = logs + metadataCopy = metadata } - print("", to: &stream) + let localOutput = formatLog(logs: logsCopy, metadata: metadataCopy) + stream.write(localOutput) + } + private func formatLog(logs: [LogAttachment], metadata: Metadata) -> String { + var result = "System information:\n" + for (key, value) in metadata { + result += "\(key.rawValue): \(value)\n" + } + result += "\n" for attachment in logs { - print(kLogDelimiter, to: &stream) - print(attachment.label, to: &stream) - print(kLogDelimiter, to: &stream) - print(attachment.content, to: &stream) - print("", to: &stream) + result += "\(kLogDelimiter)\n" + result += "\(attachment.label)\n" + result += "\(kLogDelimiter)\n" + result += "\(attachment.content)\n\n" } + return result } private func addSingleLogFile(_ fileURL: URL) { @@ -92,10 +120,11 @@ class ConsolidatedApplicationLog: TextOutputStreamable { let path = fileURL.path let redactedPath = redact(string: path) - if let lossyString = Self.readFileLossy(path: path, maxBytes: ApplicationConfiguration.logMaximumFileSize) { + if let lossyString = readFileLossy(path: path, maxBytes: bufferSize) { let redactedString = redact(string: lossyString) - - logs.append(LogAttachment(label: redactedPath, content: redactedString)) + logQueue.async(flags: .barrier) { + self.logs.append(LogAttachment(label: redactedPath, content: redactedString)) + } } else { addError(message: redactedPath, error: "Log file does not exist: \(path).") } @@ -113,7 +142,7 @@ class ConsolidatedApplicationLog: TextOutputStreamable { ] } - private static func readFileLossy(path: String, maxBytes: UInt64) -> String? { + private func readFileLossy(path: String, maxBytes: UInt64) -> String? { guard let fileHandle = FileHandle(forReadingAtPath: path) else { return nil } @@ -125,35 +154,37 @@ class ConsolidatedApplicationLog: TextOutputStreamable { fileHandle.seek(toFileOffset: 0) } - let data = fileHandle.readData(ofLength: Int(ApplicationConfiguration.logMaximumFileSize)) let replacementCharacter = Character(UTF8.decode(UTF8.encodedReplacementCharacter)) - let lossyString = String( - String(decoding: data, as: UTF8.self) - .drop { ch in - // Drop leading replacement characters produced when decoding data - ch == replacementCharacter - } - ) - - return lossyString + if let data = try? fileHandle.read(upToCount: Int(bufferSize)), + let lossyString = String(bytes: data, encoding: .utf8) { + let resultString = lossyString.drop { ch in + // Drop leading replacement characters produced when decoding data + ch == replacementCharacter + } + return String(resultString) + } else { + return nil + } } - private func redactCustomStrings(string: String) -> String { - redactCustomStrings.reduce(string) { resultString, redact -> String in + private func redactCustomStrings(in string: String) -> String { + guard let customStrings = redactCustomStrings, + !customStrings.isEmpty else { + return string + } + return customStrings.reduce(string) { resultString, redact in resultString.replacingOccurrences(of: redact, with: kRedactedPlaceholder) } } private func redact(string: String) -> String { - [ - redactContainerPaths, - Self.redactAccountNumber, - Self.redactIPv4Address, - Self.redactIPv6Address, - redactCustomStrings, - ].reduce(string) { resultString, transform -> String in - transform(resultString) - } + var result = string + result = redactContainerPaths(string: result) + result = redactAccountNumber(string: result) + result = redactIPv4Address(string: result) + result = redactIPv6Address(string: result) + result = redactCustomStrings(in: result) + return result } private func redactContainerPaths(string: String) -> String { @@ -165,7 +196,7 @@ class ConsolidatedApplicationLog: TextOutputStreamable { } } - private static func redactAccountNumber(string: String) -> String { + private func redactAccountNumber(string: String) -> String { redact( // swiftlint:disable:next force_try regularExpression: try! NSRegularExpression(pattern: #"\d{16}"#), @@ -174,7 +205,7 @@ class ConsolidatedApplicationLog: TextOutputStreamable { ) } - private static func redactIPv4Address(string: String) -> String { + private func redactIPv4Address(string: String) -> String { redact( regularExpression: NSRegularExpression.ipv4RegularExpression, string: string, @@ -182,7 +213,7 @@ class ConsolidatedApplicationLog: TextOutputStreamable { ) } - private static func redactIPv6Address(string: String) -> String { + private func redactIPv6Address(string: String) -> String { redact( regularExpression: NSRegularExpression.ipv6RegularExpression, string: string, @@ -190,7 +221,7 @@ class ConsolidatedApplicationLog: TextOutputStreamable { ) } - private static func redact( + private func redact( regularExpression: NSRegularExpression, string: String, replacementString: String diff --git a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift index bbc740f453ec..616ea7d081fb 100644 --- a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift +++ b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift @@ -14,51 +14,79 @@ import Operations final class ProblemReportInteractor { private let apiProxy: APIQuerying private let tunnelManager: TunnelManager + private let consolidatedLog: ConsolidatedApplicationLog + private var reportedString = "" - private lazy var consolidatedLog: ConsolidatedApplicationLog = { - let securityGroupIdentifier = ApplicationConfiguration.securityGroupIdentifier - let redactStrings = [tunnelManager.deviceState.accountData?.number].compactMap { $0 } - - let report = ConsolidatedApplicationLog( - redactCustomStrings: redactStrings, - redactContainerPathsForSecurityGroupIdentifiers: [securityGroupIdentifier] + init(apiProxy: APIQuerying, tunnelManager: TunnelManager) { + self.apiProxy = apiProxy + self.tunnelManager = tunnelManager + let redactCustomStrings = [tunnelManager.deviceState.accountData?.number].compactMap { $0 } + self.consolidatedLog = ConsolidatedApplicationLog( + redactCustomStrings: redactCustomStrings.isEmpty ? nil : redactCustomStrings, + redactContainerPathsForSecurityGroupIdentifiers: [ApplicationConfiguration.securityGroupIdentifier], + bufferSize: ApplicationConfiguration.logMaximumFileSize ) + } - let logFileURLs = ApplicationTarget.allCases.flatMap { + func fetchReportString(completion: @escaping (String) -> Void) { + consolidatedLog.addLogFiles(fileURLs: ApplicationTarget.allCases.flatMap { ApplicationConfiguration.logFileURLs(for: $0, in: ApplicationConfiguration.containerURL) + }) { [weak self] in + guard let self else { return } + completion(consolidatedLog.string) } - report.addLogFiles(fileURLs: logFileURLs) - - return report - }() - - var reportString: String { - consolidatedLog.string } - init(apiProxy: APIQuerying, tunnelManager: TunnelManager) { - self.apiProxy = apiProxy - self.tunnelManager = tunnelManager + func sendReport( + email: String, + message: String, + completion: @escaping (Result) -> Void + ) { + let logString = self.consolidatedLog.string + + if logString.isEmpty { + fetchReportString { [weak self] updatedLogString in + self?.sendProblemReport( + email: email, + message: message, + logString: updatedLogString, + completion: completion + ) + } + } else { + sendProblemReport( + email: email, + message: message, + logString: logString, + completion: completion + ) + } } - func sendReport( + private func sendProblemReport( email: String, message: String, + logString: String, completion: @escaping (Result) -> Void - ) -> Cancellable { + ) { + let metadataDict = self.consolidatedLog.metadata.reduce(into: [:]) { output, entry in + output[entry.key.rawValue] = entry.value + } + let request = REST.ProblemReportRequest( address: email, message: message, - log: consolidatedLog.string, - metadata: consolidatedLog.metadata.reduce(into: [:]) { output, entry in - output[entry.key.rawValue] = entry.value - } + log: logString, + metadata: metadataDict ) - return apiProxy.sendProblemReport( + _ = self.apiProxy.sendProblemReport( request, - retryStrategy: .default, - completionHandler: completion - ) + retryStrategy: .default + ) { result in + DispatchQueue.main.async { + completion(result) + } + } } } diff --git a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportReviewViewController.swift b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportReviewViewController.swift index a12d32362a74..d7b6f54c38e4 100644 --- a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportReviewViewController.swift +++ b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportReviewViewController.swift @@ -9,8 +9,14 @@ import UIKit class ProblemReportReviewViewController: UIViewController { + private let spinnerView = SpinnerActivityIndicatorView(style: .large) private var textView = UITextView() private let interactor: ProblemReportInteractor + private lazy var spinnerContainerView: UIView = { + let view = UIView() + view.backgroundColor = .black.withAlphaComponent(0.5) + return view + }() init(interactor: ProblemReportInteractor) { self.interactor = interactor @@ -23,7 +29,7 @@ class ProblemReportReviewViewController: UIViewController { override func viewDidLoad() { super.viewDidLoad() - + view.backgroundColor = .secondaryColor view.accessibilityIdentifier = .appLogsView navigationItem.title = NSLocalizedString( @@ -60,14 +66,21 @@ class ProblemReportReviewViewController: UIViewController { ) textView.backgroundColor = .systemBackground - view.addSubview(textView) + view.addConstrainedSubviews([textView]) { + textView.pinEdgesToSuperview(.all().excluding(.top)) + textView.pinEdgeToSuperviewMargin(.top(0)) + } - NSLayoutConstraint.activate([ - textView.topAnchor.constraint(equalTo: view.topAnchor), - textView.leadingAnchor.constraint(equalTo: view.leadingAnchor), - textView.trailingAnchor.constraint(equalTo: view.trailingAnchor), - textView.bottomAnchor.constraint(equalTo: view.bottomAnchor), - ]) + textView.addConstrainedSubviews([spinnerContainerView]) { + spinnerContainerView.pinEdgesToSuperview() + spinnerContainerView.widthAnchor.constraint(equalTo: textView.widthAnchor) + spinnerContainerView.heightAnchor.constraint(equalTo: textView.heightAnchor) + } + + spinnerContainerView.addConstrainedSubviews([spinnerView]) { + spinnerView.centerXAnchor.constraint(equalTo: view.centerXAnchor) + spinnerView.centerYAnchor.constraint(equalTo: view.centerYAnchor) + } // Used to layout constraints so that navigation controller could properly adjust the text // view insets. @@ -81,30 +94,28 @@ class ProblemReportReviewViewController: UIViewController { } private func loadLogs() { - let presentation = AlertPresentation( - id: "problem-report-load", - icon: .spinner, - buttons: [] - ) - - let alertController = AlertViewController(presentation: presentation) - - present(alertController, animated: true) { - self.textView.text = self.interactor.reportString - self.dismiss(animated: true) + spinnerView.startAnimating() + interactor.fetchReportString { [weak self] reportString in + guard let self else { return } + textView.text = reportString + spinnerView.stopAnimating() + spinnerContainerView.isHidden = true } } #if DEBUG private func share() { - let activityController = UIActivityViewController( - activityItems: [interactor.reportString], - applicationActivities: nil - ) + interactor.fetchReportString { [weak self] reportString in + guard let self,!reportString.isEmpty else { return } + let activityController = UIActivityViewController( + activityItems: [reportString], + applicationActivities: nil + ) - activityController.popoverPresentationController?.barButtonItem = navigationItem.leftBarButtonItem + activityController.popoverPresentationController?.barButtonItem = navigationItem.leftBarButtonItem - present(activityController, animated: true) + present(activityController, animated: true) + } } #endif } diff --git a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportViewController.swift b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportViewController.swift index 15d21ee9dbe1..47ba3225e963 100644 --- a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportViewController.swift +++ b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportViewController.swift @@ -131,7 +131,7 @@ final class ProblemReportViewController: UIViewController, UITextFieldDelegate { @objc func handleViewLogsButtonTap() { let reviewController = ProblemReportReviewViewController(interactor: interactor) - let navigationController = UINavigationController(rootViewController: reviewController) + let navigationController = CustomNavigationController(rootViewController: reviewController) present(navigationController, animated: true) } @@ -258,7 +258,7 @@ final class ProblemReportViewController: UIViewController, UITextFieldDelegate { willSendProblemReport() - _ = interactor.sendReport( + interactor.sendReport( email: viewModel.email, message: viewModel.message ) { completion in diff --git a/ios/MullvadVPNTests/MullvadVPN/Log/ConsolidatedApplicationLogTests.swift b/ios/MullvadVPNTests/MullvadVPN/Log/ConsolidatedApplicationLogTests.swift new file mode 100644 index 000000000000..523c02bebf28 --- /dev/null +++ b/ios/MullvadVPNTests/MullvadVPN/Log/ConsolidatedApplicationLogTests.swift @@ -0,0 +1,127 @@ +// +// ConsolidatedApplicationLogTests.swift +// MullvadVPNTests +// +// Created by Mojgan on 2024-11-21. +// Copyright © 2024 Mullvad VPN AB. All rights reserved. +// + +import XCTest + +class ConsolidatedApplicationLogTests: XCTestCase { + var consolidatedLog: ConsolidatedApplicationLog! + let mockRedactStrings = ["sensitive", "secret"] + let mockSecurityGroupIdentifiers = ["group1", "group2"] + var createdMockFiles: [URL] = [] + let kRedactedPlaceholder = "[REDACTED]" + + override func setUp() { + super.setUp() + consolidatedLog = ConsolidatedApplicationLog( + redactCustomStrings: mockRedactStrings, + redactContainerPathsForSecurityGroupIdentifiers: mockSecurityGroupIdentifiers, + bufferSize: 65_536 + ) + createdMockFiles = [] + } + + override func tearDownWithError() throws { + try super.tearDownWithError() + consolidatedLog = nil + // Remove all mock files created during tests + for file in createdMockFiles { + try FileManager.default.removeItem(at: file) + } + createdMockFiles = [] + } + + func testAddLogFiles() { + var string = "" + let expectation = self.expectation(description: "Log files added") + let mockFile = createMockFile(content: content, fileName: "\(generateRandomName()).txt") + + consolidatedLog.addLogFiles(fileURLs: [mockFile]) { + expectation.fulfill() + } + + waitForExpectations(timeout: 1) + consolidatedLog.write(to: &string) + XCTAssertTrue( + consolidatedLog.string.contains(string), + "Log should contain the file content." + ) + } + + func testAddError() { + let expectation = self.expectation(description: "Error added to log") + let errorMessage = "Test error" + let errorDetails = "A sensitive error occurred" + + consolidatedLog.addError(message: errorMessage, error: errorDetails) { + expectation.fulfill() + } + + waitForExpectations(timeout: 1) + XCTAssertTrue( + consolidatedLog.string.contains(errorMessage), + "Log should include the error message." + ) + } + + func testStringOutput() { + let expectation = self.expectation(description: "Log files added") + let mockFile = createMockFile(content: content, fileName: "\(generateRandomName()).txt") + consolidatedLog.addLogFiles(fileURLs: [mockFile]) { + expectation.fulfill() + let output = self.consolidatedLog.string + XCTAssertFalse(output.isEmpty, "Output string should include redacted log content.") + } + waitForExpectations(timeout: 1) + } + + // MARK: - Private functions + + private func createMockFile(content: String, fileName: String) -> URL { + let tempDirectory = FileManager.default.temporaryDirectory + let fileURL = tempDirectory.appendingPathComponent(fileName) + + do { + try content.write(to: fileURL, atomically: true, encoding: .utf8) + createdMockFiles.append(fileURL) + } catch { + XCTFail("Failed to create mock file: \(error)") + } + return fileURL + } + + private func generateRandomName() -> String { + let characterSet = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" + let randomName = (0 ..< 6).compactMap { _ in characterSet.randomElement() } + return String(randomName) + } +} + +extension ConsolidatedApplicationLogTests { + private var content: String { + return """ + MullvadVPN version xxxx.x + [22/11/2024 @ 08:52:22][AppDelegate][debug] Registered app refresh task. + [22/11/2024 @ 08:52:22][AppDelegate][debug] Registered address cache update task. + [22/11/2024 @ 08:52:22][AppDelegate][debug] Registered private key rotation task. + [22/11/2024 @ 08:52:23][TunnelManager][debug] Refresh device state + and tunnel status + due to application becoming active. + [22/11/2024 @ 08:52:23][RelayCacheTracker][debug] Start periodic relay updates. + [22/11/2024 @ 08:52:23][AddressCache.Tracker][debug] Start periodic address cache updates. + [22/11/2024 @ 08:52:23][AddressCache.Tracker][debug] Schedule address cache update at 23/11/2024 @ 08:49:52. + [22/11/2024 @ 08:52:23][AppDelegate][debug] Attempted migration from UI Process, but found nothing to do. + [22/11/2024 @ 08:52:23][TunnelManager][debug] Refresh tunnel status for new tunnel. + [22/11/2024 @ 08:52:23][REST.NetworkOperation][debug] name=get-access-token.2 + Send request + to /auth/v1/token via 127.0.0.1 using encrypted-dns-url-session. + [22/11/2024 @ 08:52:23][ApplicationRouter][debug] Presenting .main. + [22/11/2024 @ 08:52:23][REST.NetworkOperation][debug] name=get-access-token.2 Response: 200. + [22/11/2024 @ 08:52:23][AppDelegate][debug] Finished initialization. + """ + } +} diff --git a/ios/Shared/LaunchArguments.swift b/ios/Shared/LaunchArguments.swift index ab75e73b694e..e1f3c0ad41ef 100644 --- a/ios/Shared/LaunchArguments.swift +++ b/ios/Shared/LaunchArguments.swift @@ -7,6 +7,7 @@ // import Foundation +import MullvadTypes public protocol Taggable { static var tag: String { get } @@ -46,7 +47,9 @@ public extension ProcessInfo { extension Encodable { public func toJSON(_ encoder: JSONEncoder = JSONEncoder()) throws -> String { let data = try encoder.encode(self) - let result = String(decoding: data, as: UTF8.self) + guard let result = String(bytes: data, encoding: .utf8) else { + throw StringDecodingError(data: data) + } return result } }