From 84224c0da4de1117b8d0aa4849072fa3e3f29c7e 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 1/5] 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. resizeGalleryImageURL, in the display pipeline, carried the same confusion: it sized the src-rewrite range from `imgElementStr.count`, so a gallery image's src could slip past the range and never be swapped for its resized URL. 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 and the gallery-image src rewrite — using astral emoji, ZWJ sequences, flags, keycaps, skin-tone modifiers, and an NFD combining mark, so reverting any single site breaks exactly one test. 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 +++-- ...RichContentFormatter+DisplayPipeline.swift | 2 +- .../RichContentFormatterTests.swift | 110 ++++++++++++++++++ .../RichContentFormatterUITests.swift | 18 +++ 4 files changed, 143 insertions(+), 16 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, "👨‍👩‍👧‍👦") + } } diff --git a/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift b/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift index 8efaddb06f04..758b20ee9dd8 100644 --- a/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift +++ b/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift @@ -7,4 +7,22 @@ class RichContentFormatterUITests: XCTestCase { func testResizeGalleryImageURLsForContentEmptyString() { XCTAssertTrue("" == RichContentFormatter.resizeGalleryImageURL("", isPrivateSite: false)) } + + // The gallery-image src rewrite sized its search range from the grapheme count + // (`imgElementStr.count`) rather than the UTF-16 length, so a `src` sitting past a + // multi-code-unit cluster fell outside the range and was never swapped for the resized + // URL. Here five emoji in `alt` (10 UTF-16 units, 5 graphemes) push the trailing `src` + // past a grapheme-count range; the resized URL must still replace it, cluster intact. + func testResizeGalleryImageURLReplacesSrcPastMultibyteCluster() { + let input = + "\"😀😀😀😀😀\"" + + let output = RichContentFormatter.resizeGalleryImageURL(input, isPrivateSite: false) + + // The original src was found and rewritten to a resized (Photon) URL... + XCTAssertFalse(output.contains("https://example.com/small.jpg")) + XCTAssertTrue(output.contains(".wp.com")) + // ...and the emoji cluster survived byte-for-byte. + XCTAssertTrue(output.contains("😀😀😀😀😀")) + } } From db04c6a685614abdf0aec4fedc76163657f7ef79 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:15:31 -0600 Subject: [PATCH 2/5] Migrate RichContentFormatter tests to Swift Testing Convert RichContentFormatterTests and RichContentFormatterUITests from XCTest to Swift Testing (@Test / #expect), matching the rest of the WordPressSharedTests target. Same inputs and assertions; no coverage change. --- .../RichContentFormatterTests.swift | 112 +++++++++--------- .../RichContentFormatterUITests.swift | 18 +-- 2 files changed, 68 insertions(+), 62 deletions(-) diff --git a/Modules/Tests/WordPressSharedTests/RichContentFormatterTests.swift b/Modules/Tests/WordPressSharedTests/RichContentFormatterTests.swift index 7688ca1ce588..d30600871880 100644 --- a/Modules/Tests/WordPressSharedTests/RichContentFormatterTests.swift +++ b/Modules/Tests/WordPressSharedTests/RichContentFormatterTests.swift @@ -1,69 +1,73 @@ -import XCTest +import Foundation +import Testing + @testable import WordPressShared -class RichContentFormatterTests: XCTestCase { +struct RichContentFormatterTests { - func testRemoveInlineStyles() { + @Test func testRemoveInlineStyles() { let str = "

test

test

" let styleStr = "

test

test

" let sanitizedStr = RichContentFormatter.removeInlineStyles(styleStr) - XCTAssertTrue(str == sanitizedStr, "The inline styles were not removed.") + #expect(str == sanitizedStr, "The inline styles were not removed.") } - func testRemoveForbiddenTags() { + @Test func testRemoveForbiddenTags() { let str = "

test

test

" - let styleStr = "

test

test

\n

" + let styleStr = + "

test

test

\n

" let sanitizedStr = RichContentFormatter.removeForbiddenTags(styleStr) - XCTAssertTrue(str == sanitizedStr, "The forbidden tags were not removed.") + #expect(str == sanitizedStr, "The forbidden tags were not removed.") } - func testNormalizeParagraphs() { + @Test func testNormalizeParagraphs() { let str = "

test

\n\ntest\n\n

test

" let styleStr = "

test

