161 lines
7.2 KiB
Markdown
161 lines
7.2 KiB
Markdown
---
|
||
tags:
|
||
- ddg-browser
|
||
- bugs
|
||
- performance
|
||
- ios
|
||
- macos
|
||
created: '2026-06-23'
|
||
status: in-progress
|
||
---
|
||
# absoluteString O(n) scan — main-thread stall bugs
|
||
|
||
## Root cause
|
||
|
||
`String.firstIndex(of: "#")` decodes grapheme clusters across every UTF-8 byte, making it O(n) in string length. `URL.absoluteString` materialises the full raw URL — including multi-megabyte `data:` URIs (e.g. content-scope privacy config JSON). Calling any character-scanning API on this on the **main thread** causes visible stalls and, for large enough payloads, a **SIGKILL from the iOS watchdog**.
|
||
|
||
**Measured baseline (Apple Silicon M-series, optimised utf8 byte scan, no guard):**
|
||
|
||
| Payload | Time |
|
||
|---|---|
|
||
| 1 MB, no `#` | ~15 ms |
|
||
| 1 MB, `#` at end | ~15 ms |
|
||
| 15 MB, no `#` | **~232 ms** |
|
||
|
||
On older iPhone A-series (3–5× slower): 15 MB → **~700 ms–1.1 s per call**. Stacked across multiple nav actions per load = watchdog territory.
|
||
|
||
`Foundation.URL.fragment`, by contrast, parses all URL components eagerly at `URL(string:)` init time — accessing `.fragment` is O(1) regardless of payload size (confirmed with 1 MB `data:` URI test, both with and without `#` at the end: < 1 ms).
|
||
|
||
---
|
||
|
||
## Known fixed
|
||
|
||
### `hashedSuffixRange` — `StringExtension.swift`
|
||
|
||
**File:** `SharedPackages/Infrastructure/SystemFrameworksExtensions/Sources/FoundationExtensions/StringExtension.swift`
|
||
|
||
**Fix:** `data:` guard at top of `hashedSuffixRange` short-circuits before any scan. Also replaced `String.firstIndex(of: "#")` with `utf8.firstIndex(of: 0x23)` (byte-level, avoids grapheme decoding).
|
||
|
||
**Note:** `about:` + `%23` special case intentionally preserved — Foundation percent-encodes `#` to `%23` in opaque `about:` URL absoluteStrings in some construction paths (e.g. `URL(trimmedAddressBarString:)`). Introduced in commit `e880d888b3` ("fix about: scheme fragments dropping", Mar 2023). `URL.fragment` returns nil for these, so the string scan branch is genuinely needed.
|
||
|
||
**Asana task:** https://app.asana.com/1/137249556945/project/1202406491309510/task/1215403820548743
|
||
|
||
---
|
||
|
||
## Known bugs — not yet fixed
|
||
|
||
### HIGH — navigation hot path
|
||
|
||
#### `isSameDocument` / `droppingHashedSuffix` — `URLExtension.swift:387`
|
||
|
||
```swift
|
||
// URLExtension.swift:387
|
||
public func isSameDocument(_ other: URL) -> Bool {
|
||
self.absoluteString.droppingHashedSuffix() == other.absoluteString.droppingHashedSuffix()
|
||
}
|
||
```
|
||
|
||
Called from `WKNavigationActionExtension.isSameDocumentNavigation` (main thread, every nav action). Same O(n) scan via `droppingHashedSuffix` → `hashedSuffixRange`. Will be covered once the `data:` guard lands in `hashedSuffixRange`.
|
||
|
||
**Fix suggestion:** The `data:` guard in `hashedSuffixRange` propagates here automatically since `droppingHashedSuffix` delegates to it. No separate fix needed — covered by the `StringExtension` fix.
|
||
|
||
#### `matches(_ other: URL)` — `URLExtension.swift:32–36`
|
||
|
||
```swift
|
||
public func matches(_ other: URL) -> Bool {
|
||
let string1 = self.absoluteString
|
||
let string2 = other.absoluteString
|
||
return string1.droppingHashedSuffix().dropping(suffix: "/").appending(string1.hashedSuffix ?? "")
|
||
== string2.droppingHashedSuffix().dropping(suffix: "/").appending(string2.hashedSuffix ?? "")
|
||
}
|
||
```
|
||
|
||
Used in `FrameInfo.Equatable`, `ContextMenuManager`, `PopupHandlingTabExtension` — all main thread. Also covered by `hashedSuffixRange` `data:` guard.
|
||
|
||
#### `DistributedNavigationDelegate.swift:673`
|
||
|
||
`hashedSuffix` + `droppingHashedSuffix` in `willPerformClientRedirect`. Same chain. Covered by guard.
|
||
|
||
---
|
||
|
||
### HIGH — JS injection with full absoluteString
|
||
|
||
#### `TabViewController.swift:4719`
|
||
```swift
|
||
"window.location.href='" + url.absoluteString + "'"
|
||
```
|
||
Entire URL embedded in a JS string for `evaluateJavaScript`. If `url` is a large `data:` URI, the JS string is megabytes long.
|
||
|
||
**Fix suggestion:** Check scheme before embedding. `data:` URIs should never be navigated to via `window.location.href` injection — bail early if `url.scheme == "data"`.
|
||
|
||
#### `Tab.swift` / `WKWebViewExtension.swift` — `escapedJavaScriptString()`
|
||
|
||
Same pattern — `absoluteString` → `escapedJavaScriptString()` → `evaluateJavaScript(...)`. Used in error-page recovery (`location.replace(...)`) and `window.open`.
|
||
|
||
**Fix suggestion:** Same — guard on scheme before serialising into JS.
|
||
|
||
---
|
||
|
||
### HIGH — blob: credential stripping
|
||
|
||
#### `macOS/DuckDuckGo/Common/Extensions/URLExtension.swift:843–849`
|
||
|
||
```swift
|
||
// strippingUnsupportedCredentials() for blob: URLs
|
||
firstIndex(of: "@") + replacingOccurrences(..., .regularExpression)
|
||
```
|
||
|
||
`blob:` URLs can embed large payloads (base64-encoded media). Called from `TabViewModel.addressBarString` on `@MainActor`.
|
||
|
||
**Fix suggestion:** `blob:` URLs never have `@`-credentials in practice. Add a scheme guard:
|
||
```swift
|
||
guard scheme == "http" || scheme == "https" else { return self }
|
||
```
|
||
|
||
---
|
||
|
||
### MEDIUM — address bar rendering
|
||
|
||
#### `macOS/DuckDuckGo/Tab/ViewModel/TabViewModel.swift:403`
|
||
```swift
|
||
addressBarString = url.absoluteString // non-blob path
|
||
```
|
||
`TabViewModel.passiveAddressBarString` already guards `data:` (returns `"data:"`), but `addressBarString` does not.
|
||
|
||
**Fix suggestion:** Apply same guard — if `url.scheme == "data"`, set `addressBarString = "data:"`.
|
||
|
||
#### `iOS/DuckDuckGo/AddressDisplayHelper.swift:55–74`
|
||
`NSMutableAttributedString(string: absoluteString)` + `NSRange` deemphasis. Omnibar rendering on main thread.
|
||
|
||
**Fix suggestion:** Same — cap or replace `data:` URLs before constructing attributed string.
|
||
|
||
#### `iOS/DuckDuckGo/BackForwardMenuHistoryItem.swift:41–58`
|
||
`URL(trimmedAddressBarString: url.absoluteString)`, `dropping`, `.count`, `dropLast` on main thread for back/forward menu construction.
|
||
|
||
**Fix suggestion:** Guard on `url.scheme != "data"` before passing to `trimmedAddressBarString` initialiser.
|
||
|
||
---
|
||
|
||
### MEDIUM — navigation pipeline regex/prefix checks
|
||
|
||
#### `LinkCleaner.swift:36,61–65`
|
||
`NSRegularExpression.matches(in: url.absoluteString)` — AMP format matching. Guarded to http/https by the call site but still scans full string length.
|
||
|
||
**Fix suggestion:** `AMPCanonicalExtractor` already has a `maxURLLength` guard — ensure `LinkCleaner` applies the same.
|
||
|
||
#### `HistoryViewDataProvider.swift:478`
|
||
`absoluteString.localizedCaseInsensitiveContains(searchTerm)` — history search. Runs on main thread. History URLs are rarely `data:` but theoretically possible.
|
||
|
||
#### `AIChatMentionPickerFilter.swift:57`
|
||
`tab.url.absoluteString.lowercased().contains(query)` — AI Chat tab picker filter, main thread.
|
||
|
||
**Fix suggestion:** Both — early-return `false` if `url.scheme == "data"` before scanning.
|
||
|
||
---
|
||
|
||
## Test coverage added (branch `fix-fragment-crash`)
|
||
|
||
- `HashedSuffixDataURIPerformanceTests` in `StringExtensionTests.swift` — asserts `hashedSuffix` completes in < 1 ms on 1 MB and 15 MB `data:` URIs (with and without `#` at end). Fails without the `data:` guard (15 MB → ~232 ms).
|
||
- `URLFragmentDataURIPerformanceTests` in `WKNavigationActionExtensionTests.swift` — asserts `URL.fragment` completes in < 10 ms on 1 MB `data:` URI. Confirms Foundation parses eagerly (O(1) access).
|
||
- `WKNavigationActionExtensionTests` — full unit test suite for `isSameDocumentNavigation` covering all nav types, `data:` URLs, `about:%23` edge case.
|