From 168bd3507b28fbb385057f0fba7296385ce2e11b Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Wed, 8 May 2024 17:53:11 +0200 Subject: [PATCH] Fix problem report logs being duplicated --- ios/MullvadVPN/AppDelegate.swift | 4 +- .../ProblemReportInteractor.swift | 4 +- .../MullvadLogging/LogRotationTests.swift | 7 +-- .../MullvadLogging/LoggingTests.swift | 53 ++++++++++++++----- .../PacketTunnelProvider.swift | 7 ++- ios/Shared/ApplicationConfiguration.swift | 14 ++--- 6 files changed, 62 insertions(+), 27 deletions(-) diff --git a/ios/MullvadVPN/AppDelegate.swift b/ios/MullvadVPN/AppDelegate.swift index 156b4771428d..7e7ff266aeb0 100644 --- a/ios/MullvadVPN/AppDelegate.swift +++ b/ios/MullvadVPN/AppDelegate.swift @@ -353,7 +353,9 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD private func configureLogging() { var loggerBuilder = LoggerBuilder(header: "MullvadVPN version \(Bundle.main.productVersion)") - loggerBuilder.addFileOutput(fileURL: ApplicationConfiguration.newLogFileURL(for: .mainApp)) + loggerBuilder.addFileOutput( + fileURL: ApplicationConfiguration.newLogFileURL(for: .mainApp, in: ApplicationConfiguration.containerURL) + ) #if DEBUG loggerBuilder.addOSLogOutput(subsystem: ApplicationTarget.mainApp.bundleIdentifier) #endif diff --git a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift index f522662942be..bbc740f453ec 100644 --- a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift +++ b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift @@ -24,7 +24,9 @@ final class ProblemReportInteractor { redactContainerPathsForSecurityGroupIdentifiers: [securityGroupIdentifier] ) - let logFileURLs = ApplicationTarget.allCases.flatMap { ApplicationConfiguration.logFileURLs(for: $0) } + let logFileURLs = ApplicationTarget.allCases.flatMap { + ApplicationConfiguration.logFileURLs(for: $0, in: ApplicationConfiguration.containerURL) + } report.addLogFiles(fileURLs: logFileURLs) return report diff --git a/ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift b/ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift index f4f70d012409..d67b3099dc1a 100644 --- a/ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift +++ b/ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift @@ -11,17 +11,18 @@ import XCTest final class LogRotationTests: XCTestCase { let fileManager = FileManager.default - let directoryPath = FileManager.default.temporaryDirectory.appendingPathComponent("LogRotationTests") + let directoryPath = FileManager.default.temporaryDirectory + .appendingPathComponent("LogRotationTests", isDirectory: true) override func setUpWithError() throws { try? fileManager.createDirectory( at: directoryPath, - withIntermediateDirectories: false + withIntermediateDirectories: true ) } override func tearDownWithError() throws { - try fileManager.removeItem(atPath: directoryPath.relativePath) + try fileManager.removeItem(at: directoryPath) } func testRotatingActiveLogWhenSizeLimitIsExceeded() throws { diff --git a/ios/MullvadVPNTests/MullvadLogging/LoggingTests.swift b/ios/MullvadVPNTests/MullvadLogging/LoggingTests.swift index 63817d6aa202..7661c2714f9f 100644 --- a/ios/MullvadVPNTests/MullvadLogging/LoggingTests.swift +++ b/ios/MullvadVPNTests/MullvadLogging/LoggingTests.swift @@ -11,25 +11,24 @@ import Foundation import XCTest class MullvadLoggingTests: XCTestCase { - func temporaryFileURL() -> URL { - // Create a URL for an unique file in the system's temporary directory. - let directory = NSTemporaryDirectory() - let filename = UUID().uuidString - let fileURL = URL(fileURLWithPath: directory).appendingPathComponent(filename) - - // Add a teardown block to delete any file at `fileURL`. - addTeardownBlock { - try? FileManager.default.removeItem(at: fileURL) - } + let fileManager = FileManager.default + let directoryPath = FileManager.default.temporaryDirectory.appendingPathComponent("LoggingTests", isDirectory: true) + + override func setUpWithError() throws { + try fileManager.createDirectory( + at: directoryPath, + withIntermediateDirectories: true + ) + } - // Return the temporary file URL for use in a test method. - return fileURL + override func tearDownWithError() throws { + try fileManager.removeItem(at: directoryPath) } func testLogFileOutputStreamWritesHeader() throws { let headerText = "This is a header" let logMessage = "And this is a log message\n" - let fileURL = temporaryFileURL() + let fileURL = directoryPath.appendingPathComponent(UUID().uuidString) let stream = LogFileOutputStream(fileURL: fileURL, header: headerText) stream.write(logMessage) sync() @@ -42,7 +41,7 @@ class MullvadLoggingTests: XCTestCase { let expectedHeader = "Header of a log file" var builder = LoggerBuilder(header: expectedHeader) - let fileURL = temporaryFileURL() + let fileURL = directoryPath.appendingPathComponent(UUID().uuidString) builder.addFileOutput(fileURL: fileURL) builder.install() @@ -55,4 +54,30 @@ class MullvadLoggingTests: XCTestCase { XCTAssert(contents.hasPrefix(expectedHeader)) } + + func testGettingLogFilesByApplicationTarget() async throws { + let mainTargetLog = ApplicationConfiguration.newLogFileURL(for: .mainApp, in: directoryPath) + let packetTunnelTargetLog = ApplicationConfiguration.newLogFileURL(for: .packetTunnel, in: directoryPath) + + let logPaths = [ + directoryPath.appendingPathComponent("test1.log"), + directoryPath.appendingPathComponent("test2.log"), + mainTargetLog, + packetTunnelTargetLog, + ] + + logPaths.forEach { url in + let stream = LogFileOutputStream(fileURL: url, header: "") + stream.write("test") + sync() + } + + var urls = ApplicationConfiguration.logFileURLs(for: .mainApp, in: directoryPath) + XCTAssertEqual(urls.count, 1) + XCTAssertEqual(urls.first, mainTargetLog) + + urls = ApplicationConfiguration.logFileURLs(for: .packetTunnel, in: directoryPath) + XCTAssertEqual(urls.count, 1) + XCTAssertEqual(urls.first, packetTunnelTargetLog) + } } diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift index 628828f5282c..a63534877397 100644 --- a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift +++ b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift @@ -163,7 +163,12 @@ extension PacketTunnelProvider { var loggerBuilder = LoggerBuilder(header: "PacketTunnel version \(Bundle.main.productVersion)") let pid = ProcessInfo.processInfo.processIdentifier loggerBuilder.metadata["pid"] = .string("\(pid)") - loggerBuilder.addFileOutput(fileURL: ApplicationConfiguration.newLogFileURL(for: .packetTunnel)) + loggerBuilder.addFileOutput( + fileURL: ApplicationConfiguration.newLogFileURL( + for: .packetTunnel, + in: ApplicationConfiguration.containerURL + ) + ) #if DEBUG loggerBuilder.addOSLogOutput(subsystem: ApplicationTarget.packetTunnel.bundleIdentifier) #endif diff --git a/ios/Shared/ApplicationConfiguration.swift b/ios/Shared/ApplicationConfiguration.swift index 4e2b28c62f54..403c6ff7fa2a 100644 --- a/ios/Shared/ApplicationConfiguration.swift +++ b/ios/Shared/ApplicationConfiguration.swift @@ -21,25 +21,25 @@ enum ApplicationConfiguration { FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: securityGroupIdentifier)! } - /// Returns URL for new log file associated with application target and located within shared container. - static func newLogFileURL(for target: ApplicationTarget) -> URL { + /// Returns URL for new log file associated with application target and located within the specified container. + static func newLogFileURL(for target: ApplicationTarget, in containerURL: URL) -> URL { containerURL.appendingPathComponent( "\(target.bundleIdentifier)_\(Date().logFileFormatted).log", isDirectory: false ) } - /// Returns URLs for log files associated with application target and located within shared container. - static func logFileURLs(for target: ApplicationTarget) -> [URL] { + /// Returns URLs for log files associated with application target and located within the specified container. + static func logFileURLs(for target: ApplicationTarget, in containerURL: URL) -> [URL] { let fileManager = FileManager.default - let containerUrl = containerURL let filePathsInDirectory = try? fileManager.contentsOfDirectory(atPath: containerURL.relativePath) let filteredFilePaths: [URL] = filePathsInDirectory?.compactMap { path in let pathIsLog = path.split(separator: ".").last == "log" - let pathBelongsToTarget = path.contains(target.bundleIdentifier) + // Pattern should be "_", eg. "net.mullvad.MullvadVPN_". + let pathBelongsToTarget = path.contains("\(target.bundleIdentifier)_") - return pathIsLog && pathBelongsToTarget ? containerUrl.appendingPathComponent(path) : nil + return pathIsLog && pathBelongsToTarget ? containerURL.appendingPathComponent(path) : nil } ?? [] let sortedFilePaths = try? filteredFilePaths.sorted { path1, path2 in