\n\ntest\n\n
\n

test

\n" let sanitizedStr = RichContentFormatter.normalizeParagraphs(styleStr) - XCTAssertTrue(str == sanitizedStr, "Not all paragraphs were normalized.") + #expect(str == sanitizedStr, "Not all paragraphs were normalized.") } - func testFilterNewLines() { + @Test func testFilterNewLines() { let str = "

test

\n\ntest\n\n

test

" let styleStr = "

test

\n\ntest\n\n
\n

test

\n" let sanitizedStr = RichContentFormatter.filterNewLines(styleStr) - XCTAssertTrue(str == sanitizedStr, "Not all paragraphs were normalized.") + #expect(str == sanitizedStr, "Not all paragraphs were normalized.") } - func testRemoveTrailingBRTags() { + @Test func testRemoveTrailingBRTags() { let str = "

test


test

" let styleStr = "

test


test



" let sanitizedStr = RichContentFormatter.removeTrailingBreakTags(styleStr) - XCTAssertTrue(str == sanitizedStr, "The inline styles were not removed.") + #expect(str == sanitizedStr, "The inline styles were not removed.") } - func testRemoveGutenbergGalleryListMarkup() { - let str = "Some text. Some text." + @Test func testRemoveGutenbergGalleryListMarkup() { + let str = + "Some text. Some text." let sanitizedString = RichContentFormatter.formatGutenbergGallery(str) as NSString // Checks if the UL was removed. var range = sanitizedString.range(of: "block-gallery") - XCTAssertTrue(range.location == NSNotFound) + #expect(range.location == NSNotFound) // Checks if the LI was removed range = sanitizedString.range(of: "blocks-gallery") - XCTAssertTrue(range.location == NSNotFound) + #expect(range.location == NSNotFound) // Checks if the FIGCAPTION was kept. range = sanitizedString.range(of: "figcaption") - XCTAssertTrue(range.location != NSNotFound) + #expect(range.location != NSNotFound) } - func testFormatVideoTags() { + @Test func testFormatVideoTags() { let str1 = "

Some text.

Some text.

" let sanitizedStr1 = RichContentFormatter.formatVideoTags(str1) as NSString - XCTAssert(sanitizedStr1.contains("controls")) + #expect(sanitizedStr1.contains("controls")) let str2 = "

Some text.

Some text.

" let sanitizedStr2 = RichContentFormatter.formatVideoTags(str2) as NSString - XCTAssert(sanitizedStr2.contains(" controls ")) + #expect(sanitizedStr2.contains(" controls ")) let str3 = "

Some text.

Some text.

" let sanitizedStr3 = RichContentFormatter.formatVideoTags(str3) as NSString - XCTAssert(!sanitizedStr3.contains("controls controls")) + #expect(!sanitizedStr3.contains("controls controls")) } // MARK: - Multi-code-unit input @@ -74,105 +78,105 @@ class RichContentFormatterTests: XCTestCase { // a token near the end of the string just past the range, so the search never reaches it. // Each test drives one such spot; the exact-output check also confirms the cluster is intact. - func testRegionalFlagStyleBlockSurvivesInTail() { + @Test func testRegionalFlagStyleBlockSurvivesInTail() { // A ") - XCTAssertEqual(out, "🇺🇸hi") + #expect(out == "🇺🇸hi") } - func testZWJFamilyScriptTagSurvivesInTail() { + @Test func testZWJFamilyScriptTagSurvivesInTail() { // A ") - XCTAssertEqual(out, "👨‍👩‍👧‍👦") + #expect(out == "👨‍👩‍👧‍👦") } - func testKeycapGutenbergCommentSurvivesInTail() { + @Test func testKeycapGutenbergCommentSurvivesInTail() { // A Gutenberg block comment after a keycap emoji is stripped. let out = RichContentFormatter.removeForbiddenTags("1️⃣

") - XCTAssertEqual(out, "1️⃣") + #expect(out == "1️⃣") } - func testSkinToneDivStartNotConvertedInTail() { + @Test func testSkinToneDivStartNotConvertedInTail() { //
is converted to

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

") - XCTAssertEqual(out, "👍🏽

") + #expect(out == "👍🏽

") } - func testNFDCombiningDivEndNotConvertedInTail() { + @Test 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}

") + #expect(out == "cafe\u{301}

