Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ All notable changes to this project will be documented in this file. Take a look
* The deprecated `ReadiumAdapterGCDWebServer` and `ReadiumAdapterLCPSQLite` adapter packages have been removed.
* The `ReadiumInternal` package has been removed. Its utilities were internal helpers and are now folded into `ReadiumShared` with `package` visibility. If you imported `ReadiumInternal` directly, remove the import.

### Fixed

#### Shared

* [#876](https://github.com/readium/swift-toolkit/issues/876) Fixed a crash (`String index is out of bounds`) in `ContentSearchService` when searching a publication containing characters merging with the surrounding text, such as combining diacritical marks.


<!-- ## [Unreleased] -->

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,18 +314,24 @@ private actor Iterator: SearchIterator, Loggable {

guard !entryText.isEmpty else { return }

let startOffset: Int
if windowText.isEmpty {
startOffset = 0
} else {
if !windowText.isEmpty {
// Space separator owned by this (following) entry.
windowText.append(" ")
windowTextCount += 1
startOffset = windowTextCount
}

windowText.append(contentsOf: entryText)
windowTextCount += entryText.count

// `windowTextCount` is recomputed from `windowText` instead of adding
// the appended counts: Swift regroups grapheme clusters across the
// junction (e.g. an entry starting with a combining mark merges with
// the separator space), so the sum could exceed the real character
// count and offset arithmetic would fall out of the window's bounds.
//
// For the same reason, `startOffset` is derived from the final count
// rather than from the separator's position: when the separator merged
// with the entry's first character, the merged cluster belongs to the
// entry. This keeps `startOffset + text.count == windowTextCount`.
windowTextCount = windowText.count
let startOffset = windowTextCount - entryText.count

entries.append(ElementEntry(
text: entryText,
Expand All @@ -334,6 +340,26 @@ private actor Iterator: SearchIterator, Loggable {
))
}

/// Returns the index in `windowText` at the given character offset.
///
/// Offsets are derived from `windowTextCount`, which is recomputed from
/// `windowText` on every mutation of the window, so they are expected to
/// be in bounds. As a safety net against a crash in a user's library (see
/// issue #876), an out-of-bounds offset is logged and returns `nil`
/// instead of trapping.
private func windowIndex(at offset: Int) -> String.Index? {
guard
// `limitedBy` only bounds the forward walk: a negative offset would
// still trap.
offset >= 0,
let index = windowText.index(windowText.startIndex, offsetBy: offset, limitedBy: windowText.endIndex)
else {
log(.error, "Window offset \(offset) is out of bounds (window count: \(windowText.count))")
return nil
}
return index
}

/// Resets the window for a new resource.
private func resetWindow() {
entries = []
Expand Down Expand Up @@ -421,7 +447,10 @@ private actor Iterator: SearchIterator, Loggable {
trimAmount = 0
}

guard trimAmount > 0 else { return }
guard
trimAmount > 0,
let trimIdx = windowIndex(at: trimAmount)
else { return }

// Drop leading entries.
entries.removeFirst(dropCount)
Expand All @@ -434,9 +463,8 @@ private actor Iterator: SearchIterator, Loggable {
searchCeiling -= trimAmount

// Drop prefix from windowText.
let trimIdx = windowText.index(windowText.startIndex, offsetBy: trimAmount)
windowText = String(windowText[trimIdx...])
windowTextCount -= trimAmount
windowTextCount = windowText.count
isAtResourceStart = false
}

Expand All @@ -462,8 +490,12 @@ private actor Iterator: SearchIterator, Loggable {
private func search(dangerZoneCapacity: Int) async -> [Locator] {
guard searchCeiling > searchFloor else { return [] }

let sliceStart = windowText.index(windowText.startIndex, offsetBy: searchFloor)
let sliceEnd = windowText.index(windowText.startIndex, offsetBy: searchCeiling)
guard
let sliceStart = windowIndex(at: searchFloor),
let sliceEnd = windowIndex(at: searchCeiling)
else {
return []
}
let searchSlice = String(windowText[sliceStart ..< sliceEnd])

let ranges = await searchAlgorithm.findRanges(
Expand Down Expand Up @@ -507,8 +539,13 @@ private actor Iterator: SearchIterator, Loggable {
return nil
}

let highlightStart = windowText.index(windowText.startIndex, offsetBy: matchStart)
let highlightEnd = windowText.index(windowText.startIndex, offsetBy: matchEnd)
guard
let highlightStart = windowIndex(at: matchStart),
let highlightEnd = windowIndex(at: matchEnd)
else {
return nil
}

let highlight = String(
windowText[highlightStart ..< highlightEnd]
)
Expand Down Expand Up @@ -563,11 +600,14 @@ private actor Iterator: SearchIterator, Loggable {
/// Returns `nil` if the match is at the very beginning of a resource and
/// the resulting text is empty after trimming.
private func extractSnippetBefore(matchStart: Int) -> String? {
guard matchStart > 0 else {
guard
matchStart > 0,
let matchStartIndex = windowIndex(at: matchStart)
else {
return nil
}

let available = windowText[windowText.startIndex ..< windowText.index(windowText.startIndex, offsetBy: matchStart)]
let available = windowText[windowText.startIndex ..< matchStartIndex]

var chars: [Character] = []
var count = snippetLength
Expand Down Expand Up @@ -600,11 +640,13 @@ private actor Iterator: SearchIterator, Loggable {
/// Returns `nil` if the match is at the very end of a resource and the
/// resulting text is empty after trimming.
private func extractSnippetAfter(matchEnd: Int) -> String? {
guard matchEnd < windowTextCount else {
guard
matchEnd < windowTextCount,
let afterStart = windowIndex(at: matchEnd)
else {
return nil
}

let afterStart = windowText.index(windowText.startIndex, offsetBy: matchEnd)
let available = windowText[afterStart...]

var result = ""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
//
// Copyright 2026 Readium Foundation. All rights reserved.
// Use of this source code is governed by the BSD-style license
// available in the top-level LICENSE file of the project.
//

@testable import ReadiumShared
import Testing

enum ContentSearchServiceTests {
/// Regression tests for https://github.com/readium/swift-toolkit/issues/876
///
/// When an element's text begins with a character that merges with the
/// preceding grapheme cluster (e.g. a combining mark merging with the
/// window's space separator), the sliding window's cached character count
/// used to run ahead of the real `windowText.count`, and offset arithmetic
/// trapped with "String index is out of bounds".
struct GraphemeClusterBoundaries {
/// A publication made of one `TextContentElement` per array of segment
/// texts, containing the query "夜灯" twice.
struct Fixture: CustomTestStringConvertible {
let name: String
let elements: [[String]]

var testDescription: String {
name
}
}

static let fixtures: [Fixture] = [
Fixture(
name: "plain content (control)",
elements: [
["夜灯亮着,街角安静。"],
["猫在窗台上打盹,夜灯映出影子。"],
]
),
Fixture(
name: "element starting with a combining mark",
elements: [
["夜灯亮着,街角安静。"],
["\u{0301}猫在窗台上打盹,夜灯映出影子。"],
]
),
Fixture(
name: "element starting with a variation selector",
elements: [
["夜灯亮着,街角安静。"],
["\u{FE0F}猫在窗台上打盹,夜灯映出影子。"],
]
),
Fixture(
name: "segment starting with a combining mark",
elements: [
["夜灯亮着,街角安静", "\u{0301},猫在窗台上打盹,夜灯映出影子。"],
]
),
]

@Test(arguments: fixtures)
func findsAllMatches(fixture: Fixture) async throws {
let results = try await search(query: "夜灯", elements: fixture.elements)

#expect(results.count == 2)
#expect(results.allSatisfy { $0.text.highlight == "夜灯" })
}

/// The merged cluster shifts every character of the entry one position
/// back in the window. The entry's start offset must account for it,
/// otherwise a match is attributed to the wrong segment.
@Test func matchResolvesToTheOwningSegment() async throws {
let results = try await search(query: "夜灯", elements: [
["第一段。"],
["\u{0301}abc", "夜灯xyz"],
])

#expect(results.count == 1)
let result = try #require(results.first)
#expect(result.text.highlight == "夜灯")
#expect(result.locations.fragments == ["e1s1"])
}

@Test func snippetsAroundAMergedCluster() async throws {
let results = try await search(query: "abc", elements: [
["第一段。"],
["\u{0301}abc def"],
])

let result = try #require(results.first)
#expect(result.text.highlight == "abc")
#expect(result.text.before == "第一段。 \u{0301}")
#expect(result.text.after == "def")
}

/// A long resource forces the window to be front-trimmed repeatedly,
/// which rebases every entry's offset. Merged clusters must not make
/// the offsets drift across trims.
@Test(arguments: ["\u{0301}", "\u{FE0F}", "\u{200C}"])
func matchesSurviveWindowTrimming(mergingCharacter: String) async throws {
let elementCount = 100
let results = try await search(query: "夜灯", elements: (0 ..< elementCount).map { _ in
["\(mergingCharacter)猫在窗台上打盹,", "\(mergingCharacter)夜灯映出影子。"]
})

#expect(results.count == elementCount)
#expect(results.allSatisfy { $0.text.highlight == "夜灯" })
#expect(results.map(\.locations.fragments) == (0 ..< elementCount).map { ["e\($0)s1"] })
}
}
}

// MARK: - Helpers

/// Runs a search over a publication whose content is made of one
/// `TextContentElement` per array of segment texts.
private func search(query: String, elements segmentTexts: [[String]]) async throws -> [Locator] {
let locator = Locator(href: "chap1", mediaType: .html)
let elements: [ContentElement] = segmentTexts.enumerated().map { elementIndex, texts in
TextContentElement(
locator: locator,
role: .body,
segments: texts.enumerated().map { segmentIndex, text in
TextContentElement.Segment(
locator: locator.copy(locations: { $0.fragments = ["e\(elementIndex)s\(segmentIndex)"] }),
text: text
)
}
)
}

let publication = Publication(
manifest: Manifest(
metadata: Metadata(title: ""),
readingOrder: [Link(href: "chap1", mediaType: .html)]
),
servicesBuilder: PublicationServicesBuilder(
content: { _ in StubContentService(elements: elements) },
search: ContentSearchService.makeFactory()
)
)

let iterator = try await publication.search(query: query).get()

var locators: [Locator] = []
while let batch = try await iterator.next().get() {
locators.append(contentsOf: batch.locators)
}
return locators
}

private final class StubContentService: ContentService {
private let elements: [ContentElement]

init(elements: [ContentElement]) {
self.elements = elements
}

func content(from start: Locator?) -> Content? {
StubContent(elements: elements)
}

private struct StubContent: Content {
let elements: [ContentElement]

func iterator() -> ContentIterator {
StubContentIterator(elements: elements)
}
}

private actor StubContentIterator: ContentIterator {
private let elements: [ContentElement]
private var index = 0

init(elements: [ContentElement]) {
self.elements = elements
}

func next() async throws -> ContentElement? {
guard index < elements.count else {
return nil
}
defer { index += 1 }
return elements[index]
}

func previous() async throws -> ContentElement? {
nil
}
}
}
Loading