From ea3df4b38171a64c4911d4e915fb9e726b7129f3 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:35:56 -0600 Subject: [PATCH] Fix grapheme-vs-UTF-16 range bug in RichContentFormatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RichContentFormatter built its NSRanges from `content.count` (Swift grapheme count), but NSRegularExpression matches over UTF-16. With multi-code-unit characters (emoji, flags, combining sequences) the grapheme count is shorter than the UTF-16 length, so the search range was truncated and any tag or style near the end silently escaped stripping. removeTrailingBreakTags also fed a UTF-16 match offset to String.index(_:offsetBy:), which counts graphemes — right for ASCII, a crash once the range was corrected. Range over UTF-16 via `String.utf16.count`, and convert the trailing-BR match with Range(_:in:). Adds one isolated test per fix site — each forbidden-tag, div/paragraph, filterNewLines, inline-style, and trailing-break site, plus the trailing-break index-offset cut — using astral emoji, ZWJ sequences, flags, keycaps, skin-tone modifiers, and an NFD combining mark, so reverting any single site breaks exactly one test (verified by reverting each). Two further tests pin the exact off-by-one boundary and confirm the corrected range strips the intended tag rather than everything. Each fails on the old code and passes now, and the exact-output assertions confirm the clusters survive byte-for-byte. --- .../Utility/RichContentFormatter.swift | 29 +++-- .../RichContentFormatterTests.swift | 110 ++++++++++++++++++ 2 files changed, 124 insertions(+), 15 deletions(-) diff --git a/Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift b/Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift index 38530a2d2043..05e44922b3ca 100644 --- a/Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift +++ b/Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift @@ -50,17 +50,17 @@ import Foundation content = RegEx.styleTags.stringByReplacingMatches(in: content, options: .reportCompletion, - range: NSRange(location: 0, length: content.count), + range: NSRange(location: 0, length: content.utf16.count), withTemplate: "") content = RegEx.scriptTags.stringByReplacingMatches(in: content, options: .reportCompletion, - range: NSRange(location: 0, length: content.count), + range: NSRange(location: 0, length: content.utf16.count), withTemplate: "") content = RegEx.gutenbergComments.stringByReplacingMatches(in: content, options: .reportCompletion, - range: NSRange(location: 0, length: content.count), + range: NSRange(location: 0, length: content.utf16.count), withTemplate: "") return content @@ -84,23 +84,23 @@ import Foundation // Convert div tags to p tags content = RegEx.divTagsStart.stringByReplacingMatches(in: content, options: .reportCompletion, - range: NSRange(location: 0, length: content.count), + range: NSRange(location: 0, length: content.utf16.count), withTemplate: openPTag) content = RegEx.divTagsEnd.stringByReplacingMatches(in: content, options: .reportCompletion, - range: NSRange(location: 0, length: content.count), + range: NSRange(location: 0, length: content.utf16.count), withTemplate: closePTag) // Remove duplicate/redundant p tags. content = RegEx.pTagsStart.stringByReplacingMatches(in: content, options: .reportCompletion, - range: NSRange(location: 0, length: content.count), + range: NSRange(location: 0, length: content.utf16.count), withTemplate: openPTag) content = RegEx.pTagsEnd.stringByReplacingMatches(in: content, options: .reportCompletion, - range: NSRange(location: 0, length: content.count), + range: NSRange(location: 0, length: content.utf16.count), withTemplate: closePTag) content = filterNewLines(content) @@ -114,11 +114,11 @@ import Foundation var ranges = [NSRange]() // We don't want to remove new lines from preformatted tag blocks, // so get the ranges of such blocks. - let matches = RegEx.preTags.matches(in: content, options: .reportCompletion, range: NSRange(location: 0, length: content.count)) + let matches = RegEx.preTags.matches(in: content, options: .reportCompletion, range: NSRange(location: 0, length: content.utf16.count)) if matches.isEmpty { // No blocks found, so we'll parse the whole string. - ranges.append(NSRange(location: 0, length: content.count)) + ranges.append(NSRange(location: 0, length: content.utf16.count)) } else { // One or more preformatted blocks found, we don't want to remove new lines @@ -133,7 +133,7 @@ import Foundation location = match.range.location + match.range.length } - length = content.count - location + length = content.utf16.count - location ranges.append(NSRange(location: location, length: length)) } @@ -163,7 +163,7 @@ import Foundation content = RegEx.styleAttr.stringByReplacingMatches(in: content, options: .reportCompletion, - range: NSRange(location: 0, length: content.count), + range: NSRange(location: 0, length: content.utf16.count), withTemplate: "") return content @@ -206,10 +206,9 @@ import Foundation } var content = string.trim() - let matches = RegEx.trailingBRTags.matches(in: content, options: .reportCompletion, range: NSRange(location: 0, length: content.count)) - if let match = matches.first { - let index = content.index(content.startIndex, offsetBy: match.range.location) - content = String(content.prefix(upTo: index)) + let matches = RegEx.trailingBRTags.matches(in: content, options: .reportCompletion, range: NSRange(location: 0, length: content.utf16.count)) + if let match = matches.first, let matchRange = Range(match.range, in: content) { + content = String(content[.. block after a flag emoji is stripped; the neighbouring stays. + let out = RichContentFormatter.removeForbiddenTags("🇺🇸hi") + XCTAssertEqual(out, "🇺🇸hi") + } + + func testZWJFamilyScriptTagSurvivesInTail() { + // A ") + XCTAssertEqual(out, "👨‍👩‍👧‍👦") + } + + func testKeycapGutenbergCommentSurvivesInTail() { + // A Gutenberg block comment after a keycap emoji is stripped. + let out = RichContentFormatter.removeForbiddenTags("1️⃣