") } - func testNormalizeParagraphsMergesTrailingDoubleOpenParagraph() { + @Test func testNormalizeParagraphsMergesTrailingDoubleOpenParagraph() { // A redundant

is collapsed to a single

. let out = RichContentFormatter.normalizeParagraphs("😀

") - XCTAssertEqual(out, "😀

") + #expect(out == "😀

") } - func testNormalizeParagraphsMergesTrailingDoubleCloseParagraph() { + @Test func testNormalizeParagraphsMergesTrailingDoubleCloseParagraph() { // A redundant

is collapsed to a single

. let out = RichContentFormatter.normalizeParagraphs("😀

") - XCTAssertEqual(out, "😀

") + #expect(out == "😀

") } - func testFilterNewLinesNoPreFallbackRemovesNewlinePastWideCluster() { + @Test func testFilterNewLinesNoPreFallbackRemovesNewlinePastWideCluster() { // A newline outside any
 block is removed.
         let out = RichContentFormatter.filterNewLines("👨‍👩‍👧‍👦\nA")
-        XCTAssertEqual(out, "👨‍👩‍👧‍👦A")
+        #expect(out == "👨‍👩‍👧‍👦A")
     }
 
-    func testFilterNewLinesElseBranchPreservesTrailingNewlineAfterWideCluster() {
+    @Test 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") + #expect(out == "
\n
👨‍👩‍👧‍👦Z") } - func testFilterNewLinesMultiPreInverseRanges() { + @Test 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
🇺🇸") + #expect(out == "👨‍👩‍👧‍👦
a\nb
😀
c\nd
🇺🇸") } - func testZWJFamilyStyleAttrSurvivesInTruncatedTail() { + @Test func testZWJFamilyStyleAttrSurvivesInTruncatedTail() { // An inline style attribute after a family emoji is stripped. let out = RichContentFormatter.removeInlineStyles("👨‍👩‍👧‍👦
") - XCTAssertEqual(out, "👨‍👩‍👧‍👦
") + #expect(out == "👨‍👩‍👧‍👦
") } - func testZWJFamilyTrailingBreakSurvivesAndCutsCleanly() { + @Test 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") + #expect(out == "👨‍👩‍👧‍👦text") } - func testTrailingBreakOnlyFinalRemovedEmojiIntact() { + @Test func testTrailingBreakOnlyFinalRemovedEmojiIntact() { // Only the trailing
is removed; an earlier
in the middle of the text stays. let out = RichContentFormatter.removeTrailingBreakTags("😀
text
") - XCTAssertEqual(out, "😀
text") + #expect(out == "😀
text") } - func testForbiddenCleanMultibyteUnchanged() { + @Test func testForbiddenCleanMultibyteUnchanged() { // Content with no tags to strip passes through unchanged. let out = RichContentFormatter.removeForbiddenTags("Hello 👨‍👩‍👧‍👦 world 😀!") - XCTAssertEqual(out, "Hello 👨‍👩‍👧‍👦 world 😀!") + #expect(out == "Hello 👨‍👩‍👧‍👦 world 😀!") } // MARK: - Boundary + selectivity (not new fix sites) - func testBoundaryStraddleOffByOne() { + @Test 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") + #expect(out == "text") } - func testStripsTagInRangeAndInTailNotJustEverything() { + @Test 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, "👨‍👩‍👧‍👦") + #expect(out == "👨‍👩‍👧‍👦") } } diff --git a/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift b/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift index 758b20ee9dd8..b123ae42bb50 100644 --- a/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift +++ b/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift @@ -1,11 +1,13 @@ -import XCTest +import Foundation +import Testing + @testable import WordPressShared @testable import WordPressSharedUI -class RichContentFormatterUITests: XCTestCase { +struct RichContentFormatterUITests { - func testResizeGalleryImageURLsForContentEmptyString() { - XCTAssertTrue("" == RichContentFormatter.resizeGalleryImageURL("", isPrivateSite: false)) + @Test func testResizeGalleryImageURLsForContentEmptyString() { + #expect(RichContentFormatter.resizeGalleryImageURL("", isPrivateSite: false).isEmpty) } // The gallery-image src rewrite sized its search range from the grapheme count @@ -13,16 +15,16 @@ class RichContentFormatterUITests: XCTestCase { // multi-code-unit cluster fell outside the range and was never swapped for the resized // URL. Here five emoji in `alt` (10 UTF-16 units, 5 graphemes) push the trailing `src` // past a grapheme-count range; the resized URL must still replace it, cluster intact. - func testResizeGalleryImageURLReplacesSrcPastMultibyteCluster() { + @Test func testResizeGalleryImageURLReplacesSrcPastMultibyteCluster() { let input = "\"😀😀😀😀😀\"" let output = RichContentFormatter.resizeGalleryImageURL(input, isPrivateSite: false) // The original src was found and rewritten to a resized (Photon) URL... - XCTAssertFalse(output.contains("https://example.com/small.jpg")) - XCTAssertTrue(output.contains(".wp.com")) + #expect(!output.contains("https://example.com/small.jpg")) + #expect(output.contains(".wp.com")) // ...and the emoji cluster survived byte-for-byte. - XCTAssertTrue(output.contains("😀😀😀😀😀")) + #expect(output.contains("😀😀😀😀😀")) } } From 123548809b094b2ac823dcc0c9a9e4698d4c0bc2 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:34:56 -0600 Subject: [PATCH 3/5] Harden parseValueForAttribute against a missing closing quote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parseValueForAttribute located an attribute's closing quote and fed the result straight into substring(with:). When the closing quote is absent — malformed markup with an opening quote and no close — the search returns NSNotFound, so the range length underflowed to NSIntegerMax and crashed with an out-of-bounds NSRange. Guard on the closing quote and return "" when it's missing, matching the attribute-not-found default. --- .../Utility/RichContentFormatter.swift | 4 +++- .../RichContentFormatterTests.swift | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift b/Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift index 05e44922b3ca..01d2f4d3fec1 100644 --- a/Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift +++ b/Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift @@ -187,7 +187,9 @@ import Foundation let location = attrRange.location + attrRange.length let length = elementStr.length - location let ending = elementStr.range(of: "\"", options: .caseInsensitive, range: NSRange(location: location, length: length)) - value = elementStr.substring(with: NSRange(location: location, length: ending.location - location)) + if ending.location != NSNotFound { + value = elementStr.substring(with: NSRange(location: location, length: ending.location - location)) + } } return value diff --git a/Modules/Tests/WordPressSharedTests/RichContentFormatterTests.swift b/Modules/Tests/WordPressSharedTests/RichContentFormatterTests.swift index d30600871880..bdc88c7910d7 100644 --- a/Modules/Tests/WordPressSharedTests/RichContentFormatterTests.swift +++ b/Modules/Tests/WordPressSharedTests/RichContentFormatterTests.swift @@ -179,4 +179,23 @@ struct RichContentFormatterTests { let out = RichContentFormatter.removeInlineStyles("👨‍👩‍👧‍👦") #expect(out == "👨‍👩‍👧‍👦") } + + // MARK: - parseValueForAttribute robustness + + @Test func testParseValueForAttributeReturnsValue() { + let value = RichContentFormatter.parseValueForAttribute("src", inElement: "") + #expect(value == "http://x/a.jpg") + } + + @Test func testParseValueForAttributeMissingClosingQuoteReturnsEmpty() { + // Opening quote but no closing quote: the closing-quote search returns NSNotFound, so the + // range length would underflow to a huge value and crash substring(with:). Return "" instead. + let value = RichContentFormatter.parseValueForAttribute("src", inElement: "") + #expect(value.isEmpty) + } + + @Test func testParseValueForAttributeAbsentReturnsEmpty() { + let value = RichContentFormatter.parseValueForAttribute("src", inElement: "\"x\"") + #expect(value.isEmpty) + } } From 541611e8c911b9af39e197e3ac079a6e76c885fd Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:46:23 -0600 Subject: [PATCH 4/5] Harden RichContentFormatter sanitization beyond the UTF-16 range fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #25833 (the String.count/UTF-16 NSRange fix + the missing-closing-quote guard). Adds the remaining sanitization fixes: - Strip unclosed ", options: .caseInsensitive) + static let styleTags = try! NSRegularExpression(pattern: "]*?>[\\s\\S]*?(?:|$)", options: .caseInsensitive) + static let scriptTags = try! NSRegularExpression(pattern: "]*?>[\\s\\S]*?(?:|$)", options: .caseInsensitive) static let gutenbergComments = try! NSRegularExpression(pattern: "

[\\n]?", options: .caseInsensitive) // Normalizaing Paragraphs @@ -19,10 +19,10 @@ import Foundation static let pTagsEnd = try! NSRegularExpression(pattern: "

\\s*

", options: .caseInsensitive) static let newLines = try! NSRegularExpression(pattern: "\\n", options: .caseInsensitive) static let preTags = try! NSRegularExpression(pattern: "]*>[\\s\\S]*?
", options: .caseInsensitive) - static let videoTags = try! NSRegularExpression(pattern: "]*>", options: .caseInsensitive) + static let videoTags = try! NSRegularExpression(pattern: "]*)?>", options: .caseInsensitive) // Inline Styles - static let styleAttr = try! NSRegularExpression(pattern: "\\s*style=\"[^\"]*\"", options: .caseInsensitive) + static let styleAttr = try! NSRegularExpression(pattern: "\\s+style=(?:\"[^\"]*\"|'[^']*')", options: .caseInsensitive) // Gallery Images public static let galleryImgTags = try! NSRegularExpression(pattern: "]*data-orig-file[^>]*/>", options: .caseInsensitive) @@ -178,21 +178,20 @@ import Foundation /// - Returns: The value for the attribute or an empty string.. /// @objc public class func parseValueForAttribute(_ attribute: String, inElement element: String) -> String { - let elementStr = element as NSString - var value = "" - let attrStr = "\(attribute)=\"" - let attrRange = elementStr.range(of: attrStr) - - if attrRange.location != NSNotFound { - let location = attrRange.location + attrRange.length - let length = elementStr.length - location - let ending = elementStr.range(of: "\"", options: .caseInsensitive, range: NSRange(location: location, length: length)) - if ending.location != NSNotFound { - value = elementStr.substring(with: NSRange(location: location, length: ending.location - location)) - } + // Match the attribute name on a word boundary (so "rc" does not match inside "src") + // and capture its double-quoted value. A missing closing quote fails to match, so there + // is no out-of-bounds range to crash on. + let escaped = NSRegularExpression.escapedPattern(for: attribute) + guard let regex = try? NSRegularExpression(pattern: "(? Date: Wed, 2 Sep 2026 21:46:23 -0600 Subject: [PATCH 5/5] Add a comprehensive RichContentFormatter sanitization test suite ~75 parameterized Swift Testing cases across every sanitization method (quote, whitespace, and case variants, multiple matches, empty input, no-ops), plus an iOS srcset-preservation regression test for resizeGalleryImageURL. --- ...ichContentFormatterSanitizationTests.swift | 292 ++++++++++++++++++ .../RichContentFormatterUITests.swift | 9 + 2 files changed, 301 insertions(+) create mode 100644 Modules/Tests/WordPressSharedTests/RichContentFormatterSanitizationTests.swift diff --git a/Modules/Tests/WordPressSharedTests/RichContentFormatterSanitizationTests.swift b/Modules/Tests/WordPressSharedTests/RichContentFormatterSanitizationTests.swift new file mode 100644 index 000000000000..46e6cc4c7601 --- /dev/null +++ b/Modules/Tests/WordPressSharedTests/RichContentFormatterSanitizationTests.swift @@ -0,0 +1,292 @@ +import Foundation +import Testing + +@testable import WordPressShared + +/// Behavioural coverage for `RichContentFormatter`'s platform-independent text +/// transformations — the regex-driven sanitisation that runs over untrusted +/// remote post and comment HTML. +/// +/// These methods live in `WordPressShared`, so this suite runs under `swift test` +/// on macOS with no simulator. +@Suite("RichContentFormatter sanitization") +struct RichContentFormatterSanitizationTests { + + // MARK: - removeForbiddenTags + + @Suite("removeForbiddenTags") + struct RemoveForbiddenTags { + @Test( + "strips script, style, and Gutenberg-comment paragraphs", + arguments: [ + // Basic script/style removal. + ("Hello", "Hello"), + ("Hello", "Hello"), + // Case-insensitive. + ("Hello", "Hello"), + ("Hello", "Hello"), + // Attributes on the opening tag. + ("Hi", "Hi"), + // Newlines inside the element body. + ("Hi", "Hi"), + // Multiple occurrences. + ("abc", "abc"), + ("abc", "abc"), + // Gutenberg block comments wrapped in

