Truncate the current log file in half when reaching size limit

4 files changed, 175 insertions, 75 deletions

diff --git a/ios/MullvadLogging/LogFileOutputStream.swift b/ios/MullvadLogging/LogFileOutputStream.swift
index 6d63d43b77..a359e238d1 100644
--- a/ios/MullvadLogging/LogFileOutputStream.swift
+++ b/ios/MullvadLogging/LogFileOutputStream.swift
@@ -9,10 +9,6 @@
 import Foundation
 import MullvadTypes
 
-/// Interval used for reopening the log file descriptor in the event of failure to open it in
-/// the first place, or when writing to it.
-private let reopenFileLogInterval: Duration = .seconds(5)
-
 class LogFileOutputStream: TextOutputStream, @unchecked Sendable {
     private let queue = DispatchQueue(label: "LogFileOutputStreamQueue", qos: .utility)
 
@@ -22,8 +18,11 @@ class LogFileOutputStream: TextOutputStream, @unchecked Sendable {
     private let maximumBufferCapacity: Int
     private let fileHeader: String
     private let fileSizeLimit: UInt64
-    private var partialFileSizeCounter: UInt64 = 0
-    private var partialFileNameCounter = 1
+    private let newLineChunkReadSize: Int
+
+    /// Interval used for reopening the log file descriptor in the event of failure to open it in
+    /// the first place, or when writing to it.
+    private let reopenFileLogInterval: Duration
 
     private var state: State = .closed {
         didSet {
@@ -40,6 +39,9 @@ class LogFileOutputStream: TextOutputStream, @unchecked Sendable {
         }
     }
 
+    /// Shorthand to get the file header in a `Data` writeable format
+    private var headerData: Data { "\(fileHeader)\n".data(using: encoding, allowLossyConversion: true)! }
+
     private var timer: DispatchSourceTimer?
     private var buffer = Data()
 
@@ -54,13 +56,17 @@ class LogFileOutputStream: TextOutputStream, @unchecked Sendable {
         header: String,
         fileSizeLimit: UInt64 = ApplicationConfiguration.logMaximumFileSize,
         encoding: String.Encoding = .utf8,
-        maxBufferCapacity: Int = 16 * 1024
+        maxBufferCapacity: Int = 16 * 1024,
+        reopenFileLogInterval: Duration = .seconds(5),
+        newLineChunkReadSize: Int = 35
     ) {
         self.fileURL = fileURL
         self.fileHeader = header
         self.fileSizeLimit = fileSizeLimit
         self.encoding = encoding
         self.maximumBufferCapacity = maxBufferCapacity
+        self.reopenFileLogInterval = reopenFileLogInterval
+        self.newLineChunkReadSize = newLineChunkReadSize
 
         baseFileURL = fileURL.deletingPathExtension()
     }
@@ -83,6 +89,7 @@ class LogFileOutputStream: TextOutputStream, @unchecked Sendable {
     }
 
     private func writeOnQueue(_ string: String) {
+        dispatchPrecondition(condition: .onQueue(queue))
         guard let data = string.data(using: encoding) else { return }
 
         switch state {
@@ -120,39 +127,34 @@ class LogFileOutputStream: TextOutputStream, @unchecked Sendable {
             throw POSIXError(.EDQUOT)
         }
 
-        let predictedFileSize = partialFileSizeCounter + incomingDataSize
+        let predictedFileSize = try fileHandle.offset() + incomingDataSize
 
-        // Rotate file if threshold has been met, then rerun the write operation.
-        guard predictedFileSize <= fileSizeLimit else {
-            try rotateFile(handle: fileHandle)
-            write(String(data: data, encoding: encoding) ?? "")
-            return
+        // Truncate file in half if threshold has been met, otherwise just write.
+        if predictedFileSize >= fileSizeLimit {
+            try truncateFileInHalf(fileHandle: fileHandle)
         }
 
-        let bytesWritten = data.withUnsafeBytes { buffer -> Int in
-            guard let ptr = buffer.baseAddress else { return 0 }
+        try fileHandle.write(contentsOf: data)
+    }
 
-            return Darwin.write(fileHandle.fileDescriptor, ptr, buffer.count)
-        }
+    private func truncateFileInHalf(fileHandle: FileHandle) throws {
+        let fileCenterOffset = UInt64(fileSizeLimit / 2)
 
-        if bytesWritten == -1 {
-            let code = POSIXErrorCode(rawValue: errno)!
-            throw POSIXError(code)
-        }
+        try fileHandle.seek(toOffset: fileCenterOffset)
 
-        partialFileSizeCounter += UInt64(bytesWritten)
-    }
+        /// Advance the file offset to the next line (delimited by a \n) to make the log
+        /// truncation appear more user friendly by not potentially cutting a log line in half
+        try fileHandle.readUntilNextLineBreak(readSize: UInt64(newLineChunkReadSize), sizeLimit: fileSizeLimit)
 
-    private func rotateFile(handle: FileHandle) throws {
-        try handle.close()
+        let fileLastHalf = fileHandle.availableData
 
-        state = .closed
-        partialFileSizeCounter = 0
-        fileURL = try incrementFileName()
+        try fileHandle.truncate(atOffset: 0)
+        try fileHandle.write(contentsOf: headerData)
+        try fileHandle.write(contentsOf: fileLastHalf)
     }
 
     private func openFile() throws -> FileHandle {
-        let oflag: Int32 = O_WRONLY | O_CREAT
+        let oflag: Int32 = O_RDWR | O_CREAT
         let mode: mode_t = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
 
         let fd = fileURL.path.withCString { Darwin.open($0, oflag, mode) }
@@ -189,10 +191,7 @@ class LogFileOutputStream: TextOutputStream, @unchecked Sendable {
     private func openFileWithHeader(_ header: String) throws -> FileHandle {
         let fileHandle = try openFile()
 
-        let messageData =
-            "\(header)\n"
-                .data(using: encoding, allowLossyConversion: true)!
-        try write(fileHandle: fileHandle, data: messageData)
+        try write(fileHandle: fileHandle, data: headerData)
 
         return fileHandle
     }
@@ -222,14 +221,31 @@ class LogFileOutputStream: TextOutputStream, @unchecked Sendable {
             buffer.removeFirst(buffer.count - maximumBufferCapacity)
         }
     }
+}
 
-    private func incrementFileName() throws -> URL {
-        partialFileNameCounter += 1
+fileprivate extension FileHandle {
+    /// Reads into the file until the next "\n" is reached.
+    ///
+    /// The file pointer will be set to the offset after the first "\n"
+    /// character encountered. If the attempted read would go past
+    /// the `sizeLimit` no reads are attempted and the file pointer
+    /// will not be moved.
+    ///
+    func readUntilNextLineBreak(readSize: UInt64, sizeLimit: UInt64) throws {
+        let currentOffset = try offset()
+        // Ignore Integer overflow checks, files would not reach that size in the first place
+        // Do not try to read past the end of the file
+        guard currentOffset + readSize <= sizeLimit else { return }
+        let readBytes = try read(upToCount: Int(readSize))
 
-        if let url = URL(string: baseFileURL.relativePath + "_\(partialFileNameCounter).log") {
-            return url
+        // Find the first instance of the "\n" character
+        if let newLineIndex = readBytes?.firstIndex(of: 10) {
+            let offsetAfterNewLine = currentOffset + UInt64(newLineIndex) + 1
+            try seek(toOffset: offsetAfterNewLine)
+            return
         } else {
-            throw POSIXError(.ENOENT)
+            // Keep reading until either a "\n" character, or the end of the file are found
+            try readUntilNextLineBreak(readSize: readSize, sizeLimit: sizeLimit)
         }
     }
 }
diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
index 9ff46af6fc..63d3c291d4 100644
--- a/ios/MullvadVPN.xcodeproj/project.pbxproj
+++ b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -711,6 +711,7 @@
 		85EC620C2B838D10005AFFB5 /* MullvadAPIWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85557B132B5983CF00795FE1 /* MullvadAPIWrapper.swift */; };
 		85FB5A0C2B6903990015DCED /* WelcomePage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85FB5A0B2B6903990015DCED /* WelcomePage.swift */; };
 		85FB5A102B6960A30015DCED /* AccountDeletionPage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85FB5A0F2B6960A30015DCED /* AccountDeletionPage.swift */; };
+		A902E7A62D3FB0D9007F844A /* LogFileOutputStreamTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A902E7A52D3FB0D9007F844A /* LogFileOutputStreamTests.swift */; };
 		A90763B02B2857D50045ADF0 /* Socks5ConnectCommand.swift in Sources */ = {isa = PBXBuildFile; fileRef = A90763A02B2857D50045ADF0 /* Socks5ConnectCommand.swift */; };
 		A90763B12B2857D50045ADF0 /* Socks5Endpoint.swift in Sources */ = {isa = PBXBuildFile; fileRef = A90763A12B2857D50045ADF0 /* Socks5Endpoint.swift */; };
 		A90763B22B2857D50045ADF0 /* Socks5EndpointReader.swift in Sources */ = {isa = PBXBuildFile; fileRef = A90763A22B2857D50045ADF0 /* Socks5EndpointReader.swift */; };
@@ -2111,6 +2112,7 @@
 		A900E9BB2ACC609200C95F67 /* DevicesProxy+Stubs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DevicesProxy+Stubs.swift"; sourceTree = "<group>"; };
 		A900E9BD2ACC654100C95F67 /* APIProxy+Stubs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "APIProxy+Stubs.swift"; sourceTree = "<group>"; };
 		A900E9BF2ACC661900C95F67 /* AccessTokenManager+Stubs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "AccessTokenManager+Stubs.swift"; sourceTree = "<group>"; };
+		A902E7A52D3FB0D9007F844A /* LogFileOutputStreamTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LogFileOutputStreamTests.swift; sourceTree = "<group>"; };
 		A90763A02B2857D50045ADF0 /* Socks5ConnectCommand.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Socks5ConnectCommand.swift; sourceTree = "<group>"; };
 		A90763A12B2857D50045ADF0 /* Socks5Endpoint.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Socks5Endpoint.swift; sourceTree = "<group>"; };
 		A90763A22B2857D50045ADF0 /* Socks5EndpointReader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Socks5EndpointReader.swift; sourceTree = "<group>"; };
@@ -2646,6 +2648,7 @@
 			children = (
 				44B02E3A2BC5732D008EDF34 /* LoggingTests.swift */,
 				7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */,
+				A902E7A52D3FB0D9007F844A /* LogFileOutputStreamTests.swift */,
 			);
 			path = MullvadLogging;
 			sourceTree = "<group>";
@@ -5668,6 +5671,7 @@
 				7A516C3C2B712F0B00BBD33D /* IPOverrideWrapperTests.swift in Sources */,
 				A9A5FA092ACB05160083449F /* SendStoreReceiptOperation.swift in Sources */,
 				A9A5FA0A2ACB05160083449F /* StorePaymentEvent.swift in Sources */,
+				A902E7A62D3FB0D9007F844A /* LogFileOutputStreamTests.swift in Sources */,
 				A9A5FA0B2ACB05160083449F /* StorePaymentManager.swift in Sources */,
 				A9A5FA0C2ACB05160083449F /* StorePaymentManagerDelegate.swift in Sources */,
 				A9A5FA0D2ACB05160083449F /* StorePaymentManagerError.swift in Sources */,
diff --git a/ios/MullvadVPNTests/MullvadLogging/LogFileOutputStreamTests.swift b/ios/MullvadVPNTests/MullvadLogging/LogFileOutputStreamTests.swift
new file mode 100644
index 0000000000..c1e400865c
--- /dev/null
+++ b/ios/MullvadVPNTests/MullvadLogging/LogFileOutputStreamTests.swift
@@ -0,0 +1,117 @@
+//
+//  LogFileOutputStreamTests.swift
+//  MullvadVPNTests
+//
+//  Created by Marco Nikic on 2025-01-21.
+//  Copyright © 2025 Mullvad VPN AB. All rights reserved.
+//
+
+import Foundation
+@testable import MullvadLogging
+import Testing
+
+@Suite("LogFileOutputStream Tests")
+actor LogFileOutputStreamTests {
+    let fileManager = FileManager.default
+    var directoryPath: URL!
+
+    init() async throws {
+        directoryPath = FileManager.default.temporaryDirectory.appendingPathComponent(
+            "LogFileOutputStreamTests",
+            isDirectory: true
+        )
+
+        try fileManager.createDirectory(
+            at: directoryPath,
+            withIntermediateDirectories: true
+        )
+    }
+
+    deinit {
+        try? fileManager.removeItem(at: directoryPath)
+    }
+
+    @Test func logHeaderGetsWrittenAtFileStartAfterTruncation() async throws {
+        let header = "header"
+        let message = """
+        old
+
+        """
+        let fileSizeLimit: UInt64 = 20
+        let fileURL = directoryPath.appendingPathComponent(UUID().uuidString)
+        let stream = LogFileOutputStream(
+            fileURL: fileURL,
+            header: header,
+            fileSizeLimit: fileSizeLimit,
+            newLineChunkReadSize: 3
+        )
+        // Fill the file with the word "old" to force truncation in half
+        for _ in 0 ..< 3 {
+            stream.write(message)
+        }
+        /* At this point, the file contains the following string (of length 19)
+         "header\nold\nold\nold"
+                    ^
+                    Half point of the file
+
+         Writing the word "new" goes over the file size limit (20),
+         so the file will get truncated to its half point.
+         In order to keep a nice UX for reading log, the stream will move the internal file cursor to after the next "\n"
+         character, and read the last half of the file in order to paste it at the beginning
+         after truncation.
+         In this example, the string "old\nold\n" will be buffered, which will then
+         get prepended with "header\n"
+          */
+        stream.synchronize()
+        stream.write("new")
+        stream.synchronize()
+
+        let fileContents = try #require(
+            try String(contentsOf: fileURL, encoding: .utf8)
+        )
+        let expectedContents = """
+        header
+        old
+        old
+        new
+        """
+
+        #expect(fileContents == expectedContents)
+    }
+
+    @Test func fileSizeCounterGetsResetAfterTruncation() async throws {
+        let header = "header"
+        let message = """
+        old
+
+        """
+        let fileSizeLimit: UInt64 = 20
+        let fileURL = directoryPath.appendingPathComponent(UUID().uuidString)
+        let stream = LogFileOutputStream(
+            fileURL: fileURL,
+            header: header,
+            fileSizeLimit: fileSizeLimit
+        )
+        // Fill the file with the word "old" to force truncation in half
+        for _ in 0 ..< 3 {
+            stream.write(message)
+        }
+        // File gets truncated in half here
+        stream.write("new")
+        stream.write("a")
+        stream.synchronize()
+
+        /// If the `partialFileSizeCounter` didn't get reset after truncating,
+        /// a new write will truncate the file again instead of just appending
+        let expectedContents = """
+        header
+        d
+        old
+        newa
+        """
+        let fileContents = try #require(
+            try String(contentsOf: fileURL, encoding: .utf8)
+        )
+        #expect(fileContents == expectedContents)
+    }
+}
diff --git a/ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift b/ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift
index a6194bfc98..ddfbe58b92 100644
--- a/ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift
+++ b/ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift
@@ -27,43 +27,6 @@ final class LogRotationTests: XCTestCase {
         try fileManager.removeItem(at: directoryPath)
     }
 
