Repository navigation
Trim an oversized device report's comment by bytes - #41
Merged
Merged
Conversation
fitted dropped one unicode scalar per byte over the limit. An emoji is four bytes, so it threw away up to four times what it had to, and when the excess passed the scalar count the whole comment went (2000 emoji fitted to 4000 bytes came back with no comment at all). Keep the longest prefix of whole characters whose UTF-8 size fits instead, so a multi-scalar emoji is never left half built. The loop still re-checks for JSON escaping.
Device and crash reports had the same comment-trimming loop. It now lives in `ReportComment.trimmed`, used by both. It also measures each character as the report's encoder writes it, so a comment full of quotes, backslashes, slashes or newlines (two bytes each in the JSON) is cut by what it really costs. Before, those were counted at one byte and twice as much was cut as needed. Only characters JSON may escape go through the encoder; the rest count as their UTF-8 size, which keeps it cheap for the windows that fit the report on every keystroke. One pass now always fits; the loop is only a guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
DeviceReportOutput.fittedcut the comment withunicodeScalars.dropLast(excess), whereexcessis a byte count. An emoji is four bytes, so it dropped up to four times what it had to. When the excess was bigger than the number of scalars, the comment was lost entirely. For example, 2000 emoji fitted to 4000 bytes came back with no comment.Now device and crash reports share one trim,
ReportComment.trimmed:fittedis only a guard.Tests
ReportCommentTests: emoji cut by their bytes, escaped characters at their JSON size, whole characters only, nothing left is nil.swift test: 344 tests, 0 failures. No warnings.