diff --git a/commons/plans/2026-09-12-audit-findings.md b/commons/plans/2026-09-12-audit-findings.md new file mode 100644 index 0000000000..1deb8c39be --- /dev/null +++ b/commons/plans/2026-09-12-audit-findings.md @@ -0,0 +1,75 @@ +# Audit: bugs and performance in `commons` / `commonsUI` (2026-09-12) + +**Method.** Three independent read-only sweeps (commons hot paths: feeds, +relayClient, model, viewmodels; commons services; commonsUI composables) +produced 44 candidate findings. Every one was re-read in the source before +acting; 38 held up, 6 were rejected (below). 34 are fixed on this branch, +4 are deferred because they need a design decision. + +## Fixed — correctness + +| Where | Defect | +|---|---| +| `keystorage/SecureKeyStorage.kt` (jvm) | AES-GCM initialised with `IvParameterSpec` → `InvalidAlgorithmParameterException`; the whole no-keyring fallback (headless Linux, containers) was dead. Now `GCMParameterSpec(128, iv)`; round-trip test added. | +| `service/http/OnionLocationInterceptor.kt` | Any `Onion-Location` header was cached and every Tor request re-pointed at it for 24 h, https→http allowed. Only `.onion` hosts are cached now. | +| `service/http/EncryptedBlobInterceptor.kt` | `peekBody(MAX)` + `bytes()` buffered the blob twice and never closed the original body (a network interceptor: the pool slot stayed open). Reads once, closes. | +| `service/BundledUpdate.kt` `BasicBundledInsert` | No `finally`: one throw out of `onUpdate` left `isProcessing = true` and every later `invalidateList` enqueued forever. Mirrors `BasicBundledUpdate`. | +| `richtext/CachedRichTextParser.kt` | Parse cache keyed on a 32-bit hash with no input check; a `String.hashCode` collision served one note's segments under another author. Hits now verify content/tags/uri/author. | +| `richtext/RichTextParser.kt` `noProtocolUrlValidator` | `(sep?[word]+)*` backtracks exponentially on a line terminator (13 s at 26 chars, run per keystroke in the composer). Separator made mandatory; timing test added. | +| `blurhash/Base83.kt` | `charMap[c.code]` on a 255-entry table → `ArrayIndexOutOfBounds` for any non-Latin-1 char in an attacker-controlled `imeta` blurhash. Bounds-checked. | +| `blurhash/CosineCache.kt` | Keyed on `size * components` so (50,4) and (100,2) shared a table; has()/get() pair could NPE under a concurrent eviction. Composite key + single lookup. | +| `service/upload/MediaCompressor.kt` | EXIF strip gated on the file *name* and failures swallowed silently; camera JPEGs could upload with GPS intact. Sniffs bytes, logs the throwable. | +| `service/upload/StaticSitePublisher.kt` | `walkTopDown()` followed symlinks out of the published root. Links are skipped and files must resolve under the root. | +| `preview/UrlPreview.kt` | `body.bytes()` on an attacker-controlled URL; a host streaming `text/html` forever OOMs the app. Reads at most 512 KB. | +| `service/namecoin/NamecoinNameService.kt` | `catch (e: Exception)` swallowed `CancellationException` and dropped the stack trace. | +| `relayClient/preload/MetadataRateLimiter.kt` | The "flush remaining" ran only after the channel closed, which never happens: fewer than 20 queued pubkeys were never requested (desktop DM list names stayed `npub1…`). Also an unguarded `HashSet` shared across threads. Timed flush + lock; tests added. | +| `relayClient/assemblers/FeedMetadataCoordinator.kt` | Six plain `HashSet`s mutated from Compose callbacks and IO coroutines; and `loadMetadataBatched` marked the whole list as asked while requesting only `take(100)`. One lock, chunked filters. | +| `model/Note.kt` | `isHiddenFor` computed and discarded the scoped-comment muted-word check; `relayHintUrl` computed and discarded live-activity relays; `flow()` check-then-`!!` could NPE against `clearFlow()`; `removeReport` left an empty bucket so deleted reports kept counting; reply/boost/edit/timestamp/reaction/report/label mutators did unlocked read-modify-write while zaps used `syncLock`. | +| `state/EventCollectionState.kt` | Cancel-and-restart on every insert (the KDoc promised the opposite): a feed busier than one event per 250 ms never flushed. Leading-edge throttle. | +| `model/privateChats/Chatroom.kt` | `pruneMessagesToTheLatestOnly` rewrote `messages` outside the lock its add/remove use; a just-decrypted DM could vanish. | +| `viewmodels/LiveStreamTopZappersViewModel.kt` | `publish()` iterated two maps outside the mutex their two writer coroutines hold. | +| `relayClient/assemblers/{Metadata,Reactions}FilterAssembler.kt`, `feeds/ChatroomFeedFilter.kt` | `hashCode()` used as the dedup / feed key; a collision dropped a whole pubkey set from the REQ or merged two rooms' feeds. Keys are the values themselves. | +| `model/ThreadAssembler.kt` `OnlyLatestVersionSet` | `addAll`/`removeAll` returned `elements.map{…}.any()` — always true for non-empty input. | +| `ui/components/ClickableTexts.kt` | `remember(text)` baked `onClick` into the link annotation; a recycled row fired the previous target. | +| `ui/components/ZonedSwipeModifier.kt` | `remember {}` captured the first `openDrawer` forever. | +| `ui/components/RobohashImage.kt` vs `UserAvatar.kt` | Two different "is light theme" predicates fed one global cache that evicts everything when the flag flips. Unified. | +| `nip64Chess/ui/InteractiveChessBoard.kt` | `remember(localMoveCount + positionVersion)` — a sum is not a composite key. | + +## Fixed — performance + +| Where | Change | +|---|---| +| `viewmodels/SearchBarState.kt`, `search/UserSearchEngine.kt` | Full user-cache scan ran on the Compose dispatcher per debounced keystroke; `flowOn(Dispatchers.Default)`. | +| `search/SearchResultSorter.kt` | 1+N `Regex` compiled per scored event; hoisted and cached. | +| `ui/components/ShimmerPlaceholder.kt`, `LoadingAnimation.kt`, `BunkerHeartbeatIndicator.kt` | Frame-rate animation values read in composition (whole composable recomposed every frame, brush/list re-allocated); moved into draw/layer lambdas. | +| `ui/components/GlowingCard.kt` | Brush, dash array and stroke allocated on every draw; `drawWithCache`. | +| `ui/richtext/CustomEmojiRenderer.kt` | `inlineContent` map rebuilt per recomposition on every emoji-bearing name/note; remembered. | +| `ui/search/SearchPeoplePicker.kt` | `indexOf` inside `items` (O(n²) per arrow key, wrong for duplicates); `itemsIndexed`. | +| `nip64Chess/ui/LiveChessGame.kt` | `moves.chunked(2)` per recomposition; remembered. | + +## Deferred (need a design call) + +- **`feeds/FeedContentState.kt`** — the full-refresh bundler and the additive-insert + bundler both end in `updateFeed`, a read-modify-write on `_feedContent` with + separate mutexes. A slow `loadTop()` can overwrite an additive emission that + landed in between. Fix is a single mutex/bundler or a CAS `update {}`; the + choice affects feed latency, so it is left to the maintainer. +- **`model/Note.kt` child lists** — `replies`/`boosts`/`edits` are `List`s, so + every insert is an O(n) `contains` plus a full copy (O(n²) per thread load). + `Chatroom.messages` already uses a `Set`; switching these changes a public type. +- **`ui/note/GitDiffView.kt`** — every line of every file in a patch is composed + eagerly in a non-lazy `Column` (expanded by default). Needs a `LazyColumn` + restructure or a collapse-past-N-lines default. +- **`ui/components/UserAvatar.kt`** — the robohash fallback `ImageVector` is + assembled synchronously in composition even when Coil already has the image; + past 100 distinct authors every one is a cache miss on the UI thread. Needs + the fallback to be produced lazily/off-thread (`produceCachedStateAsync`). + +## Rejected after verification + +- `NewPostsChipState`: "chip can never appear" — `val isAtTop by derivedStateOf` + routes reads through the State, so the outer `derivedStateOf` does track it. +- `EditNicknameDialog`: the initial `snapshotFlow` emission mis-targets the + emoji field, but typing retargets before any suggestion can exist. +- Three "internal visibility" and "same-package reference" hits were + false positives of the name scan. diff --git a/commons/plans/README.md b/commons/plans/README.md index 3e35c3732c..01bc81b0cf 100644 --- a/commons/plans/README.md +++ b/commons/plans/README.md @@ -18,6 +18,7 @@ _Audited 2026-06-30 (+ 2026-09-12 split entry). 7 plans: 3 shipped, 2 in-progres ## Shipped | Plan | Summary | | ---- | ------- | +| [2026-09-12-audit-findings.md](2026-09-12-audit-findings.md) | Bug/performance audit of `commons` + `commonsUI` after the split: 34 verified fixes shipped, 4 deferred with rationale, 6 rejected. | | [2026-09-12-commons-ui-split.md](2026-09-12-commons-ui-split.md) | Split the Compose half of `commons` into the new `:commonsUI` module (same packages, `api(:commons)`), so `cli` no longer carries Compose/Skiko; records the classification method and follow-ups. | ## Archived (shipped) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/Base83.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/Base83.kt index ec75e7f01f..081810762b 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/Base83.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/Base83.kt @@ -57,12 +57,12 @@ object Base83 { fun decodeAt( str: String, at: Int = 0, - ): Int = charMap[str[at].code] + ): Int = valueOf(str[at]) fun decodeFixed2( str: String, from: Int = 0, - ): Int = charMap[str[from].code] * 83 + charMap[str[from + 1].code] + ): Int = valueOf(str[from]) * 83 + valueOf(str[from + 1]) fun decode( str: String, @@ -71,13 +71,24 @@ object Base83 { ): Int { var result = 0 for (i in from until to) { - result = result * 83 + charMap[str[i].code] + result = result * 83 + valueOf(str[i]) } return result } val ALPHABET: CharArray = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz#$%*+,-.:;=?@[]^_{|}~".toCharArray() + /** + * Value of [c] in the base-83 alphabet, or 0 for any character outside it. + * Blurhashes come from attacker-controlled `imeta` tags, so a non-Latin-1 + * character must degrade to a wrong colour, not an + * ArrayIndexOutOfBoundsException out of the image fetcher. + */ + private fun valueOf(c: Char): Int { + val code = c.code + return if (code < charMap.size) charMap[code] else 0 + } + private val charMap = ALPHABET .mapIndexed { i, c -> c.code to i } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/BlurHashDecoder.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/BlurHashDecoder.kt index 52a299a1ac..46abcd162f 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/BlurHashDecoder.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/BlurHashDecoder.kt @@ -122,10 +122,8 @@ object BlurHashDecoder { ): IntArray { // use an array for better performance when writing pixel colors val imageArray = IntArray(width * height) - val calculateCosX = !useCache || !CosineCache.hasX(width * numCompX) - val cosinesX = CosineCache.getArrayForCosinesX(calculateCosX, width, numCompX) - val calculateCosY = !useCache || !CosineCache.hasY(height * numCompY) - val cosinesY = CosineCache.getArrayForCosinesY(calculateCosY, height, numCompY) + val cosinesX = CosineCache.getArrayForCosinesX(useCache, width, numCompX) + val cosinesY = CosineCache.getArrayForCosinesY(useCache, height, numCompY) var r = 0.0f var g = 0.0f diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/BlurHashEncoder.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/BlurHashEncoder.kt index e1c299749a..57d8e97076 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/BlurHashEncoder.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/BlurHashEncoder.kt @@ -83,10 +83,8 @@ class BlurHashEncoder { val factors = Array(componentX * componentY) { DoubleArray(3) } - val calculateCosX = !useCache || !CosineCache.hasX(width * componentX) - val cosinesX = CosineCache.getArrayForCosinesX(calculateCosX, width, componentX) - val calculateCosY = !useCache || !CosineCache.hasY(height * componentY) - val cosinesY = CosineCache.getArrayForCosinesY(calculateCosY, height, componentY) + val cosinesX = CosineCache.getArrayForCosinesX(useCache, width, componentX) + val cosinesY = CosineCache.getArrayForCosinesY(useCache, height, componentY) val scale = 1.0 / (width * height) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/CosineCache.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/CosineCache.kt index 5510c69ef0..e61e0aa2a2 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/CosineCache.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/blurhash/CosineCache.kt @@ -30,8 +30,15 @@ object CosineCache { // 2 * nBitmaps // the cache is enabled by default, it is recommended to disable it only when just a few images // are displayed - private val cacheCosinesX = LruCache(20) - private val cacheCosinesY = LruCache(20) + // Keyed on (size, components) as a pair, not their product: (50, 4) and + // (100, 2) both produce a 200-entry table with different contents. + private val cacheCosinesX = LruCache(20) + private val cacheCosinesY = LruCache(20) + + private fun key( + size: Int, + components: Int, + ): Long = (size.toLong() shl 32) or components.toLong() /** * Clear calculations stored in memory cache. The cache is not big, but will increase when many @@ -42,45 +49,46 @@ object CosineCache { cacheCosinesY.evictAll() } - fun hasX(idx: Int) = cacheCosinesX.get(idx) != null - - fun hasY(idx: Int) = cacheCosinesY.get(idx) != null - - fun getArrayForCosinesY( - calculate: Boolean, + private fun computeY( height: Int, numCompY: Int, - ) = when { - calculate -> { - DoubleArray(height * numCompY) { - val y = it / numCompY - val j = it % numCompY - cos(PI * y * j / height) - }.also { - cacheCosinesY.put(height * numCompY, it) - } - } + ) = DoubleArray(height * numCompY) { + val y = it / numCompY + val j = it % numCompY + cos(PI * y * j / height) + } - else -> { - cacheCosinesY[height * numCompY]!! - } + private fun computeX( + width: Int, + numCompX: Int, + ) = DoubleArray(width * numCompX) { + val x = it / numCompX + val i = it % numCompX + cos(PI * x * i / width) + } + + /** + * The Y cosine table for ([height], [numCompY]). With [useCache] the lookup + * and the insert are one operation, so a concurrent decoder evicting the + * entry between a has()/get() pair can no longer produce a null. + */ + fun getArrayForCosinesY( + useCache: Boolean, + height: Int, + numCompY: Int, + ): DoubleArray { + if (!useCache) return computeY(height, numCompY) + val k = key(height, numCompY) + return cacheCosinesY[k] ?: computeY(height, numCompY).also { cacheCosinesY.put(k, it) } } fun getArrayForCosinesX( - calculate: Boolean, + useCache: Boolean, width: Int, numCompX: Int, - ) = when { - calculate -> { - DoubleArray(width * numCompX) { - val x = it / numCompX - val i = it % numCompX - cos(PI * x * i / width) - }.also { cacheCosinesX.put(width * numCompX, it) } - } - - else -> { - cacheCosinesX[width * numCompX]!! - } + ): DoubleArray { + if (!useCache) return computeX(width, numCompX) + val k = key(width, numCompX) + return cacheCosinesX[k] ?: computeX(width, numCompX).also { cacheCosinesX.put(k, it) } } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/feeds/ChatroomFeedFilter.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/feeds/ChatroomFeedFilter.kt index ca40cdffe3..75865e839e 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/feeds/ChatroomFeedFilter.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/feeds/ChatroomFeedFilter.kt @@ -34,7 +34,7 @@ class ChatroomFeedFilter( override fun changesFlow() = chatroom().changesFlow() // returns the last Note of each user. - override fun feedKey(): String = withUser.hashCode().toString() + override fun feedKey(): String = withUser.users.sorted().joinToString(",") override fun feed(): List = chatroom().messages.filter { account.isAcceptable(it) }.sortedWith(DefaultFeedOrder) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index be6da5ffa0..c7e11dd6d3 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -194,12 +194,15 @@ open class Note( var otsVerification: VerificationState? = null // These fields are updated every time an event related to this note is received. + @Volatile var replies = listOf() private set + @Volatile var reactions = mapOf>() private set + @Volatile var boosts = listOf() private set @@ -211,6 +214,7 @@ open class Note( * (channel-retained) message keeps it strongly reachable. The chat bubble overlays the latest * author-matching edit. */ + @Volatile var edits = listOf() private set @@ -221,9 +225,11 @@ open class Note( * delete). Anchoring them here also replaces the old full-cache scan: the OTS pill folds this * list directly. Each attestation memoizes its own blockchain verdict in [Note.otsVerification]. */ + @Volatile var timestamps = listOf() private set + @Volatile var reports = mapOf>() private set @@ -234,6 +240,7 @@ open class Note( * so `source.author` identifies who labeled it. Used by the hashtag feed to * surface posts a follow has tagged and to attribute the label in the UI. */ + @Volatile var labels = mapOf>() private set @@ -358,7 +365,7 @@ open class Note( } is LiveActivitiesEvent -> { - noteEvent.relays().ifEmpty { null }?.toSet() + noteEvent.relays().firstOrNull()?.let { return it } } is LiveActivitiesChatMessageEvent -> { @@ -449,51 +456,65 @@ open class Note( } fun addReply(note: Note) { - if (note !in replies) { - replies = replies + note - flowSet?.replies?.invalidateData() + syncLock.withLock { + if (note !in replies) { + replies = replies + note + flowSet?.replies?.invalidateData() + } } } fun removeReply(note: Note) { - if (note in replies) { - replies = replies - note - flowSet?.replies?.invalidateData() + syncLock.withLock { + if (note in replies) { + replies = replies - note + flowSet?.replies?.invalidateData() + } } } fun addEdit(note: Note) { - if (note !in edits) { - edits = edits + note - flowSet?.edits?.invalidateData() + syncLock.withLock { + if (note !in edits) { + edits = edits + note + flowSet?.edits?.invalidateData() + } } } fun removeEdit(note: Note) { - if (note in edits) { - edits = edits - note - flowSet?.edits?.invalidateData() + syncLock.withLock { + if (note in edits) { + edits = edits - note + flowSet?.edits?.invalidateData() + } } } fun addTimestamp(note: Note) { - if (note !in timestamps) { - timestamps = timestamps + note - flowSet?.ots?.invalidateData() + syncLock.withLock { + if (note !in timestamps) { + timestamps = timestamps + note + flowSet?.ots?.invalidateData() + } } } fun removeTimestamp(note: Note) { - if (note in timestamps) { - timestamps = timestamps - note - flowSet?.ots?.invalidateData() + syncLock.withLock { + if (note in timestamps) { + timestamps = timestamps - note + flowSet?.ots?.invalidateData() + } } } fun removeBoost(note: Note) { - if (note in boosts) { - boosts = boosts - note - flowSet?.boosts?.invalidateData() + syncLock.withLock { + if (note in boosts) { + boosts = boosts - note + flowSet?.boosts?.invalidateData() + } } } @@ -577,32 +598,44 @@ open class Note( } fun removeReaction(note: Note) { - val tags = note.event?.tags ?: emptyArray() - val reaction = note.event?.content?.firstFullCharOrEmoji(ImmutableListOfLists(tags)) ?: "+" + syncLock.withLock { + val tags = note.event?.tags ?: emptyArray() + val reaction = note.event?.content?.firstFullCharOrEmoji(ImmutableListOfLists(tags)) ?: "+" - if (reactions[reaction]?.contains(note) == true) { - reactions[reaction]?.let { - if (note in it) { - val newList = it.minus(note) - if (newList.isEmpty()) { - reactions = reactions.minus(reaction) - } else { - reactions = reactions + Pair(reaction, newList) + if (reactions[reaction]?.contains(note) == true) { + reactions[reaction]?.let { + if (note in it) { + val newList = it.minus(note) + if (newList.isEmpty()) { + reactions = reactions.minus(reaction) + } else { + reactions = reactions + Pair(reaction, newList) + } + + flowSet?.reactions?.invalidateData() } - - flowSet?.reactions?.invalidateData() } } } } fun removeReport(deleteNote: Note) { - val author = deleteNote.author ?: return + syncLock.withLock { + val author = deleteNote.author ?: return - if (reports[author]?.contains(deleteNote) == true) { - reports[author]?.let { - reports = reports + Pair(author, it.minus(deleteNote)) - flowSet?.reports?.invalidateData() + if (reports[author]?.contains(deleteNote) == true) { + reports[author]?.let { + val newList = it.minus(deleteNote) + // Drop the author key when their last report goes, otherwise + // countReportAuthorsBy() keeps counting a deleted report. + reports = + if (newList.isEmpty()) { + reports.minus(author) + } else { + reports + Pair(author, newList) + } + flowSet?.reports?.invalidateData() + } } } } @@ -630,9 +663,11 @@ open class Note( } fun addBoost(note: Note) { - if (note !in boosts) { - boosts = boosts + note - flowSet?.boosts?.invalidateData() + syncLock.withLock { + if (note !in boosts) { + boosts = boosts + note + flowSet?.boosts?.invalidateData() + } } } @@ -908,30 +943,34 @@ open class Note( } fun addReaction(note: Note) { - val tags = note.event?.tags ?: emptyArray() - val reaction = note.event?.content?.firstFullCharOrEmoji(ImmutableListOfLists(tags)) ?: "+" + syncLock.withLock { + val tags = note.event?.tags ?: emptyArray() + val reaction = note.event?.content?.firstFullCharOrEmoji(ImmutableListOfLists(tags)) ?: "+" - val listOfAuthors = reactions[reaction] - if (listOfAuthors == null) { - reactions = reactions + Pair(reaction, listOf(note)) - flowSet?.reactions?.invalidateData() - } else if (!listOfAuthors.contains(note)) { - reactions = reactions + Pair(reaction, listOfAuthors + note) - flowSet?.reactions?.invalidateData() + val listOfAuthors = reactions[reaction] + if (listOfAuthors == null) { + reactions = reactions + Pair(reaction, listOf(note)) + flowSet?.reactions?.invalidateData() + } else if (!listOfAuthors.contains(note)) { + reactions = reactions + Pair(reaction, listOfAuthors + note) + flowSet?.reactions?.invalidateData() + } } } fun addReport(note: Note) { - val author = note.author ?: return + syncLock.withLock { + val author = note.author ?: return - val reportsByAuthor = reports[author] + val reportsByAuthor = reports[author] - if (reportsByAuthor == null) { - reports = reports + Pair(author, listOf(note)) - flowSet?.reports?.invalidateData() - } else if (!reportsByAuthor.contains(note)) { - reports = reports + Pair(author, reportsByAuthor + note) - flowSet?.reports?.invalidateData() + if (reportsByAuthor == null) { + reports = reports + Pair(author, listOf(note)) + flowSet?.reports?.invalidateData() + } else if (!reportsByAuthor.contains(note)) { + reports = reports + Pair(author, reportsByAuthor + note) + flowSet?.reports?.invalidateData() + } } } @@ -940,25 +979,29 @@ open class Note( hashtag: String, note: Note, ) { - val listOfLabelers = labels[hashtag] - if (listOfLabelers == null) { - labels = labels + Pair(hashtag, listOf(note)) - flowSet?.labels?.invalidateData() - } else if (!listOfLabelers.contains(note)) { - labels = labels + Pair(hashtag, listOfLabelers + note) - flowSet?.labels?.invalidateData() + syncLock.withLock { + val listOfLabelers = labels[hashtag] + if (listOfLabelers == null) { + labels = labels + Pair(hashtag, listOf(note)) + flowSet?.labels?.invalidateData() + } else if (!listOfLabelers.contains(note)) { + labels = labels + Pair(hashtag, listOfLabelers + note) + flowSet?.labels?.invalidateData() + } } } /** Detach a LabelEvent note (e.g. deleted) from every hashtag bucket it was in. */ fun removeLabel(note: Note) { - if (labels.none { it.value.contains(note) }) return + syncLock.withLock { + if (labels.none { it.value.contains(note) }) return - labels = - labels - .mapValues { it.value - note } - .filterValues { it.isNotEmpty() } - flowSet?.labels?.invalidateData() + labels = + labels + .mapValues { it.value - note } + .filterValues { it.isNotEmpty() } + flowSet?.labels?.invalidateData() + } } fun addRelaySync(relay: NormalizedRelayUrl) = @@ -1518,8 +1561,8 @@ open class Note( return true } - if (thisEvent is CommentEvent) { - thisEvent.isScoped { it.containsAny(accountChoices.hiddenWordsCase) } + if (thisEvent is CommentEvent && thisEvent.isScoped { it.containsAny(accountChoices.hiddenWordsCase) }) { + return true } if (thisEvent.anyHashTag { it.containsAny(accountChoices.hiddenWordsCase) }) { @@ -1550,12 +1593,13 @@ open class Note( } } - fun flow(): NoteFlowSet { - if (flowSet == null) { - createOrDestroyFlowSync(true) + fun flow(): NoteFlowSet = + // Fast path reads the @Volatile field once; the slow path re-checks under + // the same lock createOrDestroyFlowSync() uses, so a concurrent clearFlow() + // between the two can't leave us dereferencing a just-nulled set. + flowSet ?: syncLock.withLock { + flowSet ?: NoteFlowSet(this).also { flowSet = it } } - return flowSet!! - } fun clearFlow() { if (flowSet != null && flowSet?.isInUse() == false) { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/ThreadAssembler.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/ThreadAssembler.kt index 51bff82663..e07c1c98f9 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/ThreadAssembler.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/ThreadAssembler.kt @@ -223,7 +223,7 @@ class OnlyLatestVersionSet : MutableSet { } } - override fun addAll(elements: Collection): Boolean = elements.map { add(it) }.any() + override fun addAll(elements: Collection): Boolean = elements.fold(false) { changed, it -> add(it) || changed } override val size: Int get() = set.size @@ -243,7 +243,7 @@ class OnlyLatestVersionSet : MutableSet { override fun retainAll(elements: Collection): Boolean = set.retainAll(elements) - override fun removeAll(elements: Collection): Boolean = elements.map { remove(it) }.any() + override fun removeAll(elements: Collection): Boolean = elements.fold(false) { changed, it -> remove(it) || changed } override fun remove(element: Note): Boolean { element.address()?.let { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/privateChats/Chatroom.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/privateChats/Chatroom.kt index 0101161787..b197e44971 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/privateChats/Chatroom.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/privateChats/Chatroom.kt @@ -135,7 +135,13 @@ class Chatroom : NotesGatherer { fun senderIntersects(keySet: Set): Boolean = activeSenders.any { it.pubkeyHex in keySet } - fun pruneMessagesToTheLatestOnly(): Set { + fun pruneMessagesToTheLatestOnly(): Set = + // Same lock as addMessageSync/removeMessageSync: the snapshot, the + // rewrite of `messages` and the change emission must not interleave + // with a gift wrap being decrypted into this room. + syncLock.withLock { pruneMessagesToTheLatestOnlyLocked() } + + private fun pruneMessagesToTheLatestOnlyLocked(): Set { val sorted = messages.sortedWith(DefaultFeedOrder) val toKeep = diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/FeedMetadataCoordinator.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/FeedMetadataCoordinator.kt index 2260b279a5..8f97e89244 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/FeedMetadataCoordinator.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/FeedMetadataCoordinator.kt @@ -24,6 +24,8 @@ import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.relayClient.preload.MetadataPreloader import com.vitorpamplona.amethyst.commons.relayClient.subscriptions.PrioritizedSubscriptionQueue import com.vitorpamplona.amethyst.commons.relayClient.subscriptions.SubscriptionPriority +import com.vitorpamplona.amethyst.commons.util.KmpLock +import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent @@ -86,6 +88,26 @@ class FeedMetadataCoordinator( private val inFlightBatchedMetadata = mutableSetOf() private val inFlightBatchedKind3 = mutableSetOf() + // The six sets above are touched from the caller's thread (Compose scroll + // callbacks) and from the batched REQ coroutines on IO; a plain HashSet + // corrupts under that interleaving. Every read-filter-then-add goes + // through [claim] or an explicit lock. + private val queueLock = KmpLock() + + /** + * Atomically returns the members of [candidates] not yet in [set] and marks + * them, so two callers racing on the same pubkeys cannot both claim them. + */ + private fun claim( + set: MutableSet, + candidates: Collection, + ): List = + queueLock.withLock { + val fresh = candidates.filter { it !in set }.distinct() + set.addAll(fresh) + fresh + } + /** * Start processing the subscription queue. * Call once when coordinator is created. @@ -142,13 +164,9 @@ class FeedMetadataCoordinator( .mapNotNull { it.event } .flatMap { event -> event.tags.mapNotNull { ETag.parseId(it) } } - val allReferencedIds = - (repostBoostedIds + quotedNoteIds) - .filter { it !in queuedBoostedIds } - .distinct() + val allReferencedIds = claim(queuedBoostedIds, repostBoostedIds + quotedNoteIds) if (allReferencedIds.isNotEmpty()) { - queuedBoostedIds.addAll(allReferencedIds) val referencedFilter = Filter( ids = allReferencedIds, @@ -161,23 +179,13 @@ class FeedMetadataCoordinator( } // Extract unique authors that we haven't already queued - val authors = - notes - .mapNotNull { it.author?.pubkeyHex } - .filter { it !in queuedPubkeys } - .distinct() + val authors = claim(queuedPubkeys, notes.mapNotNull { it.author?.pubkeyHex }) // Extract unique note IDs that we haven't already queued - val noteIds = - notes - .map { it.idHex } - .filter { it !in queuedNoteIds } - .distinct() + val noteIds = claim(queuedNoteIds, notes.map { it.idHex }) // Queue metadata first (highest priority) if (authors.isNotEmpty()) { - queuedPubkeys.addAll(authors) - // Use preloader if available for rate-limited loading if (preloader != null) { notes.mapNotNull { it.author }.forEach { user -> @@ -201,8 +209,6 @@ class FeedMetadataCoordinator( // Queue reactions second (lower priority) if (noteIds.isNotEmpty()) { - queuedNoteIds.addAll(noteIds) - val reactionsFilter = Filter( kinds = listOf(ReactionEvent.KIND), @@ -221,11 +227,9 @@ class FeedMetadataCoordinator( * Useful for loading follower/following metadata. */ fun loadMetadataForPubkeys(pubkeys: List) { - val newPubkeys = pubkeys.filter { it !in queuedPubkeys } + val newPubkeys = claim(queuedPubkeys, pubkeys) if (newPubkeys.isEmpty()) return - queuedPubkeys.addAll(newPubkeys) - val filter = Filter( kinds = listOf(MetadataEvent.KIND), @@ -243,11 +247,9 @@ class FeedMetadataCoordinator( * Load reactions for specific note IDs. */ fun loadReactionsForNotes(noteIds: List) { - val newNoteIds = noteIds.filter { it !in queuedNoteIds } + val newNoteIds = claim(queuedNoteIds, noteIds) if (newNoteIds.isEmpty()) return - queuedNoteIds.addAll(newNoteIds) - val filter = Filter( kinds = listOf(ReactionEvent.KIND), @@ -274,22 +276,29 @@ class FeedMetadataCoordinator( timeoutMs: Long = 5_000L, ) { val newPubkeys = - pubkeys - .asSequence() - .filter { it !in queuedPubkeys && it !in inFlightBatchedMetadata } - .distinct() - .toList() + queueLock.withLock { + pubkeys + .asSequence() + .filter { it !in queuedPubkeys && it !in inFlightBatchedMetadata } + .distinct() + .toList() + .also { inFlightBatchedMetadata.addAll(it) } + } if (newPubkeys.isEmpty()) return - inFlightBatchedMetadata.addAll(newPubkeys) scope.launch { - val filter = - Filter( - kinds = listOf(MetadataEvent.KIND), - authors = newPubkeys.take(100), - limit = newPubkeys.size, - ) - val filterMap = indexRelays.associateWith { listOf(filter) } + // One filter per 100 authors, like loadKind3Batched: a single + // `take(100)` filter used to mark the whole list as asked-for while + // only ever requesting the first hundred. + val filters = + newPubkeys.chunked(100).map { chunk -> + Filter( + kinds = listOf(MetadataEvent.KIND), + authors = chunk, + limit = chunk.size, + ) + } + val filterMap = indexRelays.associateWith { filters } val subId = newSubId() val gate = BatchEoseGate(scope, target = indexRelays.size) @@ -316,10 +325,12 @@ class FeedMetadataCoordinator( val eosedRelays = gate.awaitAll(timeoutMs) client.unsubscribe(subId) - if (eosedRelays > 0) { - queuedPubkeys.addAll(newPubkeys) + queueLock.withLock { + if (eosedRelays > 0) { + queuedPubkeys.addAll(newPubkeys) + } + inFlightBatchedMetadata.removeAll(newPubkeys.toSet()) } - inFlightBatchedMetadata.removeAll(newPubkeys.toSet()) } } @@ -345,16 +356,18 @@ class FeedMetadataCoordinator( onEose: () -> Unit = {}, ) { val newPubkeys = - pubkeys - .asSequence() - .filter { it !in queuedKind3Pubkeys && it !in inFlightBatchedKind3 } - .distinct() - .toList() + queueLock.withLock { + pubkeys + .asSequence() + .filter { it !in queuedKind3Pubkeys && it !in inFlightBatchedKind3 } + .distinct() + .toList() + .also { inFlightBatchedKind3.addAll(it) } + } if (newPubkeys.isEmpty()) { onEose() return } - inFlightBatchedKind3.addAll(newPubkeys) scope.launch { val filters = @@ -392,10 +405,12 @@ class FeedMetadataCoordinator( val eosedRelays = gate.awaitAll(timeoutMs) client.unsubscribe(subId) - if (eosedRelays > 0) { - queuedKind3Pubkeys.addAll(newPubkeys) + queueLock.withLock { + if (eosedRelays > 0) { + queuedKind3Pubkeys.addAll(newPubkeys) + } + inFlightBatchedKind3.removeAll(newPubkeys.toSet()) } - inFlightBatchedKind3.removeAll(newPubkeys.toSet()) onEose() } @@ -406,11 +421,14 @@ class FeedMetadataCoordinator( */ fun clear() { priorityQueue.clear() - queuedPubkeys.clear() - queuedNoteIds.clear() - queuedKind3Pubkeys.clear() - inFlightBatchedMetadata.clear() - inFlightBatchedKind3.clear() + queueLock.withLock { + queuedPubkeys.clear() + queuedNoteIds.clear() + queuedBoostedIds.clear() + queuedKind3Pubkeys.clear() + inFlightBatchedMetadata.clear() + inFlightBatchedKind3.clear() + } } /** diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/MetadataFilterAssembler.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/MetadataFilterAssembler.kt index 12bb81893d..9f8b1a9d1b 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/MetadataFilterAssembler.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/MetadataFilterAssembler.kt @@ -54,7 +54,7 @@ class MetadataFilterAssembler( client: INostrClient, allKeys: () -> Set, ) : SingleSubEoseManager(client, allKeys, invalidateAfterEose = true) { - override fun distinct(key: MetadataQueryState): Any = key.pubkeys.hashCode() + override fun distinct(key: MetadataQueryState): Any = key.pubkeys override fun updateFilter( keys: List, diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/ReactionsFilterAssembler.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/ReactionsFilterAssembler.kt index d897f238e8..66ee002cfc 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/ReactionsFilterAssembler.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/assemblers/ReactionsFilterAssembler.kt @@ -52,7 +52,7 @@ class ReactionsFilterAssembler( client: INostrClient, allKeys: () -> Set, ) : SingleSubEoseManager(client, allKeys, invalidateAfterEose = true) { - override fun distinct(key: ReactionsQueryState): Any = key.noteIds.hashCode() + override fun distinct(key: ReactionsQueryState): Any = key.noteIds override fun updateFilter( keys: List, diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/preload/MetadataRateLimiter.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/preload/MetadataRateLimiter.kt index a7469a2cb4..7e8274121c 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/preload/MetadataRateLimiter.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/preload/MetadataRateLimiter.kt @@ -20,11 +20,13 @@ */ package com.vitorpamplona.amethyst.commons.relayClient.preload +import com.vitorpamplona.amethyst.commons.util.KmpLock +import com.vitorpamplona.amethyst.commons.util.withLock import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.consumeAsFlow import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeoutOrNull /** * Global rate limiter for metadata requests to prevent thundering herd on fast scroll. @@ -36,16 +38,25 @@ import kotlinx.coroutines.launch class MetadataRateLimiter( private val maxRequestsPerSecond: Int = 20, private val scope: CoroutineScope, + /** How long a partial batch may wait for more pubkeys before it is flushed. */ + private val flushDelayMs: Long = 250, ) { private val queue = Channel(Channel.BUFFERED) + + // Written from enqueue() (feed/scroll path, any thread) and from the + // collector coroutine — a plain HashSet corrupts under that. + private val processedLock = KmpLock() private val processed = mutableSetOf() + /** Returns true when [pubkey] was not processed before (and marks it). */ + private fun markProcessed(pubkey: String) = processedLock.withLock { processed.add(pubkey) } + /** * Enqueue a pubkey for metadata fetching. * Duplicates within the same batch are automatically filtered. */ fun enqueue(pubkey: String) { - if (pubkey !in processed) { + if (!isProcessed(pubkey)) { queue.trySend(pubkey) } } @@ -64,24 +75,28 @@ class MetadataRateLimiter( fun start(onRequest: suspend (String) -> Unit) { scope.launch { val batch = mutableListOf() - queue.consumeAsFlow().collect { pubkey -> - if (pubkey !in processed) { + while (true) { + // A partial batch must not wait for the channel to close (it + // never does): flush it once the queue has been quiet for + // flushDelayMs, otherwise fewer than maxRequestsPerSecond + // pubkeys would never be requested at all. + val pubkey = + if (batch.isEmpty()) { + queue.receive() + } else { + withTimeoutOrNull(flushDelayMs) { queue.receive() } + } + + if (pubkey != null && markProcessed(pubkey)) { batch.add(pubkey) - processed.add(pubkey) } - // Process batch when we hit the limit or queue is empty - if (batch.size >= maxRequestsPerSecond) { + if (batch.size >= maxRequestsPerSecond || (pubkey == null && batch.isNotEmpty())) { processBatch(batch, onRequest) batch.clear() delay(1000) // Wait 1 second before next batch } } - - // Process remaining - if (batch.isNotEmpty()) { - processBatch(batch, onRequest) - } } } @@ -99,11 +114,11 @@ class MetadataRateLimiter( * Call this when switching accounts or clearing cache. */ fun reset() { - processed.clear() + processedLock.withLock { processed.clear() } } /** * Check if a pubkey has already been processed. */ - fun isProcessed(pubkey: String): Boolean = pubkey in processed + fun isProcessed(pubkey: String): Boolean = processedLock.withLock { pubkey in processed } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt index df7ce55e1a..bc7b83c218 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt @@ -479,7 +479,7 @@ class RichTextParser { val noProtocolUrlValidator = Regex( - "(([a-zA-Z0-9_-]+@)?([a-zA-Z0-9_-]+\\.)*[a-zA-Z0-9_-]+[\\.\\:][a-zA-Z0-9_]+([\\/ \\?\\=\\&\\#\\.]?[a-zA-Z0-9_-]+)*\\/?)(.*)", + "(([a-zA-Z0-9_-]+@)?([a-zA-Z0-9_-]+\\.)*[a-zA-Z0-9_-]+[\\.\\:][a-zA-Z0-9_]+([\\/ \\?\\=\\&\\#\\.][a-zA-Z0-9_-]+)*\\/?)(.*)", ) val imageExt = listOf("png", "jpg", "gif", "bmp", "jpeg", "webp", "svg", "avif") diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/search/SearchResultSorter.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/search/SearchResultSorter.kt index 54425f800f..0c1884976f 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/search/SearchResultSorter.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/search/SearchResultSorter.kt @@ -21,7 +21,9 @@ package com.vitorpamplona.amethyst.commons.search import com.vitorpamplona.amethyst.commons.model.User +import com.vitorpamplona.amethyst.commons.util.KmpLock import com.vitorpamplona.amethyst.commons.util.sortedBySnapshot +import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip23LongContent.LongTextNoteEvent import com.vitorpamplona.quartz.utils.currentTimeSeconds @@ -46,7 +48,7 @@ object SearchResultSorter { var score = 0.0 val content = event.content.lowercase() - val tokens = query.split("\\s+".toRegex()) + val tokens = query.split(WHITESPACE) // Exact phrase match in content if (content.contains(query)) { @@ -56,8 +58,7 @@ object SearchResultSorter { // Per-token scoring for (token in tokens) { if (token.isEmpty()) continue - val wordBoundary = "\\b${Regex.escape(token)}\\b".toRegex() - if (wordBoundary.containsMatchIn(content)) { + if (wordBoundaryRegex(token).containsMatchIn(content)) { score += 5.0 } else if (content.contains(token)) { score += 2.0 @@ -88,4 +89,20 @@ object SearchResultSorter { return score } + + private val WHITESPACE = Regex("\\s+") + + // scoreEvent runs once per result with the same query tokens; compiling a + // Pattern per (event, token) pair made a 500-result sort build thousands. + // Small, lock-guarded (sorts can run from several search coroutines). + private val cacheLock = KmpLock() + private val wordBoundaryCache = mutableMapOf() + + private fun wordBoundaryRegex(token: String): Regex = + cacheLock.withLock { + wordBoundaryCache[token] ?: Regex("\\b${Regex.escape(token)}\\b").also { + if (wordBoundaryCache.size > 64) wordBoundaryCache.clear() + wordBoundaryCache[token] = it + } + } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/search/UserSearchEngine.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/search/UserSearchEngine.kt index d094da60ab..a5d38a3742 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/search/UserSearchEngine.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/search/UserSearchEngine.kt @@ -24,12 +24,14 @@ import com.vitorpamplona.amethyst.commons.model.User import com.vitorpamplona.amethyst.commons.model.cache.ICacheProvider import com.vitorpamplona.quartz.nip19Bech32.decodePublicKeyAsHexOrNull import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach @@ -119,7 +121,11 @@ class UserSearchEngine( _localResults.value = emptyList() _isSearching.value = false } - }.launchIn(scope) + } + // findUsersStartingWith walks the whole user cache; callers pass a + // Compose scope, so keep the scan off the main dispatcher. + .flowOn(Dispatchers.Default) + .launchIn(scope) } private fun startRelaySearch(text: String) { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/BundledUpdate.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/BundledUpdate.kt index 79ff14f824..773b0544c5 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/BundledUpdate.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/BundledUpdate.kt @@ -152,26 +152,35 @@ class BasicBundledInsert( isProcessing = true } - processLoop@ while (true) { - val batch = - mutex.withLock { - if (queue.isEmpty()) { - isProcessing = false - null - } else { - val items = queue.toSet() - queue.clear() - items + try { + processLoop@ while (true) { + val batch = + mutex.withLock { + if (queue.isEmpty()) { + isProcessing = false + null + } else { + val items = queue.toSet() + queue.clear() + items + } } + + if (batch == null) break@processLoop + + if (batch.isNotEmpty()) { + onUpdate(batch) } - if (batch == null) break@processLoop - - if (batch.isNotEmpty()) { - onUpdate(batch) + delay(delay) + } + } finally { + // Mirrors BasicBundledUpdate: a throw out of onUpdate (or a + // cancellation) must not leave isProcessing stuck at true, or + // every later invalidateList() would enqueue and return forever. + withContext(NonCancellable) { + mutex.withLock { isProcessing = false } } - - delay(delay) } } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/state/EventCollectionState.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/state/EventCollectionState.kt index c357e72d0d..6a7a8e6235 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/state/EventCollectionState.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/state/EventCollectionState.kt @@ -173,11 +173,14 @@ class EventCollectionState( get() = _items.value.isEmpty() /** - * Schedules a batched update if not already scheduled. - * Cancels existing batch job and starts a new one. + * Schedules a batched update if not already scheduled (leading-edge + * throttle). Cancelling and restarting the timer on every insert instead + * would starve the flush on a busy feed: items arriving more often than + * [batchDelayMs] would keep resetting the delay and the screen would never + * update until the stream paused. */ private fun scheduleBatchUpdate() { - batchJob?.cancel() + if (batchJob?.isActive == true) return batchJob = scope.launch { delay(batchDelayMs) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/LiveStreamTopZappersViewModel.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/LiveStreamTopZappersViewModel.kt index 5fe963a9cd..0dc37c687d 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/LiveStreamTopZappersViewModel.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/LiveStreamTopZappersViewModel.kt @@ -153,8 +153,10 @@ class LiveStreamTopZappersViewModel( } } - private fun publish() { - val merged = streamContributions.values + goalContributions.values + private suspend fun publish() { + // Snapshot both maps under the same mutex their writers hold; the two + // collectors (channel changes, goal zaps) run on different coroutines. + val merged = mutex.withLock { streamContributions.values + goalContributions.values } _topZappers.value = LiveActivityTopZappersAggregator.aggregate(merged, limit) } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/SearchBarState.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/SearchBarState.kt index 657758c47a..c8dfef6af0 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/SearchBarState.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/SearchBarState.kt @@ -25,11 +25,13 @@ import com.vitorpamplona.amethyst.commons.model.cache.ICacheProvider import com.vitorpamplona.amethyst.commons.search.SearchResult import com.vitorpamplona.amethyst.commons.search.parseSearchInput import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach @@ -92,7 +94,11 @@ class SearchBarState( } else { _cachedUserResults.value = emptyList() } - }.launchIn(scope) + } + // The scan walks every cached user; callers hand us a Compose scope, + // so keep it off the main dispatcher. + .flowOn(Dispatchers.Default) + .launchIn(scope) } fun updateSearchText(text: String) { diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/blurhash/Base83Test.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/blurhash/Base83Test.kt new file mode 100644 index 0000000000..5b022d9aa7 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/blurhash/Base83Test.kt @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.blurhash + +import kotlin.test.Test +import kotlin.test.assertEquals + +class Base83Test { + @Test + fun charactersOutsideLatin1DecodeAsZeroInsteadOfThrowing() { + // The blurhash string comes from an attacker-controlled imeta tag. + assertEquals(0, Base83.decodeAt("€")) + assertEquals(0, Base83.decode("€￿")) + assertEquals(Base83.decodeFixed2("00"), Base83.decodeFixed2("€€")) + } + + @Test + fun alphabetRoundTrips() { + Base83.ALPHABET.forEachIndexed { i, c -> assertEquals(i, Base83.decodeAt(c.toString())) } + } +} diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/blurhash/CosineCacheTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/blurhash/CosineCacheTest.kt new file mode 100644 index 0000000000..fc3d251639 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/blurhash/CosineCacheTest.kt @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.blurhash + +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertFalse + +class CosineCacheTest { + @Test + fun sameProductDifferentShapeGetsDifferentTables() { + CosineCache.clearCache() + // (50, 4) and (100, 2) both have 200 entries; the old product key served + // whichever was computed first to both. + val a = CosineCache.getArrayForCosinesX(useCache = true, width = 50, numCompX = 4) + val b = CosineCache.getArrayForCosinesX(useCache = true, width = 100, numCompX = 2) + assertFalse(a.contentEquals(b)) + assertContentEquals(CosineCache.getArrayForCosinesX(useCache = false, width = 100, numCompX = 2), b) + } +} diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/relayClient/preload/MetadataRateLimiterTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/relayClient/preload/MetadataRateLimiterTest.kt new file mode 100644 index 0000000000..7a0fb84e63 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/relayClient/preload/MetadataRateLimiterTest.kt @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.relayClient.preload + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancel +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals + +@OptIn(ExperimentalCoroutinesApi::class) +class MetadataRateLimiterTest { + @Test + fun partialBatchIsFlushedWithoutWaitingForTheChannelToClose() = + runTest { + val scope = TestScope(testScheduler) + val requested = mutableListOf() + val limiter = MetadataRateLimiter(maxRequestsPerSecond = 20, scope = scope, flushDelayMs = 250) + limiter.start { requested.add(it) } + + // Fewer pubkeys than one batch: the old collect-then-flush never ran. + repeat(8) { limiter.enqueue("pubkey-$it") } + scope.advanceTimeBy(1_000) + + assertEquals((0 until 8).map { "pubkey-$it" }, requested) + scope.cancel() + } + + @Test + fun duplicatesAreRequestedOnce() = + runTest { + val scope = TestScope(testScheduler) + val requested = mutableListOf() + val limiter = MetadataRateLimiter(maxRequestsPerSecond = 20, scope = scope, flushDelayMs = 250) + limiter.start { requested.add(it) } + + limiter.enqueue("a") + limiter.enqueue("a") + scope.advanceTimeBy(2_000) + limiter.enqueue("a") + scope.advanceTimeBy(2_000) + + assertEquals(listOf("a"), requested) + scope.cancel() + } + + @Test + fun fullBatchesStillRateLimit() = + runTest { + val scope = TestScope(testScheduler) + val requested = mutableListOf() + val limiter = MetadataRateLimiter(maxRequestsPerSecond = 3, scope = scope, flushDelayMs = 250) + limiter.start { requested.add(it) } + + repeat(5) { limiter.enqueue("p$it") } + scope.advanceTimeBy(100) + assertEquals(3, requested.size, "first full batch goes out immediately") + scope.advanceTimeBy(2_000) + assertEquals(5, requested.size, "the partial remainder is flushed after the pause") + scope.cancel() + } +} diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/UrlWithoutSchemeTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/UrlWithoutSchemeTest.kt new file mode 100644 index 0000000000..54b0b6b1bd --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/UrlWithoutSchemeTest.kt @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.richtext + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import kotlin.time.measureTime + +class UrlWithoutSchemeTest { + @Test + fun recognisesSchemelessUrls() { + assertTrue(RichTextParser.isUrlWithoutScheme("example.com")) + assertTrue(RichTextParser.isUrlWithoutScheme("example.com/path?x=1")) + assertTrue(RichTextParser.isUrlWithoutScheme("user@example.com")) + assertTrue(RichTextParser.isUrlWithoutScheme("sub.example.com:8080/a-b_c")) + assertFalse(RichTextParser.isUrlWithoutScheme("just words")) + } + + @Test + fun pathologicalInputDoesNotBacktrackExponentially() { + // A trailing line terminator forces the regex to fail; with an optional + // separator inside the repeated group that failure took 2^n steps + // (13 s at n = 26). The composer runs this on every keystroke. + val input = "a.b" + "c".repeat(40) + "\r" + val elapsed = measureTime { RichTextParser.isUrlWithoutScheme(input) } + assertTrue(elapsed.inWholeMilliseconds < 1_000, "took $elapsed") + } +} diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlPreview.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlPreview.kt index d53c85f8e6..a3e3676e3f 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlPreview.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlPreview.kt @@ -26,6 +26,7 @@ import kotlinx.coroutines.withContext import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient import okhttp3.Request +import okhttp3.ResponseBody import okhttp3.coroutines.executeAsync class UrlPreview { @@ -63,7 +64,7 @@ class UrlPreview { ?: throw IllegalArgumentException("Website returned unknown mimetype: ${response.headers["Content-Type"]}") when { mimeType.type == "text" && mimeType.subtype == "html" -> { - val metaTags = HtmlParser().parseHtml(response.body.bytes(), mimeType.charset()?.name()) + val metaTags = HtmlParser().parseHtml(response.body.readPrefix(MAX_PREVIEW_BYTES), mimeType.charset()?.name()) val data = OpenGraphParser().extractUrlInfo(metaTags) UrlInfoItem( url, @@ -112,3 +113,16 @@ class UrlPreview { } } } + +/** + * Link previews only need the `` (MetaTagsParser stops at ``), and + * the URL is attacker-controlled: any note can point at a host that streams + * `text/html` forever. Read at most [max] bytes instead of `bytes()`. + */ +private fun ResponseBody.readPrefix(max: Long): ByteArray = + source().use { src -> + src.request(max) + src.buffer.readByteArray(minOf(src.buffer.size, max)) + } + +private const val MAX_PREVIEW_BYTES = 512L * 1024L diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/richtext/CachedRichTextParser.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/richtext/CachedRichTextParser.kt index 6f4b18c3fe..92a7bc5593 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/richtext/CachedRichTextParser.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/richtext/CachedRichTextParser.kt @@ -37,8 +37,35 @@ object CachedRichTextParser { // working set plus a few other feeds' recent entries, so pre-parsed bodies survive // until the render reads them and feed switches don't thrash. Each entry is one // note's parsed segments — typically single-digit KB. - private val richTextCache = ConcurrentLruCache(500) - private val isMarkdownCache = ConcurrentLruCache(200) + // Keyed by a 32-bit hash for speed, but every hit re-checks the inputs: + // String.hashCode() is trivially collidable ("Aa"/"BB"), and serving one + // note's parsed segments (images, invoices, link cards) under another + // note's author is not an acceptable failure mode. + private class CachedParse( + val content: String, + val tagsHash: Int, + val callbackUri: String?, + val authorPubKey: String?, + val state: RichTextViewerState, + ) { + fun matches( + content: String, + tagsHash: Int, + callbackUri: String?, + authorPubKey: String?, + ) = this.tagsHash == tagsHash && + this.callbackUri == callbackUri && + this.authorPubKey == authorPubKey && + this.content == content + } + + private class CachedMarkdown( + val content: String, + val isMarkdown: Boolean, + ) + + private val richTextCache = ConcurrentLruCache(500) + private val isMarkdownCache = ConcurrentLruCache(200) private fun hashCodeCache( content: String, @@ -75,7 +102,11 @@ object CachedRichTextParser { tags: ImmutableListOfLists, callbackUri: String? = null, authorPubKey: String? = null, - ): RichTextViewerState? = richTextCache.get(hashCodeCache(content, tags, callbackUri, authorPubKey)) + ): RichTextViewerState? = + richTextCache + .get(hashCodeCache(content, tags, callbackUri, authorPubKey)) + ?.takeIf { it.matches(content, tags.contentHash(), callbackUri, authorPubKey) } + ?.state fun parseText( content: String, @@ -84,12 +115,13 @@ object CachedRichTextParser { authorPubKey: String? = null, ): RichTextViewerState { val key = hashCodeCache(content, tags, callbackUri, authorPubKey) + val tagsHash = tags.contentHash() val cached = richTextCache.get(key) - return if (cached != null) { - cached + return if (cached != null && cached.matches(content, tagsHash, callbackUri, authorPubKey)) { + cached.state } else { val newState = RichTextParser().parseText(content, tags, callbackUri, authorPubKey) - richTextCache.put(key, newState) + richTextCache.put(key, CachedParse(content, tagsHash, callbackUri, authorPubKey, newState)) newState } } @@ -98,9 +130,9 @@ object CachedRichTextParser { // notes only pays for the scan once. The decision is purely a function of `content`. fun isMarkdown(content: String): Boolean { val key = content.hashCode() - isMarkdownCache.get(key)?.let { return it } + isMarkdownCache.get(key)?.takeIf { it.content == content }?.let { return it.isMarkdown } val result = computeIsMarkdown(content) - isMarkdownCache.put(key, result) + isMarkdownCache.put(key, CachedMarkdown(content, result)) return result } diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/EncryptedBlobInterceptor.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/EncryptedBlobInterceptor.kt index 5e735054c0..8a99353d70 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/EncryptedBlobInterceptor.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/EncryptedBlobInterceptor.kt @@ -30,14 +30,13 @@ class EncryptedBlobInterceptor( val cache: EncryptionKeyCache, ) : Interceptor { private fun Response.decryptOrNullWithErrorCorrection(info: DecryptInformation): Response? { - val body = peekBody(Long.MAX_VALUE) - - // Only tries to decrypt if the content-type is a byte array - // if (body.contentType().toString() != "application/octet-stream") { - // return null - // } - - val bytes = body.bytes() + // The whole blob has to be in memory to decrypt it (the AEAD tag covers + // all of it), but read the body once and close it: peekBody(MAX) used to + // buffer a second full copy and left the original stream — and its + // connection-pool slot, since this is a network interceptor — open. + val body = this.body + val contentType = body.contentType() + val bytes = body.use { it.bytes() } // Tries the correct way first // if it fails, tries to decrypt as UTF8 nonce, which was how @@ -57,7 +56,7 @@ class EncryptedBlobInterceptor( .apply { body( decrypted.toResponseBody( - info.mimeType?.toMediaTypeOrNull() ?: body.contentType(), + info.mimeType?.toMediaTypeOrNull() ?: contentType, ), ) // removes hints that would make the app requrest partial byte arrays diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/OnionLocationInterceptor.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/OnionLocationInterceptor.kt index 9554663538..108b22b9c2 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/OnionLocationInterceptor.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/OnionLocationInterceptor.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.commons.service.http +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.Interceptor import okhttp3.Response @@ -47,9 +48,25 @@ class OnionLocationInterceptor( override fun intercept(chain: Interceptor.Chain): Response { val response = chain.proceed(chain.request()) val onionLocation = response.header("Onion-Location") - if (onionLocation != null) { + if (onionLocation != null && isOnionService(onionLocation)) { cache.put(chain.request().url.host, onionLocation) } return response } + + private companion object { + /** + * Only a `.onion` target may be cached. [OnionUrlRewriteInterceptor] + * re-points every Tor request for the host at the cached location and + * deliberately allows https → http because the Tor circuit provides the + * transport security — a guarantee that only holds for onion services. + * A clearnet (or attacker-supplied) `Onion-Location` would otherwise + * redirect LNURL/NIP-05/media traffic to an arbitrary plaintext host + * for the cache TTL. + */ + fun isOnionService(location: String): Boolean { + val host = location.toHttpUrlOrNull()?.host ?: return false + return host.endsWith(".onion", ignoreCase = true) + } + } } diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/namecoin/NamecoinNameService.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/namecoin/NamecoinNameService.kt index c88e38e9fa..7fb2690203 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/namecoin/NamecoinNameService.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/namecoin/NamecoinNameService.kt @@ -28,6 +28,8 @@ import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.NamecoinLookupCache import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.NamecoinNameResolver import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.NamecoinNostrResult import com.vitorpamplona.quartz.nip05DnsIdentifiers.namecoin.NamecoinResolveOutcome +import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -130,8 +132,11 @@ class NamecoinNameService( } else { NamecoinResolveState.NotFound } + } catch (e: CancellationException) { + throw e } catch (e: Exception) { - state.value = NamecoinResolveState.Error(e.message ?: "Unknown error") + Log.w("NamecoinNameService", "resolve failed for $identifier", e) + state.value = NamecoinResolveState.Error(e.message ?: e::class.simpleName ?: "Unknown error") } } return state diff --git a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt index 85c8b9c95e..5cd5aba9a0 100644 --- a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt +++ b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt @@ -37,7 +37,7 @@ import java.security.SecureRandom import java.util.Base64 import javax.crypto.Cipher import javax.crypto.SecretKeyFactory -import javax.crypto.spec.IvParameterSpec +import javax.crypto.spec.GCMParameterSpec import javax.crypto.spec.PBEKeySpec import javax.crypto.spec.SecretKeySpec @@ -78,6 +78,10 @@ actual class SecureKeyStorage private actual constructor() { // Encryption constants for fallback private const val ALGORITHM = "AES" private const val TRANSFORMATION = "AES/GCM/NoPadding" + + // AES-GCM must be initialised with GCMParameterSpec (tag length + IV); + // IvParameterSpec throws InvalidAlgorithmParameterException on every JDK. + private const val GCM_TAG_BITS = 128 private const val KEY_LENGTH = 256 private const val ITERATION_COUNT = 100000 private const val IV_LENGTH = 12 // GCM standard @@ -393,7 +397,7 @@ actual class SecureKeyStorage private actual constructor() { } } - private fun encryptData( + internal fun encryptData( plaintext: String, password: String, ): String { @@ -405,14 +409,14 @@ actual class SecureKeyStorage private actual constructor() { val key = SecretKeySpec(secretKey.encoded, ALGORITHM) val cipher = Cipher.getInstance(TRANSFORMATION) - cipher.init(Cipher.ENCRYPT_MODE, key, IvParameterSpec(iv)) + cipher.init(Cipher.ENCRYPT_MODE, key, GCMParameterSpec(GCM_TAG_BITS, iv)) val encrypted = cipher.doFinal(plaintext.toByteArray()) val combined = salt + iv + encrypted return Base64.getEncoder().encodeToString(combined) } - private fun decryptData( + internal fun decryptData( ciphertext: String, password: String, ): String { @@ -427,7 +431,7 @@ actual class SecureKeyStorage private actual constructor() { val key = SecretKeySpec(secretKey.encoded, ALGORITHM) val cipher = Cipher.getInstance(TRANSFORMATION) - cipher.init(Cipher.DECRYPT_MODE, key, IvParameterSpec(iv)) + cipher.init(Cipher.DECRYPT_MODE, key, GCMParameterSpec(GCM_TAG_BITS, iv)) val decrypted = cipher.doFinal(encrypted) return String(decrypted) diff --git a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/service/upload/MediaCompressor.kt b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/service/upload/MediaCompressor.kt index 7c12f8a647..469eaee0e8 100644 --- a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/service/upload/MediaCompressor.kt +++ b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/service/upload/MediaCompressor.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.commons.service.upload +import com.vitorpamplona.quartz.utils.Log import org.apache.commons.imaging.Imaging import org.apache.commons.imaging.formats.jpeg.exif.ExifRewriter import java.io.ByteArrayOutputStream @@ -37,7 +38,9 @@ object MediaCompressor { * carries no EXIF, or strip fails. */ fun stripExif(file: File): File { - if (!file.name.lowercase().let { it.endsWith(".jpg") || it.endsWith(".jpeg") }) { + // Sniff the bytes rather than trusting the name: a camera JPEG handed + // over as an extension-less temp file must still lose its GPS tags. + if (ImageFormatSniffer.sniff(file) !is ImageFormat.Jpeg) { return file } @@ -52,7 +55,9 @@ object MediaCompressor { val stripped = AmethystTempDir.createTempFile("amethyst_stripped_", ".jpg") stripped.writeBytes(baos.toByteArray()) stripped - } catch (_: Exception) { + } catch (e: Exception) { + // Never silent: the UI promises the original is stripped before upload. + Log.w("MediaCompressor", "EXIF strip failed for ${file.name}; uploading unstripped original", e) file } } diff --git a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/service/upload/StaticSitePublisher.kt b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/service/upload/StaticSitePublisher.kt index 6a9b5cd000..6a07c34a1b 100644 --- a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/service/upload/StaticSitePublisher.kt +++ b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/service/upload/StaticSitePublisher.kt @@ -71,7 +71,10 @@ class StaticSitePublisher( root.isDirectory -> root .walkTopDown() - .filter { it.isFile } + // Never follow a link out of the published tree: a symlink + // to ~ would publish the home directory to a Blossom server. + .onEnter { !Files.isSymbolicLink(it.toPath()) } + .filter { it.isFile && !Files.isSymbolicLink(it.toPath()) && it.canonicalFile.startsWith(root) } .sortedBy { it.invariantPath() } .toList() else -> throw IllegalArgumentException("No such file or directory: ${root.path}") diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageFallbackCipherTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageFallbackCipherTest.kt new file mode 100644 index 0000000000..39d5553704 --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageFallbackCipherTest.kt @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.keystorage + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Test + +class SecureKeyStorageFallbackCipherTest { + @Test + fun encryptedFileFallbackRoundTrips() { + // AES/GCM initialised with IvParameterSpec threw on every JDK, which made + // the whole no-keyring fallback (headless Linux, containers) dead code. + val storage = SecureKeyStorage.create() + val ciphertext = storage.encryptData("nsec1secret", "correct horse") + assertNotEquals("nsec1secret", ciphertext) + assertEquals("nsec1secret", storage.decryptData(ciphertext, "correct horse")) + } + + @Test(expected = Exception::class) + fun wrongPasswordIsRejected() { + val storage = SecureKeyStorage.create() + storage.decryptData(storage.encryptData("nsec1secret", "right"), "wrong") + } +} diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/nip64Chess/ui/InteractiveChessBoard.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/nip64Chess/ui/InteractiveChessBoard.kt index be771b0c58..20bce5dffd 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/nip64Chess/ui/InteractiveChessBoard.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/nip64Chess/ui/InteractiveChessBoard.kt @@ -80,9 +80,11 @@ fun InteractiveChessBoard( ) { // Track local move count + external version to trigger recomposition when board changes var localMoveCount by remember { mutableStateOf(0) } - val effectiveVersion = localMoveCount + positionVersion - val position = remember(engine, effectiveVersion) { engine.getPosition() } - val sideToMove = remember(engine, effectiveVersion) { engine.getSideToMove() } + // Two independent counters are two keys, not a sum: a re-synced history + // that resets positionVersion while localMoveCount persists must not land + // on the same key and serve a stale position. + val position = remember(engine, localMoveCount, positionVersion) { engine.getPosition() } + val sideToMove = remember(engine, localMoveCount, positionVersion) { engine.getSideToMove() } var selectedSquare by remember { mutableStateOf?>(null) } var legalMoves by remember { mutableStateOf>(emptyList()) } diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/nip64Chess/ui/LiveChessGame.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/nip64Chess/ui/LiveChessGame.kt index 8a1b529390..06787b1477 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/nip64Chess/ui/LiveChessGame.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/nip64Chess/ui/LiveChessGame.kt @@ -648,8 +648,10 @@ private fun MoveHistoryDisplay(moves: List) { horizontalArrangement = Arrangement.spacedBy(4.dp), verticalAlignment = Alignment.CenterVertically, ) { - // Group moves into pairs (white, black) - moves.chunked(2).forEachIndexed { index, movePair -> + // Group moves into pairs (white, black); remembered so a + // recomposition per relay move doesn't re-chunk the list. + val movePairs = remember(moves) { moves.chunked(2) } + movePairs.forEachIndexed { index, movePair -> // Move number Text( text = "${index + 1}.", diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/BunkerHeartbeatIndicator.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/BunkerHeartbeatIndicator.kt index 43f01bf82c..8dfc7b1cb8 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/BunkerHeartbeatIndicator.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/BunkerHeartbeatIndicator.kt @@ -34,10 +34,9 @@ import androidx.compose.material3.TooltipBox import androidx.compose.material3.TooltipDefaults import androidx.compose.material3.rememberTooltipState import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.scale import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.commons.domain.nip46.SignerConnectionState import com.vitorpamplona.amethyst.commons.icons.symbols.Icon @@ -79,21 +78,28 @@ fun BunkerHeartbeatIndicator( when (signerConnectionState) { is SignerConnectionState.Connected -> { val infiniteTransition = rememberInfiniteTransition(label = "heartbeat") - val scale by infiniteTransition.animateFloat( - initialValue = 0.85f, - targetValue = 1.15f, - animationSpec = - infiniteRepeatable( - animation = tween(800), - repeatMode = RepeatMode.Reverse, - ), - label = "heartbeatScale", - ) + // No `by`: reading the value in composition recomposed the + // indicator every frame for as long as a bunker was connected. + val scale = + infiniteTransition.animateFloat( + initialValue = 0.85f, + targetValue = 1.15f, + animationSpec = + infiniteRepeatable( + animation = tween(800), + repeatMode = RepeatMode.Reverse, + ), + label = "heartbeatScale", + ) Icon( MaterialSymbols.Favorite, contentDescription = "Bunker connected", tint = Color(0xFF4CAF50), - modifier = Modifier.size(20.dp).scale(scale), + modifier = + Modifier.size(20.dp).graphicsLayer { + scaleX = scale.value + scaleY = scale.value + }, ) } diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ClickableTexts.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ClickableTexts.kt index b1a933a09d..eaa3fbff7e 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ClickableTexts.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ClickableTexts.kt @@ -70,9 +70,12 @@ fun ClickableTextColor( linkColor: Color = MaterialTheme.colorScheme.primary, onClick: () -> Unit, ) { + // onClick is baked into the LinkAnnotation, so it must be a key: a recycled + // slot with the same label and a new handler would otherwise keep firing + // the old target. Text( text = - remember(text) { + remember(text, linkColor, onClick) { buildAnnotatedString { appendLink(text, linkColor, onClick) } @@ -97,7 +100,7 @@ fun ClickableTextNormal( ) { Text( text = - remember(text) { + remember(text, onClick) { buildAnnotatedString { appendLink(text, onClick) } diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/GlowingCard.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/GlowingCard.kt index efb78583f7..d00cb843b1 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/GlowingCard.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/GlowingCard.kt @@ -31,7 +31,8 @@ import androidx.compose.foundation.layout.padding import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.drawBehind +import androidx.compose.ui.draw.drawWithCache +import androidx.compose.ui.geometry.CornerRadius import androidx.compose.ui.graphics.Brush import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.PathEffect @@ -44,6 +45,9 @@ import androidx.compose.ui.unit.TextUnit import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp +private val GLOW_BRUSH = Brush.sweepGradient(colors = listOf(Color.Cyan, Color.Magenta, Color.Yellow)) +private val DASH_INTERVALS = floatArrayOf(10f, 10f) + @Composable fun AnimatedBorderTextCornerRadius( text: String, @@ -69,25 +73,24 @@ fun AnimatedBorderTextCornerRadius( fontSize = fontSize, modifier = modifier - .drawBehind { - val brush = - Brush.sweepGradient( - colors = listOf(Color.Cyan, Color.Magenta, Color.Yellow), + .drawWithCache { + // Brush, stroke geometry and corner radius don't change per + // frame; only the dash phase does. Build them once per size. + val strokeWidth = 2.dp.toPx() + val cornerRadius = CornerRadius(6.dp.toPx()) + onDrawBehind { + drawRoundRect( + brush = GLOW_BRUSH, + style = + Stroke( + width = strokeWidth, + cap = StrokeCap.Round, + join = StrokeJoin.Round, + pathEffect = PathEffect.dashPathEffect(DASH_INTERVALS, animatedFloatRestart.value), + ), + cornerRadius = cornerRadius, ) - - drawRoundRect( - brush = brush, - style = - Stroke( - width = 2.dp.toPx(), - cap = StrokeCap.Round, - join = StrokeJoin.Round, - pathEffect = PathEffect.dashPathEffect(floatArrayOf(10f, 10f), animatedFloatRestart.value), - ), - cornerRadius = - androidx.compose.ui.geometry - .CornerRadius(6.dp.toPx()), - ) + } }.padding(3.dp), color = color, textAlign = textAlign, diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/LoadingAnimation.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/LoadingAnimation.kt index 2b98a61fe5..26e57a9a44 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/LoadingAnimation.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/LoadingAnimation.kt @@ -32,11 +32,11 @@ import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.ProgressIndicatorDefaults import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.rotate import androidx.compose.ui.graphics.Brush import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import kotlinx.collections.immutable.ImmutableList @@ -65,7 +65,9 @@ fun LoadingAnimation( ) { val infiniteTransition = rememberInfiniteTransition() - val rotateAnimation by + // Read in graphicsLayer, not composition: `Modifier.rotate(value)` recomposed + // the indicator every frame of the loop and rebuilt the sweep gradient. + val rotateAnimation = infiniteTransition.animateFloat( initialValue = 0f, targetValue = 360f, @@ -80,15 +82,17 @@ fun LoadingAnimation( label = "UploadGalleryUploadingAnimation", ) + val brush = remember(circleColors) { Brush.sweepGradient(circleColors) } + CircularProgressIndicator( progress = { 1f }, modifier = Modifier .size(size = indicatorSize) - .rotate(degrees = rotateAnimation) + .graphicsLayer { rotationZ = rotateAnimation.value } .border( width = circleWidth, - brush = Brush.sweepGradient(circleColors), + brush = brush, shape = CircleShape, ), color = MaterialTheme.colorScheme.background, diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/RobohashImage.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/RobohashImage.kt index 6421bc9717..2ea14b21e8 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/RobohashImage.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/RobohashImage.kt @@ -31,17 +31,7 @@ import androidx.compose.ui.layout.ContentScale import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.icons.symbols.rememberMaterialSymbolPainter import com.vitorpamplona.amethyst.commons.robohash.CachedRobohash - -/** - * Determines if the current color scheme is light. - * Uses the luminance of the background color. - */ -@Composable -private fun isLightTheme(): Boolean { - val background = MaterialTheme.colorScheme.background - // Simple luminance check: if any RGB component > 0.5, consider it light - return (background.red + background.green + background.blue) / 3 > 0.5f -} +import com.vitorpamplona.amethyst.commons.ui.theme.isLight /** * Displays a robohash image based on a seed string (typically a public key). @@ -61,7 +51,7 @@ fun RobohashImage( ) { if (loadRobohash) { Image( - imageVector = CachedRobohash.get(robot, isLightTheme()), + imageVector = CachedRobohash.get(robot, MaterialTheme.colorScheme.isLight), contentDescription = contentDescription, modifier = modifier, ) @@ -90,7 +80,7 @@ fun RobohashImage( ) { if (loadRobohash) { Image( - painter = rememberVectorPainter(CachedRobohash.get(robot, isLightTheme())), + painter = rememberVectorPainter(CachedRobohash.get(robot, MaterialTheme.colorScheme.isLight)), contentDescription = contentDescription, modifier = modifier, alignment = alignment, diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ShimmerPlaceholder.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ShimmerPlaceholder.kt index 0be7a93ab2..c38be8f417 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ShimmerPlaceholder.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ShimmerPlaceholder.kt @@ -26,7 +26,6 @@ import androidx.compose.animation.core.animateFloat import androidx.compose.animation.core.infiniteRepeatable import androidx.compose.animation.core.rememberInfiniteTransition import androidx.compose.animation.core.tween -import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -38,9 +37,10 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.CircleShape import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.draw.drawBehind import androidx.compose.ui.geometry.Offset import androidx.compose.ui.graphics.Brush import androidx.compose.ui.unit.dp @@ -48,32 +48,47 @@ import androidx.compose.ui.unit.dp @Composable fun ShimmerPlaceholder(modifier: Modifier = Modifier) { val transition = rememberInfiniteTransition(label = "shimmer") - val translateAnim by transition.animateFloat( - initialValue = 0f, - targetValue = 1000f, - animationSpec = - infiniteRepeatable( - animation = tween(durationMillis = 1200, easing = LinearEasing), - repeatMode = RepeatMode.Restart, - ), - label = "shimmerTranslate", - ) + // Kept as a State (no `by`) and read inside drawBehind: reading it in + // composition recomposed every shimmer — six per NoteCardSkeleton — at the + // display refresh rate and rebuilt the brush and background modifier each + // frame, exactly while the feed is busy parsing events. + val translateAnim = + transition.animateFloat( + initialValue = 0f, + targetValue = 1000f, + animationSpec = + infiniteRepeatable( + animation = tween(durationMillis = 1200, easing = LinearEasing), + repeatMode = RepeatMode.Restart, + ), + label = "shimmerTranslate", + ) + val colorScheme = MaterialTheme.colorScheme val shimmerColors = - listOf( - MaterialTheme.colorScheme.surfaceContainerHigh, - MaterialTheme.colorScheme.surfaceContainer, - MaterialTheme.colorScheme.surfaceContainerHigh, - ) + remember(colorScheme) { + listOf( + colorScheme.surfaceContainerHigh, + colorScheme.surfaceContainer, + colorScheme.surfaceContainerHigh, + ) + } + val shape = MaterialTheme.shapes.small - val brush = - Brush.linearGradient( - colors = shimmerColors, - start = Offset(translateAnim - 200f, translateAnim - 200f), - end = Offset(translateAnim, translateAnim), - ) - - Box(modifier.background(brush, MaterialTheme.shapes.small)) + Box( + modifier + .clip(shape) + .drawBehind { + val t = translateAnim.value + drawRect( + Brush.linearGradient( + colors = shimmerColors, + start = Offset(t - 200f, t - 200f), + end = Offset(t, t), + ), + ) + }, + ) } @Composable diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ZonedSwipeModifier.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ZonedSwipeModifier.kt index 7ac6216eca..a0e8754ed1 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ZonedSwipeModifier.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/ZonedSwipeModifier.kt @@ -28,6 +28,7 @@ import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.composed @@ -50,8 +51,12 @@ fun Modifier.zonedDrawerSwipe( var gestureStartPage by remember { mutableIntStateOf(0) } var drawerOpened by remember { mutableStateOf(false) } + // The connection is remembered for the pager's lifetime; read the + // current lambda through rememberUpdatedState so a caller that + // re-creates openDrawer (new drawer state, account switch) is honoured. + val currentOpenDrawer by rememberUpdatedState(openDrawer) val connection = - remember { + remember(pagerState) { object : NestedScrollConnection { override fun onPreScroll( available: Offset, @@ -68,7 +73,7 @@ fun Modifier.zonedDrawerSwipe( if (!wasOnFirstPage && !isInPagerZone) { drawerOpened = true - openDrawer() + currentOpenDrawer() return Offset(available.x, 0f) } } @@ -87,7 +92,7 @@ fun Modifier.zonedDrawerSwipe( // so child LazyRows can scroll first. if (available.x > 0f && gestureStartPage == 0) { drawerOpened = true - openDrawer() + currentOpenDrawer() return Offset(available.x, 0f) } return Offset.Zero diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/richtext/CustomEmojiRenderer.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/richtext/CustomEmojiRenderer.kt index 0b611c9958..1cce447446 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/richtext/CustomEmojiRenderer.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/richtext/CustomEmojiRenderer.kt @@ -82,28 +82,13 @@ fun InLineIconRenderer( if (fontSize == TextUnit.Unspecified) 22.sp else fontSize.times(1.1f) } + // Remembered like annotatedText below: rebuilding this map handed Text a + // fresh unstable Map (plus a Placeholder and lambda per emoji) on every + // recomposition of every name/note with a custom emoji. val inlineContent = - wordsInOrder - .mapIndexedNotNull { idx, value -> - if (value is CustomEmoji.ImageUrlType) { - "inlineContent$idx" to - InlineTextContent( - Placeholder( - width = placeholderSize, - height = placeholderSize, - placeholderVerticalAlign = PlaceholderVerticalAlign.Center, - ), - ) { - AsyncImage( - model = value.url, - contentDescription = null, - modifier = Modifier.fillMaxSize().padding(horizontal = 0.dp), - ) - } - } else { - null - } - }.associate { it.first to it.second } + remember(wordsInOrder, placeholderSize) { + wordsInOrder.buildInlineContent(placeholderSize) + } val annotatedText = remember(wordsInOrder, style) { @@ -127,3 +112,25 @@ fun InLineIconRenderer( modifier = modifier, ) } + +private fun ImmutableList.buildInlineContent(placeholderSize: TextUnit): Map = + mapIndexedNotNull { idx, value -> + if (value is CustomEmoji.ImageUrlType) { + "inlineContent$idx" to + InlineTextContent( + Placeholder( + width = placeholderSize, + height = placeholderSize, + placeholderVerticalAlign = PlaceholderVerticalAlign.Center, + ), + ) { + AsyncImage( + model = value.url, + contentDescription = null, + modifier = Modifier.fillMaxSize().padding(horizontal = 0.dp), + ) + } + } else { + null + } + }.associate { it.first to it.second } diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/search/SearchPeoplePicker.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/search/SearchPeoplePicker.kt index a8f3ade857..ad427f317a 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/search/SearchPeoplePicker.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/search/SearchPeoplePicker.kt @@ -29,7 +29,7 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -76,9 +76,9 @@ fun SearchPeoplePicker( ) { if (candidates.isEmpty()) return LazyColumn(modifier.heightIn(max = 280.dp)) { - items(candidates, key = { it.pubkeyHex }) { candidate -> + itemsIndexed(candidates, key = { _, candidate -> candidate.pubkeyHex }) { index, candidate -> PickerRow( - highlighted = candidates.indexOf(candidate) == highlighted, + highlighted = index == highlighted, onClick = { onPick(candidate) }, leading = { UserAvatar( @@ -105,9 +105,9 @@ fun SearchGroupPicker( ) { if (candidates.isEmpty()) return LazyColumn(modifier.heightIn(max = 280.dp)) { - items(candidates, key = { it.id }) { candidate -> + itemsIndexed(candidates, key = { _, candidate -> candidate.id }) { index, candidate -> PickerRow( - highlighted = candidates.indexOf(candidate) == highlighted, + highlighted = index == highlighted, onClick = { onPick(candidate) }, title = candidate.name, subtitle = candidate.subtitle, @@ -132,9 +132,9 @@ fun SearchKindPicker( ) { if (candidates.isEmpty()) return LazyColumn(modifier.heightIn(max = 280.dp)) { - items(candidates, key = { it.alias }) { candidate -> + itemsIndexed(candidates, key = { _, candidate -> candidate.alias }) { index, candidate -> PickerRow( - highlighted = candidates.indexOf(candidate) == highlighted, + highlighted = index == highlighted, onClick = { onPick(candidate) }, title = candidate.alias, // What the token will actually ask for. `kind:video` is four kinds and