-    func testRotatingActiveLogWhenSizeLimitIsExceeded() throws {
-        let logName = "test.log"
-        let logPath = directoryPath.appendingPathComponent(logName)
-
-        let totalLogSizeLimit = 200
-        let totalLogTestSize = 645
-        let logChunkSize = 20
-
-        let expectedLogCount = Int(ceil(Double(totalLogTestSize) / Double(totalLogSizeLimit)))
-        let writeOperationCount = Int(ceil(Double(totalLogTestSize) / Double(logChunkSize)))
-
-        let stream = LogFileOutputStream(fileURL: logPath, header: "", fileSizeLimit: UInt64(totalLogSizeLimit))
-        for _ in 0 ..< writeOperationCount {
-            stream.write(stringOfSize(logChunkSize))
-        }
-        stream.synchronize()
-
-        let actualLogCount = try fileManager.contentsOfDirectory(atPath: directoryPath.relativePath).count
-        XCTAssertEqual(expectedLogCount, actualLogCount)
-
-        for index in 0 ..< actualLogCount {
-            var expectedFileName = logName
-
-            if index != 0 {
-                // Rotated log filenames start at "_2".
-                expectedFileName = expectedFileName.replacingOccurrences(of: ".log", with: "_\(index + 1).log")
-            }
-
-            let logExists = fileManager.fileExists(
-                atPath: directoryPath
-                    .appendingPathComponent(expectedFileName)
-                    .relativePath
-            )
-            XCTAssertTrue(logExists)
-        }
-    }
-
     func testRotateLogsByStorageSizeLimit() throws {
         let logPaths = [
             directoryPath.appendingPathComponent("test1.log"),