, with and without trailing newline. + ("

\nHi", "Hi"), + ("

Hi", "Hi"), + ("

Hi", "Hi"), + // An unclosed forbidden tag is stripped through the end of the input. + ("", "😀😀😀😀😀"), + // No-ops. + ("

plain paragraph

", "

plain paragraph

"), + ("", "") + ] + ) + func strips(input: String, expected: String) { + #expect(RichContentFormatter.removeForbiddenTags(input) == expected) + } + } + + // MARK: - removeInlineStyles + + @Suite("removeInlineStyles") + struct RemoveInlineStyles { + @Test( + "strips double-quoted style attributes and the whitespace before them", + arguments: [ + ("

x

", "

x

"), + ("

x

", "

x

"), + ("

x

", "

x

"), + // Leading whitespace is consumed with the attribute. + ("
y", "y"), + // Multiple styled elements. + ("

1

2

", "

1

2

"), + // Single-quoted styles are stripped too, leaving other attributes intact. + ("

x

", "

x

"), + ("y", "y"), + // Attribute names ending in `style` are NOT corrupted (left boundary). + ("", ""), + ("
t
", "
t
"), + // Non-BMP prefix must not shrink the UTF-16 search range. + ("😀

t

", "😀

t

"), + // No-ops. + ("

no style here

", "

no style here

"), + ("", "") + ] + ) + func strips(input: String, expected: String) { + #expect(RichContentFormatter.removeInlineStyles(input) == expected) + } + } + + // MARK: - normalizeParagraphs + + @Suite("normalizeParagraphs") + struct NormalizeParagraphs { + @Test( + "converts DIVs to Ps, collapses redundant Ps, and drops non-PRE newlines", + arguments: [ + // Anchor case from the original suite. + ( + "

test

\n\ntest\n\n
\n

test

\n", + "

test

\n\ntest\n\n

test

" + ), + // Simple div -> p. + ("
x
", "

x

"), + // Div with attributes. + ("
x
", "

x

"), + // Already-normal paragraphs are left alone. + ("

a

b

", "

a

b

"), + // Non-BMP prefix must not desync the div->p conversion (balanced tags). + ("😀😀😀😀😀
x
", "😀😀😀😀😀

x

"), + ("", "") + ] + ) + func normalizes(input: String, expected: String) { + #expect(RichContentFormatter.normalizeParagraphs(input) == expected) + } + } + + // MARK: - filterNewLines + + @Suite("filterNewLines") + struct FilterNewLines { + @Test( + "removes newlines except inside
 blocks",
+            arguments: [
+                // No PRE: every newline goes.
+                ("a\nb\nc", "abc"),
+                ("\n\n\n", ""),
+                // Newlines inside PRE are preserved; those outside are removed.
+                ("
a\nb
", "
a\nb
"), + ("x\n
a\nb
\ny", "x
a\nb
y"), + // Multiple PRE blocks. + ("
1\n2
\n
3\n4
\n", "
1\n2
3\n4
"), + ("", "") + ] + ) + func filters(input: String, expected: String) { + #expect(RichContentFormatter.filterNewLines(input) == expected) + } + + @Test("drops a newline after a non-BMP character (UTF-16-correct NSRange)") + func dropsNewlineAfterNonBMP() { + // "😀" is one Character but two UTF-16 code units; the range must use the UTF-16 + // length or the trailing newline falls outside it and is left in place. + #expect(RichContentFormatter.filterNewLines("ab😀\n") == "ab😀") + } + } + + // MARK: - removeTrailingBreakTags + + @Suite("removeTrailingBreakTags") + struct RemoveTrailingBreakTags { + @Test( + "trims trailing
runs (and surrounding whitespace) but keeps interior ones", + arguments: [ + // Anchor case. + ("

test


test



", "

test


test

"), + // Single trailing break, various spellings. + ("text
", "text"), + ("text
", "text"), + ("text
", "text"), + ("text
", "text"), + // Runs. + ("text


", "text"), + ("

", ""), + // Interior break is preserved. + ("a
b", "a
b"), + // Non-BMP prefix: trailing
still trimmed, and no out-of-bounds crash. + ("😀
", "😀"), + ("😀😀😀😀😀
", "😀😀😀😀😀"), + // Leading/trailing whitespace is trimmed even with no break. + (" spaced ", "spaced"), + ("no breaks", "no breaks"), + ("", "") + ] + ) + func trims(input: String, expected: String) { + #expect(RichContentFormatter.removeTrailingBreakTags(input) == expected) + } + } + + // MARK: - formatVideoTags + + @Suite("formatVideoTags") + struct FormatVideoTags { + @Test( + "adds a controls attribute only when absent", + arguments: [ + ("

x

y

", "

x

y

"), + ("", ""), + // Already has controls -> untouched. + ("", ""), + // `controls` inside an attribute value/name is not the controls attribute. + ( + "", + "" + ), + ("", ""), + ("", ""), + // Existing CONTROLS (case-insensitive) is not duplicated. + ("", ""), + // Must not over-match a different element. + ("", ""), + // No video -> untouched. + ("

no video

", "

no video

"), + ("", "") + ] + ) + func addsControls(input: String, expected: String) { + #expect(RichContentFormatter.formatVideoTags(input) == expected) + } + + @Test("preserves the opening tag's original casing when inserting controls") + func preservesOpeningTagCasing() { + #expect(RichContentFormatter.formatVideoTags("") == "") + } + } + + // MARK: - parseValueForAttribute + + @Suite("parseValueForAttribute") + struct ParseValueForAttribute { + @Test( + "returns the double-quoted value of an attribute, or empty when absent", + arguments: [ + ("src", "", "http://example.com/a.jpg"), + ( + "data-orig-file", "", + "http://example.com/o.jpg" + ), + // Missing attribute. + ("href", "", ""), + // Present but empty. + ("src", "", ""), + // First match wins. + ("src", "", "a") + ] + ) + func parses(attribute: String, element: String, expected: String) { + #expect(RichContentFormatter.parseValueForAttribute(attribute, inElement: element) == expected) + } + + @Test("Regression: an unterminated attribute quote returns empty instead of crashing") + func unterminatedQuoteReturnsEmpty() { + // Previously `ending.location == NSNotFound` was fed into `substringWithRange:`, + // throwing NSInvalidArgumentException. Reachable from adversarial gallery HTML via + // `resizeGalleryImageURL`. Now guarded to return "". + #expect(RichContentFormatter.parseValueForAttribute("src", inElement: "").isEmpty) + } + + @Test("does not match an attribute name as a substring of another attribute") + func doesNotMatchSubstringAttributeName() { + // "rc" must not match inside "src", nor "orig-file" inside "data-orig-file". + #expect(RichContentFormatter.parseValueForAttribute("rc", inElement: "").isEmpty) + #expect( + RichContentFormatter.parseValueForAttribute("orig-file", inElement: "") + .isEmpty + ) + } + } + + // MARK: - formatGutenbergGallery + + @Suite("formatGutenbergGallery") + struct FormatGutenbergGallery { + @Test( + "leaves non-gallery content untouched", + arguments: [ + "

hello

", + "
  • plain list
", + "" + ] + ) + func noOp(input: String) { + #expect(RichContentFormatter.formatGutenbergGallery(input) == input) + } + + @Test("removes gallery UL/LI markup while keeping the figures inside") + func stripsGalleryMarkupKeepsFigures() { + let input = + "Before. After." + let output = RichContentFormatter.formatGutenbergGallery(input) as NSString + + #expect(output.range(of: "block-gallery").location == NSNotFound) + #expect(output.range(of: "blocks-gallery").location == NSNotFound) + #expect(output.range(of: "
").location != NSNotFound) + #expect(output.range(of: "figcaption").location != NSNotFound) + #expect(output.range(of: "https://example.com/1.jpg").location != NSNotFound) + #expect(output.range(of: "https://example.com/2.jpg").location != NSNotFound) + } + } +} diff --git a/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift b/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift index b123ae42bb50..2fbf53a960c5 100644 --- a/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift +++ b/Modules/Tests/WordPressSharedTests/RichContentFormatterUITests.swift @@ -27,4 +27,13 @@ struct RichContentFormatterUITests { // ...and the emoji cluster survived byte-for-byte. #expect(output.contains("😀😀😀😀😀")) } + + @Test func testResizeGalleryImageURLLeavesSrcsetIntact() { + // The src value also appears in srcset; only the src attribute should be rewritten. + let srcset = "srcset=\"https://example.com/a.jpg 1x, https://example.com/b.jpg 2x\"" + let input = + "" + let output = RichContentFormatter.resizeGalleryImageURL(input, isPrivateSite: false) + #expect(output.contains(srcset), "srcset must be left intact when the src is resized") + } }