Trim an oversized device report's comment by bytes

Sources/MacRazer/CrashReport.swift

@@ -193,16 +193,9 @@ enum CrashReportOutput {
             r.frames.removeLast()
             r.framesDropped = (r.framesDropped ?? 0) + 1
         }
-        // Cut by bytes, not characters: an emoji is four bytes or more, and dropping one
-        // character per byte over would throw away far more than it has to. Whole characters
-        // only, so a family emoji isn't left half built.
+        // One trim fits (see `ReportComment`); the loop is only a guard.
         while case let excess = size(r) - maxBytes, excess > 0, let comment = r.comment, !comment.isEmpty {
-            var budget = comment.utf8.count - excess
-            let kept = String(comment.prefix {
-                budget -= $0.utf8.count
-                return budget >= 0
-            })
-            r.comment = kept.isEmpty ? nil : kept
+            r.comment = ReportComment.trimmed(comment, dropping: excess)
         }
         return r
     }

Sources/MacRazer/DeviceReportOutput.swift

@@ -40,11 +40,11 @@ enum DeviceReportOutput {
             r[keyPath: path] = []
             r.exchangesDropped = (r.exchangesDropped ?? []) + [name]
         }
-        // Every scalar is at least a byte, so dropping as many scalars as there are bytes
-        // over always gets under, in one pass. The loop is only a guard.
+        // One trim fits, since it measures the comment as the encoder writes it (see
+        // `ReportComment`). The loop is only a guard in case that measurement and the
+        // report's encoding ever disagree.
         while case let excess = size(r) - maxBytes, excess > 0, let comment = r.comment, !comment.isEmpty {
-            let kept = String(comment.unicodeScalars.dropLast(excess))
-            r.comment = kept.isEmpty ? nil : kept
+            r.comment = ReportComment.trimmed(comment, dropping: excess)
         }
         return r
     }

Sources/MacRazer/ReportComment.swift

@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Part of MacRazer, a control app for Razer mice on macOS. See LICENSE and NOTICE.md.
+
+import Foundation
+
+/// Trims a report's free-text comment so the report fits a byte limit. Shared by device and
+/// crash reports, whose JSON the Worker caps at the same size.
+enum ReportComment {
+    /// `comment` with whole characters removed from the end until its JSON is at least
+    /// `excess` bytes smaller, or nil when nothing is left.
+    ///
+    /// Each character is measured as the report's encoder writes it: an emoji is its UTF-8
+    /// bytes, a quote or a slash is two, a control character up to six. Counting raw bytes
+    /// instead would cut too much from a comment full of characters JSON escapes, and cutting
+    /// a character per byte over would cut far too much from one full of emoji. Whole
+    /// characters only, so a family emoji is never left half built.
+    static func trimmed(_ comment: String, dropping excess: Int) -> String? {
+        guard excess > 0 else { return comment }
+        let encoder = JSONEncoder()
+        var sizes: [Character: Int] = [:]
+        func jsonSize(_ c: Character) -> Int {
+            // Almost every character is written as its plain UTF-8 bytes. Only the few JSON may
+            // escape (control characters, quote, backslash, slash, the two line separators)
+            // go through the encoder, which keeps this cheap: the report windows fit the
+            // report on every keystroke.
+            guard c.unicodeScalars.contains(where: Self.mayBeEscaped) else { return c.utf8.count }
+            if let known = sizes[c] { return known }
+            // The encoded fragment is the character in quotes; the quotes aren't its cost.
+            let size = (try? encoder.encode(String(c))).map { $0.count - 2 } ?? c.utf8.count
+            sizes[c] = size
+            return size
+        }
+        var budget = comment.reduce(0) { $0 + jsonSize($1) } - excess
+        let kept = String(comment.prefix {
+            budget -= jsonSize($0)
+            return budget >= 0
+        })
+        return kept.isEmpty ? nil : kept
+    }
+
+    private static func mayBeEscaped(_ s: Unicode.Scalar) -> Bool {
+        s.value < 0x20 || s == "\"" || s == "\\" || s == "/" || s.value == 0x2028 || s.value == 0x2029
+    }
+}

Tests/MacRazerTests/CrashReportTests.swift

@@ -158,6 +158,15 @@ final class CrashReportOutputTests: XCTestCase {
         XCTAssertTrue(fitted.comment?.allSatisfy { $0 == "😀" } ?? false, "cut between emoji, not inside one")
     }
 
