Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix problem report logs being duplicated #6235

Merged
merged 1 commit into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ios/MullvadVPN/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
53 changes: 39 additions & 14 deletions ios/MullvadVPNTests/MullvadLogging/LoggingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,12 @@
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
Expand Down Expand Up @@ -279,7 +284,7 @@

extension PacketTunnelProvider: PostQuantumKeyReceiving {
func receivePostQuantumKey(_ key: PreSharedKey) {
// TODO: send the key to the actor

Check warning on line 287 in ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift

View workflow job for this annotation

GitHub Actions / End to end tests

Todo Violation: TODOs should be resolved (send the key to the actor) (todo)

Check warning on line 287 in ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift

View workflow job for this annotation

GitHub Actions / End to end tests

Todo Violation: TODOs should be resolved (send the key to the actor) (todo)
actor.replacePreSharedKey(key)
}
}
14 changes: 7 additions & 7 deletions ios/Shared/ApplicationConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<Target Bundle ID>_", 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
Expand Down
Loading