") + XCTAssertEqual(out, "1️⃣") + } + + func testSkinToneDivStartNotConvertedInTail() { + //
is converted to

even after a skin-tone emoji. + let out = RichContentFormatter.normalizeParagraphs("👍🏽

") + XCTAssertEqual(out, "👍🏽

") + } + + func testNFDCombiningDivEndNotConvertedInTail() { + //

is converted to

after a decomposed "é" (e + a combining accent). A composed + // "é" is a single UTF-16 unit and would not reach past the range, so the decomposition matters. + let out = RichContentFormatter.normalizeParagraphs("cafe\u{301}
") + XCTAssertEqual(out, "cafe\u{301}

") + } + + func testNormalizeParagraphsMergesTrailingDoubleOpenParagraph() { + // A redundant

is collapsed to a single

. + let out = RichContentFormatter.normalizeParagraphs("😀

") + XCTAssertEqual(out, "😀

") + } + + func testNormalizeParagraphsMergesTrailingDoubleCloseParagraph() { + // A redundant

is collapsed to a single

. + let out = RichContentFormatter.normalizeParagraphs("😀

") + XCTAssertEqual(out, "😀

") + } + + func testFilterNewLinesNoPreFallbackRemovesNewlinePastWideCluster() { + // A newline outside any 
 block is removed.
+        let out = RichContentFormatter.filterNewLines("👨‍👩‍👧‍👦\nA")
+        XCTAssertEqual(out, "👨‍👩‍👧‍👦A")
+    }
+
+    func testFilterNewLinesElseBranchPreservesTrailingNewlineAfterWideCluster() {
+        // With a 
 block present, a newline that follows it (outside the block) is still removed.
+        let out = RichContentFormatter.filterNewLines("
\n
👨‍👩‍👧‍👦\nZ")
+        XCTAssertEqual(out, "
\n
👨‍👩‍👧‍👦Z")
+    }
+
+    func testFilterNewLinesMultiPreInverseRanges() {
+        // Across several 
 blocks: newlines inside them are kept, newlines outside are removed.
+        let out = RichContentFormatter.filterNewLines("👨‍👩‍👧‍👦\n
a\nb
\n😀\n
c\nd
\n🇺🇸\n")
+        XCTAssertEqual(out, "👨‍👩‍👧‍👦
a\nb
😀
c\nd
🇺🇸")
+    }
+
+    func testZWJFamilyStyleAttrSurvivesInTruncatedTail() {
+        // An inline style attribute after a family emoji is stripped.
+        let out = RichContentFormatter.removeInlineStyles("👨‍👩‍👧‍👦
")
+        XCTAssertEqual(out, "👨‍👩‍👧‍👦
")
+    }
+
+    func testZWJFamilyTrailingBreakSurvivesAndCutsCleanly() {
+        // A trailing 
 after a family emoji is removed, and the emoji before it stays intact.
+        let out = RichContentFormatter.removeTrailingBreakTags("👨‍👩‍👧‍👦text
")
+        XCTAssertEqual(out, "👨‍👩‍👧‍👦text")
+    }
+
+    func testTrailingBreakOnlyFinalRemovedEmojiIntact() {
+        // Only the trailing 
 is removed; an earlier 
 in the middle of the text stays.
+        let out = RichContentFormatter.removeTrailingBreakTags("😀
text
")
+        XCTAssertEqual(out, "😀
text")
+    }
+
+    func testForbiddenCleanMultibyteUnchanged() {
+        // Content with no tags to strip passes through unchanged.
+        let out = RichContentFormatter.removeForbiddenTags("Hello 👨‍👩‍👧‍👦 world 😀!")
+        XCTAssertEqual(out, "Hello 👨‍👩‍👧‍👦 world 😀!")
+    }
+
+    // MARK: - Boundary + selectivity (not new fix sites)
+
+    func testBoundaryStraddleOffByOne() {
+        // One emoji makes the range exactly one UTF-16 unit short, and the token's closing ">"
+        // is exactly that dropped unit — pins the off-by-one where the wide-gap cases have slack.
+        let out = RichContentFormatter.removeForbiddenTags("text")
+        XCTAssertEqual(out, "text")
+    }
+
+    func testStripsTagInRangeAndInTailNotJustEverything() {
+        // The first style attribute is always in range; the ZWJ family pushes the second into the
+        // truncated tail. The fix strips both; the bug strips only the first — so the range, not a
+        // blanket "strip everything", decides which tags go.
+        let out = RichContentFormatter.removeInlineStyles("👨‍👩‍👧‍👦")
+        XCTAssertEqual(out, "👨‍👩‍👧‍👦")
+    }
 }