+    func testEscapedCharactersAreCutAtTheirJSONSize() {
+        // Quotes, backslashes, newlines and slashes are two bytes each in the JSON.
+        var r = CrashReportParser.parse(CrashFixture.ips(frames: 30))!
+        r.comment = String(repeating: "\"\\\n/", count: 500)
+        let fitted = CrashReportOutput.fitted(r, maxBytes: 4000)
+        XCTAssertLessThanOrEqual(CrashReportOutput.size(fitted), 4000)
+        XCTAssertGreaterThan(CrashReportOutput.size(fitted), 4000 - 2, "cut to the byte, not more")
+    }
+
     func testAMultiScalarEmojiIsNeverLeftHalfBuilt() {
         // One character of seven scalars, joined. A cut between scalars would leave a broken family.
         let family = "👩‍👩‍👧‍👦"

Tests/MacRazerTests/DeviceReportOutputTests.swift

@@ -89,6 +89,31 @@ final class DeviceReportOutputTests: XCTestCase {
         XCTAssertTrue(r.comment!.hasPrefix(fitted.comment ?? ""), "the comment loses its end, not its start")
     }
 
+    func testALongCommentIsCutByBytesNotCharacters() {
+        // Each emoji is four bytes. Cutting a character per byte over would throw away four
+        // times too much, and here more characters than the comment has.
+        let r = report(comment: String(repeating: "😀", count: DeviceReportOutput.maxComment))
+        let limit = 4000
+        let fitted = DeviceReportOutput.fitted(r, maxBytes: limit)
+        let size = DeviceReportOutput.size(fitted)
+        XCTAssertNotNil(fitted.comment, "some of the comment survives")
+        XCTAssertTrue(r.comment!.hasPrefix(fitted.comment ?? ""))
+        XCTAssertLessThanOrEqual(size, limit)
+        XCTAssertGreaterThan(size, limit - 4, "no more than one emoji short of the limit")
+    }
+
+    func testACommentOfEscapedCharactersFitsWithoutOvercutting() {
+        // Quotes, backslashes and newlines take more room in the JSON than in the comment.
+        let comment = String(repeating: "\"\\\n/", count: DeviceReportOutput.maxComment / 4)
+        let r = report(comment: comment)
+        let limit = 4000
+        let fitted = DeviceReportOutput.fitted(r, maxBytes: limit)
+        let size = DeviceReportOutput.size(fitted)
+        XCTAssertLessThanOrEqual(size, limit)
+        XCTAssertGreaterThan(size, limit - 2, "cut to the byte, not more")
+        XCTAssertTrue(comment.hasPrefix(fitted.comment ?? ""))
+    }
+
     func testEvidenceGoesOneStepAtATimeAndOnlyAsMuchAsNeeded() {
         let exchange = RecordingChannel.Exchange(commandClass: 0x04, commandId: 0x85, transactionId: 0x3F, status: 0x02,
                                                  response: [1, 2, 3, 4, 5, 6, 7, 8], error: nil, milliseconds: 40)

Tests/MacRazerTests/ReportCommentTests.swift

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Part of MacRazer, a control app for Razer mice on macOS. See LICENSE and NOTICE.md.
+
+import XCTest
+@testable import MacRazer
+
+final class ReportCommentTests: XCTestCase {
+    func testEmojiAreCutByTheirBytes() {
+        // Each emoji is four bytes: 10 bytes over means three emoji, not ten.
+        let comment = String(repeating: "😀", count: 20)
+        XCTAssertEqual(ReportComment.trimmed(comment, dropping: 10), String(repeating: "😀", count: 17))
+    }
+
+    func testEscapedCharactersCountAtTheirJSONSize() {
+        // A quote is two bytes in the JSON (\"), so 10 bytes over is five quotes. Counting raw
+        // bytes would have cut ten.
+        let comment = String(repeating: "\"", count: 100)
+        XCTAssertEqual(ReportComment.trimmed(comment, dropping: 10), String(repeating: "\"", count: 95))
+    }
+
+    func testWholeCharactersOnly() {
+        let family = "👩‍👩‍👧‍👦" // seven scalars, one character
+        XCTAssertEqual(ReportComment.trimmed("ab" + family, dropping: 1), "ab",
+                       "never a half-built family emoji")
+    }
+
+    func testNothingLeftIsNil() {
+        XCTAssertNil(ReportComment.trimmed("short", dropping: 100))
+        XCTAssertEqual(ReportComment.trimmed("short", dropping: 0), "short")
+    }
+}