diff --git a/Sources/MacRazer/CrashReport.swift b/Sources/MacRazer/CrashReport.swift index b5668d6..c899ea6 100644 --- a/Sources/MacRazer/CrashReport.swift +++ b/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 } diff --git a/Sources/MacRazer/DeviceReportOutput.swift b/Sources/MacRazer/DeviceReportOutput.swift index 80803c0..33702a8 100644 --- a/Sources/MacRazer/DeviceReportOutput.swift +++ b/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 } diff --git a/Sources/MacRazer/ReportComment.swift b/Sources/MacRazer/ReportComment.swift new file mode 100644 index 0000000..72116d3 --- /dev/null +++ b/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 + } +} diff --git a/Tests/MacRazerTests/CrashReportTests.swift b/Tests/MacRazerTests/CrashReportTests.swift index 52e0478..a858285 100644 --- a/Tests/MacRazerTests/CrashReportTests.swift +++ b/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 = "๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ" diff --git a/Tests/MacRazerTests/DeviceReportOutputTests.swift b/Tests/MacRazerTests/DeviceReportOutputTests.swift index 9705e75..92063df 100644 --- a/Tests/MacRazerTests/DeviceReportOutputTests.swift +++ b/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) diff --git a/Tests/MacRazerTests/ReportCommentTests.swift b/Tests/MacRazerTests/ReportCommentTests.swift new file mode 100644 index 0000000..365cd06 --- /dev/null +++ b/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") + } +}