From 8c62e90711b941e4bf84caa96995dac186580844 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Mon, 27 Jul 2026 18:00:59 +0200 Subject: [PATCH 1/7] ui: keep unread DM senders visible --- .../com/bitchat/android/ui/ChatViewModel.kt | 21 ++ .../bitchat/android/ui/GeohashPeopleList.kt | 14 +- .../bitchat/android/ui/MeshPeerListSheet.kt | 206 ++++++++++++++++-- .../android/ui/UnreadConversationSummary.kt | 86 ++++++++ .../ui/UnreadConversationSummaryTest.kt | 117 ++++++++++ 5 files changed, 418 insertions(+), 26 deletions(-) create mode 100644 app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt create mode 100644 app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt diff --git a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt index 3017b20c..6ef0ff81 100644 --- a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt +++ b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt @@ -8,7 +8,10 @@ import androidx.lifecycle.viewModelScope import com.bitchat.android.favorites.FavoritesPersistenceService import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.stateIn import com.bitchat.android.mesh.BluetoothMeshDelegate import com.bitchat.android.mesh.BluetoothMeshService import com.bitchat.android.mesh.MeshService @@ -171,6 +174,24 @@ class ChatViewModel( val privateChats: StateFlow>> = state.privateChats val selectedPrivateChatPeer: StateFlow = state.selectedPrivateChatPeer val unreadPrivateMessages: StateFlow> = state.unreadPrivateMessages + internal val unreadConversations: StateFlow> = combine( + state.unreadPrivateMessages, + state.privateChats, + state.nickname + ) { unreadConversationIDs, chats, currentNickname -> + val seenStore = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) + buildUnreadConversationSummaries( + unreadConversationIDs = unreadConversationIDs, + privateChats = chats, + currentUserIdentifiers = setOf(currentNickname, mesh.myPeerID), + canonicalize = ContactDirectory::canonicalConversationId, + isMessageRead = { message -> seenStore.hasRead(message.id) } + ) + }.stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = emptyList() + ) val joinedChannels: StateFlow> = state.joinedChannels val currentChannel: StateFlow = state.currentChannel val channelMessages: StateFlow>> = state.channelMessages diff --git a/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt b/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt index 0cf88b74..e8f87668 100644 --- a/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt +++ b/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt @@ -19,6 +19,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.bitchat.android.ui.theme.BitchatFontFamily import com.bitchat.android.ui.theme.colorForPeer import com.bitchat.android.R +import com.bitchat.android.services.ContactDirectory import com.bitchat.android.ui.theme.LocalBitchatPalette import java.util.* @@ -36,7 +37,8 @@ data class GeoPerson( fun GeohashPeopleList( viewModel: ChatViewModel, onTapPerson: () -> Unit, - modifier: Modifier = Modifier + modifier: Modifier = Modifier, + excludedConversationIDs: Set = emptySet() ) { val geohashPeople by viewModel.geohashPeople.collectAsStateWithLifecycle() val selectedLocationChannel by viewModel.selectedLocationChannel.collectAsStateWithLifecycle() @@ -77,9 +79,15 @@ fun GeohashPeopleList( geohashPeople } } - val sections = remember(peopleIncludingSelf, myHex, isTeleported, teleportedGeo) { + val visiblePeople = remember(peopleIncludingSelf, excludedConversationIDs) { + peopleIncludingSelf.filterNot { person -> + val alias = "nostr_${person.id.take(16)}" + ContactDirectory.canonicalConversationId(alias) in excludedConversationIDs + } + } + val sections = remember(visiblePeople, myHex, isTeleported, teleportedGeo) { sectionGeohashPeople( - people = peopleIncludingSelf, + people = visiblePeople, myId = myHex, selfIsTeleported = isTeleported, teleportedIds = teleportedGeo diff --git a/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt b/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt index c496938e..7ec2fc0a 100644 --- a/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt +++ b/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt @@ -82,9 +82,16 @@ fun MeshPeerListSheet( val peerRSSI by viewModel.peerRSSI.collectAsStateWithLifecycle() val selectedLocationChannel by viewModel.selectedLocationChannel.collectAsStateWithLifecycle() val geohashPeople by viewModel.geohashPeople.collectAsStateWithLifecycle() + val unreadConversations by viewModel.unreadConversations.collectAsStateWithLifecycle() val geohashPeopleCount = geohashPeople.size val wifiAwareConnected by com.bitchat.android.wifiaware.WifiAwareController.connectedPeers.collectAsStateWithLifecycle() val wifiAwarePeerIDs = remember(wifiAwareConnected) { wifiAwareConnected.keys.toSet() } + val unreadConversationIDs = remember(unreadConversations) { + unreadConversations.mapTo(mutableSetOf()) { it.conversationID } + } + val visibleConnectedPeers = connectedPeers.filterNot { peerID -> + ContactDirectory.canonicalConversationId(peerID) in unreadConversationIDs + } // Bottom sheet state val sheetState = rememberModalBottomSheetState( @@ -117,7 +124,22 @@ fun MeshPeerListSheet( ) { val peopleCount = when (selectedLocationChannel) { is ChannelID.Location -> geohashPeopleCount - else -> connectedPeers.count { it != viewModel.myPeerID } + else -> visibleConnectedPeers.count { it != viewModel.myPeerID } + } + + if (unreadConversations.isNotEmpty()) { + item(key = "unread_private_messages_section") { + UnreadDirectMessagesSection( + conversations = unreadConversations, + connectedPeers = connectedPeers, + viewModel = viewModel, + onPrivateChatStart = { conversationID -> + viewModel.showPrivateChatSheet(conversationID) + onDismiss() + }, + modifier = Modifier.padding(top = 8.dp) + ) + } } // Channels section @@ -127,7 +149,9 @@ fun MeshPeerListSheet( SheetIconSectionHeader( iconRes = R.drawable.ic_spec_chat_bubbles, title = stringResource(R.string.channels), - modifier = Modifier.padding(top = 8.dp) + modifier = Modifier.padding( + top = if (unreadConversations.isNotEmpty()) 20.dp else 8.dp + ) ) Surface( modifier = Modifier @@ -179,8 +203,12 @@ fun MeshPeerListSheet( GeohashPeopleList( viewModel = viewModel, onTapPerson = onDismiss, + excludedConversationIDs = unreadConversationIDs, modifier = Modifier.padding( - top = if (joinedChannels.isNotEmpty()) 20.dp else 8.dp + top = if ( + joinedChannels.isNotEmpty() || + unreadConversations.isNotEmpty() + ) 20.dp else 8.dp ) ) } @@ -188,9 +216,12 @@ fun MeshPeerListSheet( else -> { PeopleSection( modifier = Modifier.padding( - top = if (joinedChannels.isNotEmpty()) 20.dp else 8.dp + top = if ( + joinedChannels.isNotEmpty() || + unreadConversations.isNotEmpty() + ) 20.dp else 8.dp ), - connectedPeers = connectedPeers, + connectedPeers = visibleConnectedPeers, peerNicknames = peerNicknames, peerRSSI = peerRSSI, nickname = nickname, @@ -198,6 +229,7 @@ fun MeshPeerListSheet( selectedPrivatePeer = selectedPrivatePeer, wifiAwarePeerIDs = wifiAwarePeerIDs, peopleCount = peopleCount, + excludedConversationIDs = unreadConversationIDs, viewModel = viewModel, onPrivateChatStart = { peerID -> viewModel.showPrivateChatSheet(peerID) @@ -312,6 +344,7 @@ fun PeopleSection( selectedPrivatePeer: String?, wifiAwarePeerIDs: Set = emptySet(), peopleCount: Int = 0, + excludedConversationIDs: Set = emptySet(), viewModel: ChatViewModel, onPrivateChatStart: (String) -> Unit ) { @@ -410,8 +443,6 @@ fun PeopleSection( ) // Build a map of base name counts across all people shown in the list (connected + offline + nostr) - val hex64Regex = Regex("^[0-9a-fA-F]{64}$") - // Helper to compute display name used for a given key fun computeDisplayNameForPeerId(key: String): String { return if (key == nickname) "You" else (peerNicknames[key] ?: (privateChats[key]?.lastOrNull()?.sender ?: key.take(12))) @@ -430,33 +461,29 @@ fun PeopleSection( val offlineFavorites = FavoritesPersistenceService.shared.getOurFavorites() offlineFavorites.forEach { fav -> val favPeerID = ContactIdentityResolver.noiseKeyHex(fav.peerNoisePublicKey) - if (!isFavoriteMappedToConnected(fav)) { + val conversationID = ContactDirectory.canonicalConversationId(favPeerID) + if ( + conversationID !in excludedConversationIDs && + !isFavoriteMappedToConnected(fav) + ) { val dn = peerNicknames[favPeerID] ?: fav.peerNickname val (b, _) = splitSuffix(dn) if (b != "You") baseNameCounts[b] = (baseNameCounts[b] ?: 0) + 1 } } - // Nostr-only conversations - val connectedIds = sortedPeers.toSet() - privateChats.keys - .filter { key -> - (key.startsWith("nostr_") || hex64Regex.matches(key)) && - !connectedIds.contains(key) && - !connectedNoiseHexes.contains(key.lowercase()) - } - .forEach { convKey -> - val dn = peerNicknames[convKey] ?: (privateChats[convKey]?.lastOrNull()?.sender ?: convKey.take(12)) - val (b, _) = splitSuffix(dn) - if (b != "You") baseNameCounts[b] = (baseNameCounts[b] ?: 0) + 1 - } - // Every row this card will show, in final order, so the animated list can key on identity // and animate reordering. Offline favourites are appended after the connected peers. // Collected once for the whole card rather than once per row. val directMap by viewModel.peerDirect.collectAsStateWithLifecycle() - val offlineFavoriteRows = offlineFavorites.filterNot { isFavoriteMappedToConnected(it) } + val offlineFavoriteRows = offlineFavorites.filterNot { favorite -> + val favoriteConversationID = ContactDirectory.canonicalConversationId( + ContactIdentityResolver.noiseKeyHex(favorite.peerNoisePublicKey) + ) + favoriteConversationID in excludedConversationIDs || + isFavoriteMappedToConnected(favorite) + } val rowKeys: List = sortedPeers + offlineFavoriteRows.map { ContactIdentityResolver.noiseKeyHex(it.peerNoisePublicKey) } @@ -563,6 +590,139 @@ fun PeopleSection( } } +@Composable +private fun UnreadDirectMessagesSection( + conversations: List, + connectedPeers: List, + viewModel: ChatViewModel, + onPrivateChatStart: (String) -> Unit, + modifier: Modifier = Modifier +) { + val palette = LocalBitchatPalette.current + val colorScheme = MaterialTheme.colorScheme + + Column(modifier = modifier) { + SheetIconSectionHeader( + iconRes = R.drawable.ic_spec_envelope, + title = stringResource(R.string.cd_unread_private_messages) + ) + + Surface( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = AboutHorizontalPadding) + .padding(top = 10.dp), + color = colorScheme.surface, + shape = AboutCardShape + ) { + AnimatedRowColumn( + items = conversations, + key = { it.conversationID } + ) { index, conversation -> + Column { + if (index > 0) SheetCardDivider() + + val resolution = ContactDirectory.resolve(conversation.conversationID) + val connected = resolution.meshPeerID?.let(connectedPeers::contains) == true || + conversation.conversationID in connectedPeers + val aliases = ContactDirectory.aliasesForConversation( + conversation.conversationID + ) + val sourceGeohash = aliases + .asSequence() + .mapNotNull(GeohashConversationRegistry::get) + .firstOrNull() + val displayName = resolution.displayName + ?.takeUnless { it.isBlank() || it.equals("Unknown", ignoreCase = true) } + ?: conversation.displayName + val subtitle = when { + sourceGeohash != null -> "#$sourceGeohash" + conversation.transport == DirectMessageTransport.NOSTR -> + stringResource(R.string.cd_reachable_via_nostr) + !connected -> stringResource(R.string.cd_offline_mesh_chat) + else -> null + } + val peerIdentity = conversation.nostrPubkey + ?.let(viewModel::peerIdentityForNostrPubkey) + ?: viewModel.peerIdentityForMeshPeer(conversation.conversationID) + val assignedColor = colorForPeer(peerIdentity, palette) + val (baseNameRaw, suffix) = splitSuffix(displayName) + + Row( + modifier = Modifier + .fillMaxWidth() + .clickable { + onPrivateChatStart(conversation.conversationID) + } + .padding( + horizontal = SheetRowHorizontal, + vertical = SheetRowVertical + ), + verticalAlignment = Alignment.CenterVertically + ) { + Box( + modifier = Modifier.size(SheetRowLeadingSlot), + contentAlignment = Alignment.Center + ) { + Icon( + painter = painterResource(R.drawable.ic_spec_envelope), + contentDescription = stringResource(R.string.cd_unread_message), + modifier = Modifier.size(PeerRowIconSize), + tint = palette.accentOrange + ) + } + + Spacer(modifier = Modifier.width(SheetRowLeadingGutter)) + + Column(modifier = Modifier.weight(1f)) { + Row( + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(4.dp) + ) { + Text( + text = truncateNickname(baseNameRaw), + fontFamily = BitchatFontFamily, + fontSize = 14.sp, + fontWeight = FontWeight.Medium, + color = assignedColor, + maxLines = 1, + overflow = TextOverflow.Ellipsis + ) + + if (suffix.isNotEmpty()) { + Text( + text = suffix, + fontFamily = BitchatFontFamily, + fontSize = 14.sp, + fontWeight = FontWeight.Medium, + color = assignedColor.copy(alpha = SUFFIX_ALPHA) + ) + } + } + + if (subtitle != null) { + Text( + text = subtitle, + fontFamily = BitchatFontFamily, + fontSize = 11.sp, + color = palette.textTertiary, + maxLines = 1, + overflow = TextOverflow.Ellipsis + ) + } + } + + UnreadBadge( + count = conversation.unreadCount, + colorScheme = colorScheme + ) + } + } + } + } + } +} + @Composable private fun PeerItem( peerID: String, diff --git a/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt b/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt new file mode 100644 index 00000000..bad2c43c --- /dev/null +++ b/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt @@ -0,0 +1,86 @@ +package com.bitchat.android.ui + +import com.bitchat.android.model.BitchatMessage + +internal enum class DirectMessageTransport { + MESH, + NOSTR +} + +/** + * Presence-independent presentation state for an unread private conversation. + * + * A conversation remains in this model until it is read, even when none of its identities are in + * the current mesh or geohash participant lists. + */ +internal data class UnreadConversationSummary( + val conversationID: String, + val displayName: String, + val unreadCount: Int, + val latestMessageAt: Long, + val transport: DirectMessageTransport, + val nostrPubkey: String? +) + +internal fun buildUnreadConversationSummaries( + unreadConversationIDs: Set, + privateChats: Map>, + currentUserIdentifiers: Set, + canonicalize: (String) -> String, + isMessageRead: (BitchatMessage) -> Boolean +): List { + if (unreadConversationIDs.isEmpty()) return emptyList() + + val normalizedCurrentUserIdentifiers = currentUserIdentifiers.filterTo(mutableSetOf()) { + it.isNotBlank() + } + val canonicalUnreadIDs = unreadConversationIDs + .mapTo(linkedSetOf()) { canonicalize(it) } + val unreadAliasesByCanonicalID = unreadConversationIDs.groupBy(canonicalize) + val messagesByCanonicalID = linkedMapOf>() + + privateChats.forEach { (conversationID, messages) -> + val canonicalID = canonicalize(conversationID) + messagesByCanonicalID.getOrPut(canonicalID) { mutableListOf() }.addAll(messages) + } + + return canonicalUnreadIDs.map { conversationID -> + val messages = messagesByCanonicalID[conversationID] + .orEmpty() + .distinctBy { it.id } + val incomingMessages = messages.filterNot { + it.sender in normalizedCurrentUserIdentifiers + } + val unreadIncomingMessages = incomingMessages.filterNot(isMessageRead) + val latestMessage = (unreadIncomingMessages.ifEmpty { incomingMessages }) + .maxWithOrNull(compareBy { it.timestamp.time }.thenBy { it.id }) + val aliases = unreadAliasesByCanonicalID[conversationID].orEmpty() + val nostrPubkey = latestMessage?.senderNostrPubkey + val isNostrConversation = nostrPubkey != null || + aliases.any(::isNostrConversationID) || + isNostrConversationID(conversationID) + + UnreadConversationSummary( + conversationID = conversationID, + displayName = latestMessage + ?.sender + ?.takeIf { it.isNotBlank() } + ?: conversationID.take(12), + unreadCount = unreadIncomingMessages.size.coerceAtLeast(1), + latestMessageAt = latestMessage?.timestamp?.time ?: Long.MIN_VALUE, + transport = if (isNostrConversation) { + DirectMessageTransport.NOSTR + } else { + DirectMessageTransport.MESH + }, + nostrPubkey = nostrPubkey + ) + }.sortedWith( + compareByDescending { it.latestMessageAt } + .thenBy { it.displayName.lowercase() } + .thenBy { it.conversationID } + ) +} + +private fun isNostrConversationID(value: String): Boolean = + value.startsWith("nostr_") || value.startsWith("nostr:") diff --git a/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt new file mode 100644 index 00000000..e054d819 --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt @@ -0,0 +1,117 @@ +package com.bitchat.android.ui + +import com.bitchat.android.model.BitchatMessage +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.util.Date + +class UnreadConversationSummaryTest { + @Test + fun `unread conversations survive missing presence and sort by latest unread`() { + val older = incoming( + id = "older", + sender = "alice", + timestamp = 100 + ) + val newer = incoming( + id = "newer", + sender = "bob", + timestamp = 200 + ) + + val rows = buildUnreadConversationSummaries( + unreadConversationIDs = setOf("alice-peer", "bob-peer"), + privateChats = mapOf( + "alice-peer" to listOf(older), + "bob-peer" to listOf(newer) + ), + currentUserIdentifiers = setOf("me"), + canonicalize = { it }, + isMessageRead = { false } + ) + + assertEquals(listOf("bob-peer", "alice-peer"), rows.map { it.conversationID }) + assertEquals(listOf("bob", "alice"), rows.map { it.displayName }) + } + + @Test + fun `canonical aliases produce one unread conversation row`() { + val message = incoming( + id = "message", + sender = "alice", + timestamp = 100 + ) + + val rows = buildUnreadConversationSummaries( + unreadConversationIDs = setOf("mesh-alias", "nostr_alias"), + privateChats = mapOf( + "mesh-alias" to listOf(message), + "nostr_alias" to listOf(message) + ), + currentUserIdentifiers = setOf("me"), + canonicalize = { "contact_alice" }, + isMessageRead = { false } + ) + + assertEquals(1, rows.size) + assertEquals("contact_alice", rows.single().conversationID) + assertEquals(DirectMessageTransport.NOSTR, rows.single().transport) + } + + @Test + fun `only unseen incoming messages contribute to unread count`() { + val read = incoming( + id = "read", + sender = "alice", + timestamp = 100 + ) + val unread = incoming( + id = "unread", + sender = "alice", + timestamp = 200 + ) + val outgoing = incoming( + id = "outgoing", + sender = "me", + timestamp = 300 + ) + + val row = buildUnreadConversationSummaries( + unreadConversationIDs = setOf("alice-peer"), + privateChats = mapOf("alice-peer" to listOf(read, unread, outgoing)), + currentUserIdentifiers = setOf("me"), + canonicalize = { it }, + isMessageRead = { it.id == "read" } + ).single() + + assertEquals(1, row.unreadCount) + assertEquals(200, row.latestMessageAt) + } + + @Test + fun `unread key without hydrated messages still produces a row`() { + val row = buildUnreadConversationSummaries( + unreadConversationIDs = setOf("orphan-peer"), + privateChats = emptyMap(), + currentUserIdentifiers = setOf("me"), + canonicalize = { it }, + isMessageRead = { false } + ).single() + + assertEquals("orphan-peer", row.conversationID) + assertEquals(1, row.unreadCount) + assertTrue(row.displayName.isNotBlank()) + } + + private fun incoming( + id: String, + sender: String, + timestamp: Long + ) = BitchatMessage( + id = id, + sender = sender, + content = "hello", + timestamp = Date(timestamp) + ) +} From 21cfa62cc2102764957198ea1c1ecc25382ec242 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Mon, 27 Jul 2026 18:23:08 +0200 Subject: [PATCH 2/7] fix: address unread DM review feedback --- .../com/bitchat/android/ui/ChatViewModel.kt | 120 ++++++++++++++---- .../bitchat/android/ui/GeohashPeopleList.kt | 9 +- .../bitchat/android/ui/MeshPeerListSheet.kt | 44 +++---- .../com/bitchat/android/ui/MessageManager.kt | 12 +- .../bitchat/android/ui/PrivateChatManager.kt | 8 +- .../android/ui/UnreadConversationSummary.kt | 23 +++- .../android/ui/PrivateChatManagerTest.kt | 23 ++++ .../ui/UnreadConversationSummaryTest.kt | 20 +++ 8 files changed, 190 insertions(+), 69 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt index 6ef0ff81..c00504fb 100644 --- a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt +++ b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt @@ -6,11 +6,13 @@ import androidx.core.app.NotificationManagerCompat import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope import com.bitchat.android.favorites.FavoritesPersistenceService +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.stateIn import com.bitchat.android.mesh.BluetoothMeshDelegate import com.bitchat.android.mesh.BluetoothMeshService @@ -19,10 +21,12 @@ import com.bitchat.android.service.MeshServiceHolder import com.bitchat.android.model.BitchatMessage import com.bitchat.android.model.BitchatMessageType import com.bitchat.android.nostr.NostrIdentityBridge +import com.bitchat.android.nostr.GeohashConversationRegistry import com.bitchat.android.protocol.BitchatPacket import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import com.bitchat.android.util.NotificationIntervalManager import kotlinx.coroutines.delay import java.util.Date @@ -177,21 +181,54 @@ class ChatViewModel( internal val unreadConversations: StateFlow> = combine( state.unreadPrivateMessages, state.privateChats, - state.nickname - ) { unreadConversationIDs, chats, currentNickname -> + state.nickname, + state.connectedPeers + ) { unreadConversationIDs, chats, currentNickname, connectedPeerIDs -> val seenStore = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) + val connectedPeerIDSet = connectedPeerIDs.mapTo(mutableSetOf()) { it.lowercase() } buildUnreadConversationSummaries( unreadConversationIDs = unreadConversationIDs, privateChats = chats, currentUserIdentifiers = setOf(currentNickname, mesh.myPeerID), canonicalize = ContactDirectory::canonicalConversationId, isMessageRead = { message -> seenStore.hasRead(message.id) } + ).map { summary -> + val resolution = ContactDirectory.resolve(summary.conversationID) + val resolvedNostrPubkey = summary.nostrPubkey + ?: resolution.nostrPubkey?.let(ContactIdentityResolver::nostrPubkeyHex) + val aliases = buildSet { + addAll(summary.identityAliases) + add(summary.conversationID) + add(resolution.conversationID) + resolution.meshPeerID?.let(::add) + resolution.noiseKeyHex?.let(::add) + resolvedNostrPubkey + ?.let(ContactIdentityResolver::nostrAliasForPubkey) + ?.let(::add) + }.mapTo(mutableSetOf()) { it.lowercase() } + + summary.copy( + displayName = resolution.displayName + ?.takeUnless { + it.isBlank() || it.equals("Unknown", ignoreCase = true) + } + ?: summary.displayName, + nostrPubkey = resolvedNostrPubkey, + identityAliases = aliases, + isConnected = aliases.any(connectedPeerIDSet::contains), + sourceGeohash = aliases + .asSequence() + .mapNotNull(GeohashConversationRegistry::get) + .firstOrNull() + ) + } + } + .flowOn(Dispatchers.IO) + .stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = emptyList() ) - }.stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = emptyList() - ) val joinedChannels: StateFlow> = state.joinedChannels val currentChannel: StateFlow = state.currentChannel val channelMessages: StateFlow>> = state.channelMessages @@ -256,18 +293,27 @@ class ChatViewModel( } viewModelScope.launch { try { com.bitchat.android.services.AppStateStore.privateMessages.collect { byPeer -> - val canonicalChats = ContactDirectory.canonicalizePrivateChats(byPeer) + val (canonicalChats, unreadConversationIDs) = withContext(Dispatchers.IO) { + val canonical = ContactDirectory.canonicalizePrivateChats(byPeer) + val unread = try { + val seen = com.bitchat.android.services.SeenMessageStore + .getInstance(getApplication()) + val myNick = state.getNicknameValue().ifBlank { mesh.myPeerID } + canonical + .filterValues { messages -> + messages.any { message -> + message.sender != myNick && !seen.hasRead(message.id) + } + } + .keys + } catch (_: Exception) { + state.getUnreadPrivateMessagesValue() + } + canonical to unread + } state.setPrivateChats(canonicalChats) // Recompute unread set using SeenMessageStore for robustness across Activity recreation - try { - val seen = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) - val myNick = state.getNicknameValue() ?: mesh.myPeerID - val unread = mutableSetOf() - canonicalChats.forEach { (peer, list) -> - if (list.any { msg -> msg.sender != myNick && !seen.hasRead(msg.id) }) unread.add(peer) - } - state.setUnreadPrivateMessages(unread) - } catch (_: Exception) { } + state.setUnreadPrivateMessages(unreadConversationIDs) } } catch (_: Exception) { } } viewModelScope.launch { @@ -397,15 +443,26 @@ class ChatViewModel( // MARK: - Private Chat Management (delegated) - fun startPrivateChat(peerID: String) { + suspend fun startPrivateChat(peerID: String) { // For geohash conversation keys, ensure DM subscription is active if (peerID.startsWith("nostr_")) { ensureGeohashDMSubscriptionIfNeeded(peerID) } - - val success = privateChatManager.startPrivateChat(peerID, mesh) + + val (conversationID, unreadAliases) = withContext(Dispatchers.IO) { + val canonicalID = ContactDirectory.canonicalConversationId(peerID) + canonicalID to matchingUnreadAliases( + unreadConversationIDs = state.getUnreadPrivateMessagesValue(), + canonicalConversationID = canonicalID, + canonicalize = ContactDirectory::canonicalConversationId + ) + } + val success = privateChatManager.startPrivateChat( + peerID = conversationID, + meshService = mesh, + unreadAliases = unreadAliases + ) if (success) { - val conversationID = ContactDirectory.canonicalConversationId(peerID) // Notify notification manager about current private chat setCurrentPrivateChatPeer(conversationID) // Clear notifications for this sender since user is now viewing the chat @@ -413,14 +470,21 @@ class ChatViewModel( // Persistently mark all messages in this conversation as read so Nostr fetches // after app restarts won't re-mark them as unread. - try { - val seen = com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) - val chats = state.getPrivateChatsValue() - val messages = chats[conversationID] ?: emptyList() - messages.forEach { msg -> - try { seen.markRead(msg.id) } catch (_: Exception) { } + withContext(Dispatchers.IO) { + try { + val seen = + com.bitchat.android.services.SeenMessageStore.getInstance(getApplication()) + val chats = state.getPrivateChatsValue() + val messages = chats[conversationID] ?: emptyList() + messages.forEach { msg -> + try { + seen.markRead(msg.id) + } catch (_: Exception) { + } + } + } catch (_: Exception) { } - } catch (_: Exception) { } + } } } diff --git a/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt b/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt index e8f87668..d784610a 100644 --- a/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt +++ b/app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt @@ -19,7 +19,6 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.bitchat.android.ui.theme.BitchatFontFamily import com.bitchat.android.ui.theme.colorForPeer import com.bitchat.android.R -import com.bitchat.android.services.ContactDirectory import com.bitchat.android.ui.theme.LocalBitchatPalette import java.util.* @@ -38,7 +37,7 @@ fun GeohashPeopleList( viewModel: ChatViewModel, onTapPerson: () -> Unit, modifier: Modifier = Modifier, - excludedConversationIDs: Set = emptySet() + excludedIdentityAliases: Set = emptySet() ) { val geohashPeople by viewModel.geohashPeople.collectAsStateWithLifecycle() val selectedLocationChannel by viewModel.selectedLocationChannel.collectAsStateWithLifecycle() @@ -79,10 +78,10 @@ fun GeohashPeopleList( geohashPeople } } - val visiblePeople = remember(peopleIncludingSelf, excludedConversationIDs) { + val visiblePeople = remember(peopleIncludingSelf, excludedIdentityAliases) { peopleIncludingSelf.filterNot { person -> - val alias = "nostr_${person.id.take(16)}" - ContactDirectory.canonicalConversationId(alias) in excludedConversationIDs + val alias = "nostr_${person.id.take(16)}".lowercase() + alias in excludedIdentityAliases } } val sections = remember(visiblePeople, myHex, isTeleported, teleportedGeo) { diff --git a/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt b/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt index 7ec2fc0a..a5777ccf 100644 --- a/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt +++ b/app/src/main/java/com/bitchat/android/ui/MeshPeerListSheet.kt @@ -86,11 +86,12 @@ fun MeshPeerListSheet( val geohashPeopleCount = geohashPeople.size val wifiAwareConnected by com.bitchat.android.wifiaware.WifiAwareController.connectedPeers.collectAsStateWithLifecycle() val wifiAwarePeerIDs = remember(wifiAwareConnected) { wifiAwareConnected.keys.toSet() } - val unreadConversationIDs = remember(unreadConversations) { - unreadConversations.mapTo(mutableSetOf()) { it.conversationID } + val unreadIdentityAliases = remember(unreadConversations) { + unreadConversations + .flatMapTo(mutableSetOf()) { it.identityAliases } } val visibleConnectedPeers = connectedPeers.filterNot { peerID -> - ContactDirectory.canonicalConversationId(peerID) in unreadConversationIDs + peerID.lowercase() in unreadIdentityAliases } // Bottom sheet state @@ -131,7 +132,6 @@ fun MeshPeerListSheet( item(key = "unread_private_messages_section") { UnreadDirectMessagesSection( conversations = unreadConversations, - connectedPeers = connectedPeers, viewModel = viewModel, onPrivateChatStart = { conversationID -> viewModel.showPrivateChatSheet(conversationID) @@ -203,7 +203,7 @@ fun MeshPeerListSheet( GeohashPeopleList( viewModel = viewModel, onTapPerson = onDismiss, - excludedConversationIDs = unreadConversationIDs, + excludedIdentityAliases = unreadIdentityAliases, modifier = Modifier.padding( top = if ( joinedChannels.isNotEmpty() || @@ -229,7 +229,7 @@ fun MeshPeerListSheet( selectedPrivatePeer = selectedPrivatePeer, wifiAwarePeerIDs = wifiAwarePeerIDs, peopleCount = peopleCount, - excludedConversationIDs = unreadConversationIDs, + excludedIdentityAliases = unreadIdentityAliases, viewModel = viewModel, onPrivateChatStart = { peerID -> viewModel.showPrivateChatSheet(peerID) @@ -344,7 +344,7 @@ fun PeopleSection( selectedPrivatePeer: String?, wifiAwarePeerIDs: Set = emptySet(), peopleCount: Int = 0, - excludedConversationIDs: Set = emptySet(), + excludedIdentityAliases: Set = emptySet(), viewModel: ChatViewModel, onPrivateChatStart: (String) -> Unit ) { @@ -461,9 +461,8 @@ fun PeopleSection( val offlineFavorites = FavoritesPersistenceService.shared.getOurFavorites() offlineFavorites.forEach { fav -> val favPeerID = ContactIdentityResolver.noiseKeyHex(fav.peerNoisePublicKey) - val conversationID = ContactDirectory.canonicalConversationId(favPeerID) if ( - conversationID !in excludedConversationIDs && + favPeerID.lowercase() !in excludedIdentityAliases && !isFavoriteMappedToConnected(fav) ) { val dn = peerNicknames[favPeerID] ?: fav.peerNickname @@ -478,10 +477,10 @@ fun PeopleSection( val directMap by viewModel.peerDirect.collectAsStateWithLifecycle() val offlineFavoriteRows = offlineFavorites.filterNot { favorite -> - val favoriteConversationID = ContactDirectory.canonicalConversationId( - ContactIdentityResolver.noiseKeyHex(favorite.peerNoisePublicKey) + val favoriteNoiseKey = ContactIdentityResolver.noiseKeyHex( + favorite.peerNoisePublicKey ) - favoriteConversationID in excludedConversationIDs || + favoriteNoiseKey.lowercase() in excludedIdentityAliases || isFavoriteMappedToConnected(favorite) } val rowKeys: List = sortedPeers + @@ -593,7 +592,6 @@ fun PeopleSection( @Composable private fun UnreadDirectMessagesSection( conversations: List, - connectedPeers: List, viewModel: ChatViewModel, onPrivateChatStart: (String) -> Unit, modifier: Modifier = Modifier @@ -622,31 +620,19 @@ private fun UnreadDirectMessagesSection( Column { if (index > 0) SheetCardDivider() - val resolution = ContactDirectory.resolve(conversation.conversationID) - val connected = resolution.meshPeerID?.let(connectedPeers::contains) == true || - conversation.conversationID in connectedPeers - val aliases = ContactDirectory.aliasesForConversation( - conversation.conversationID - ) - val sourceGeohash = aliases - .asSequence() - .mapNotNull(GeohashConversationRegistry::get) - .firstOrNull() - val displayName = resolution.displayName - ?.takeUnless { it.isBlank() || it.equals("Unknown", ignoreCase = true) } - ?: conversation.displayName val subtitle = when { - sourceGeohash != null -> "#$sourceGeohash" + conversation.sourceGeohash != null -> "#${conversation.sourceGeohash}" conversation.transport == DirectMessageTransport.NOSTR -> stringResource(R.string.cd_reachable_via_nostr) - !connected -> stringResource(R.string.cd_offline_mesh_chat) + !conversation.isConnected -> + stringResource(R.string.cd_offline_mesh_chat) else -> null } val peerIdentity = conversation.nostrPubkey ?.let(viewModel::peerIdentityForNostrPubkey) ?: viewModel.peerIdentityForMeshPeer(conversation.conversationID) val assignedColor = colorForPeer(peerIdentity, palette) - val (baseNameRaw, suffix) = splitSuffix(displayName) + val (baseNameRaw, suffix) = splitSuffix(conversation.displayName) Row( modifier = Modifier diff --git a/app/src/main/java/com/bitchat/android/ui/MessageManager.kt b/app/src/main/java/com/bitchat/android/ui/MessageManager.kt index e93ef39a..0c7b64b2 100644 --- a/app/src/main/java/com/bitchat/android/ui/MessageManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/MessageManager.kt @@ -153,11 +153,17 @@ class MessageManager(private val state: ChatState) { state.setPrivateChats(updatedChats) } - fun clearPrivateUnreadMessages(peerID: String) { + fun clearPrivateUnreadMessages( + peerID: String, + aliases: Set = emptySet() + ) { val conversationID = ContactDirectory.canonicalConversationId(peerID) val updatedUnread = state.getUnreadPrivateMessagesValue().toMutableSet() - updatedUnread.remove(peerID) - updatedUnread.remove(conversationID) + val normalizedAliases = (aliases + peerID + conversationID) + .mapTo(mutableSetOf()) { it.lowercase() } + updatedUnread.removeAll { unreadID -> + unreadID.lowercase() in normalizedAliases + } state.setUnreadPrivateMessages(updatedUnread) } diff --git a/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt b/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt index 1af48a04..8d7c5e1d 100644 --- a/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/PrivateChatManager.kt @@ -47,7 +47,11 @@ class PrivateChatManager( // MARK: - Private Chat Lifecycle - fun startPrivateChat(peerID: String, meshService: MeshService): Boolean { + fun startPrivateChat( + peerID: String, + meshService: MeshService, + unreadAliases: Set = emptySet() + ): Boolean { val conversationID = ContactDirectory.canonicalConversationId(peerID) val route = ContactDirectory.resolve(conversationID) val meshPeerID = route.meshPeerID ?: peerID.takeIf { ContactIdentityResolver.isMeshPeerId(it) } @@ -75,7 +79,7 @@ class PrivateChatManager( state.setSelectedPrivateChatPeer(conversationID) // Clear unread - messageManager.clearPrivateUnreadMessages(conversationID) + messageManager.clearPrivateUnreadMessages(conversationID, unreadAliases) // Initialize chat if needed messageManager.initializePrivateChat(conversationID) diff --git a/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt b/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt index bad2c43c..5d0ad81a 100644 --- a/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt +++ b/app/src/main/java/com/bitchat/android/ui/UnreadConversationSummary.kt @@ -19,7 +19,10 @@ internal data class UnreadConversationSummary( val unreadCount: Int, val latestMessageAt: Long, val transport: DirectMessageTransport, - val nostrPubkey: String? + val nostrPubkey: String?, + val identityAliases: Set, + val isConnected: Boolean = false, + val sourceGeohash: String? = null ) internal fun buildUnreadConversationSummaries( @@ -73,7 +76,9 @@ internal fun buildUnreadConversationSummaries( } else { DirectMessageTransport.MESH }, - nostrPubkey = nostrPubkey + nostrPubkey = nostrPubkey, + identityAliases = (aliases + conversationID) + .mapTo(mutableSetOf()) { it.lowercase() } ) }.sortedWith( compareByDescending { it.latestMessageAt } @@ -84,3 +89,17 @@ internal fun buildUnreadConversationSummaries( private fun isNostrConversationID(value: String): Boolean = value.startsWith("nostr_") || value.startsWith("nostr:") + +internal fun matchingUnreadAliases( + unreadConversationIDs: Set, + canonicalConversationID: String, + canonicalize: (String) -> String +): Set { + val normalizedCanonicalID = canonicalConversationID.lowercase() + return unreadConversationIDs + .filterTo(mutableSetOf()) { unreadID -> + canonicalize(unreadID).equals(normalizedCanonicalID, ignoreCase = true) + } + .plus(canonicalConversationID) + .mapTo(mutableSetOf()) { it.lowercase() } +} diff --git a/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt index 852411e8..8434cab1 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/PrivateChatManagerTest.kt @@ -104,4 +104,27 @@ class PrivateChatManagerTest { verify(meshService).sendReadReceipt(message.id, meshPeerID, "bob") } + + @Test + fun `opening canonical unread conversation clears all source aliases`() { + val canonicalID = "contact_alice" + val nostrAlias = "nostr_0123456789abcdef" + val meshAlias = "0123456789abcdef" + val unrelatedConversation = "other-contact" + val meshService = mock() + state.setUnreadPrivateMessages( + setOf(canonicalID, nostrAlias, meshAlias, unrelatedConversation) + ) + + manager.startPrivateChat( + peerID = canonicalID, + meshService = meshService, + unreadAliases = setOf(canonicalID, nostrAlias, meshAlias) + ) + + assertEquals( + setOf(unrelatedConversation), + state.getUnreadPrivateMessagesValue() + ) + } } diff --git a/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt index e054d819..2b5f4613 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/UnreadConversationSummaryTest.kt @@ -57,6 +57,10 @@ class UnreadConversationSummaryTest { assertEquals(1, rows.size) assertEquals("contact_alice", rows.single().conversationID) assertEquals(DirectMessageTransport.NOSTR, rows.single().transport) + assertEquals( + setOf("mesh-alias", "nostr_alias", "contact_alice"), + rows.single().identityAliases + ) } @Test @@ -104,6 +108,22 @@ class UnreadConversationSummaryTest { assertTrue(row.displayName.isNotBlank()) } + @Test + fun `canonical unread lookup returns every matching source alias`() { + val aliases = matchingUnreadAliases( + unreadConversationIDs = setOf("mesh-alias", "nostr_alias", "other-contact"), + canonicalConversationID = "contact_alice", + canonicalize = { unreadID -> + if (unreadID == "other-contact") unreadID else "contact_alice" + } + ) + + assertEquals( + setOf("mesh-alias", "nostr_alias", "contact_alice"), + aliases + ) + } + private fun incoming( id: String, sender: String, From 719b3d1895ac400bd16321089f44b5d57c5f76ae Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:06:46 +0200 Subject: [PATCH 3/7] ui: mute theme-aware peer colors for contrast Extract PeerColorStyle so each palette owns saturation/value, keeping hues stable while dark mode stays bright-but-muted and light mode avoids neon labels. New themes only need to supply their own style. Co-authored-by: Cursor --- .../android/ui/theme/BitchatPalette.kt | 15 ++++----- .../bitchat/android/ui/theme/PeerColors.kt | 32 +++++++++++++++++-- .../com/bitchat/android/ui/ChatUIUtilsTest.kt | 21 ++++++++---- 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/ui/theme/BitchatPalette.kt b/app/src/main/java/com/bitchat/android/ui/theme/BitchatPalette.kt index 96ef57d4..8dfb87b0 100644 --- a/app/src/main/java/com/bitchat/android/ui/theme/BitchatPalette.kt +++ b/app/src/main/java/com/bitchat/android/ui/theme/BitchatPalette.kt @@ -41,10 +41,11 @@ data class BitchatPalette( val accentPurple: Color, // MARK: - Deterministic peer colors - /** Chroma applied after deriving a peer's stable hue. */ - val peerColorSaturation: Float, - /** Brightness applied after deriving a peer's stable hue. */ - val peerColorValue: Float, + /** + * Saturation/value applied after deriving a peer's stable hue. Swap this when adding a + * new theme — see [PeerColorStyle] for contrast guidelines. + */ + val peerColors: PeerColorStyle, ) val DarkBitchatPalette = BitchatPalette( @@ -56,8 +57,7 @@ val DarkBitchatPalette = BitchatPalette( textTertiary = Color(0xFF6B776B), accentOrange = Color(0xFFFF9F0A), accentPurple = Color(0xFFBF5AF2), - peerColorSaturation = 1f, - peerColorValue = 1f, + peerColors = PeerColorStyle.Dark, ) val LightBitchatPalette = BitchatPalette( @@ -69,8 +69,7 @@ val LightBitchatPalette = BitchatPalette( textTertiary = Color(0xFF757F75), accentOrange = Color(0xFFFF9500), accentPurple = Color(0xFFAF52DE), - peerColorSaturation = 0.85f, - peerColorValue = 0.45f, + peerColors = PeerColorStyle.Light, ) val LocalBitchatPalette = staticCompositionLocalOf { DarkBitchatPalette } diff --git a/app/src/main/java/com/bitchat/android/ui/theme/PeerColors.kt b/app/src/main/java/com/bitchat/android/ui/theme/PeerColors.kt index b0f1ed7d..168c9863 100644 --- a/app/src/main/java/com/bitchat/android/ui/theme/PeerColors.kt +++ b/app/src/main/java/com/bitchat/android/ui/theme/PeerColors.kt @@ -1,9 +1,36 @@ package com.bitchat.android.ui.theme +import androidx.compose.runtime.Immutable import androidx.compose.ui.graphics.Color import com.bitchat.android.ui.PeerIdentity import kotlin.math.abs +/** + * Theme-specific chroma applied after a peer's stable hue is derived. + * + * Hue stays identity-stable across themes (and byte-identical to iOS). Only saturation + * and value change so peer labels remain readable on each background. + * + * Guidelines when adding a future theme: + * - Dim / dark backgrounds: keep [value] high so colors are not lost against the surface; + * prefer muted [saturation] over neon. + * - Light backgrounds: keep [value] moderate-low so colors are not blinding; avoid + * near-full saturation. + */ +@Immutable +data class PeerColorStyle( + val saturation: Float, + val value: Float, +) { + companion object { + /** Soft pastels that stay bright enough on near-black chat surfaces. */ + val Dark = PeerColorStyle(saturation = 0.55f, value = 0.82f) + + /** Deeper, less saturated tones that stay readable on near-white surfaces. */ + val Light = PeerColorStyle(saturation = 0.70f, value = 0.42f) + } +} + /** * The single identity-to-color boundary used by chat, people sheets, and mentions. * @@ -22,9 +49,10 @@ fun colorForPeer(identity: PeerIdentity, palette: BitchatPalette): Color { hue = (hue + 0.12) % 1.0 } + val style = palette.peerColors return Color.hsv( hue = (hue * 360).toFloat(), - saturation = palette.peerColorSaturation, - value = palette.peerColorValue + saturation = style.saturation, + value = style.value ) } diff --git a/app/src/test/java/com/bitchat/android/ui/ChatUIUtilsTest.kt b/app/src/test/java/com/bitchat/android/ui/ChatUIUtilsTest.kt index e64448c3..69693b7e 100644 --- a/app/src/test/java/com/bitchat/android/ui/ChatUIUtilsTest.kt +++ b/app/src/test/java/com/bitchat/android/ui/ChatUIUtilsTest.kt @@ -14,6 +14,7 @@ import com.bitchat.android.ui.theme.LightBitchatColorScheme import com.bitchat.android.ui.theme.LightBitchatPalette import com.bitchat.android.ui.theme.MessageBodyTextStyle import com.bitchat.android.ui.theme.MessageSenderTextStyle +import com.bitchat.android.ui.theme.PeerColorStyle import com.bitchat.android.ui.theme.colorForPeer import java.text.SimpleDateFormat import java.util.Date @@ -444,8 +445,8 @@ class ChatUIUtilsTest { @Test fun `peer color hue is stable across light and dark, only chroma differs`() { - // Hue derivation must stay byte-identical to iOS; only saturation/value are tuned for - // the redesigned neutral message body. + // Hue derivation must stay byte-identical to iOS; only saturation/value are tuned per + // theme so dark mode stays muted-but-bright and light mode stays deep-but-readable. val identity = PeerIdentity.mesh("abc") val dark = colorForPeer(identity, DarkBitchatPalette) val light = colorForPeer(identity, LightBitchatPalette) @@ -456,10 +457,16 @@ class ChatUIUtilsTest { rgbToHsv(light.red, light.green, light.blue, lightHsv) assertEquals(darkHsv[0].toDouble(), lightHsv[0].toDouble(), 1.0) - assertEquals(1.0, darkHsv[1].toDouble(), 0.01) - assertEquals(1.0, darkHsv[2].toDouble(), 0.01) - assertEquals(0.85, lightHsv[1].toDouble(), 0.01) - assertEquals(0.45, lightHsv[2].toDouble(), 0.01) + assertEquals(PeerColorStyle.Dark.saturation.toDouble(), darkHsv[1].toDouble(), 0.01) + assertEquals(PeerColorStyle.Dark.value.toDouble(), darkHsv[2].toDouble(), 0.01) + assertEquals(PeerColorStyle.Light.saturation.toDouble(), lightHsv[1].toDouble(), 0.01) + assertEquals(PeerColorStyle.Light.value.toDouble(), lightHsv[2].toDouble(), 0.01) + // Dark theme: muted chroma, never dark (readable on near-black). + assertTrue(darkHsv[1] < 0.75f) + assertTrue(darkHsv[2] >= 0.75f) + // Light theme: avoid neon / near-white peer labels. + assertTrue(lightHsv[1] < 0.85f) + assertTrue(lightHsv[2] <= 0.55f) } @Test @@ -485,7 +492,7 @@ class ChatUIUtilsTest { assertEquals(Color(0xFFF5F5F5), DarkBitchatColorScheme.onSurface) assertTrue(LightBitchatColorScheme.onSurface != DarkBitchatColorScheme.onSurface) assertTrue( - LightBitchatPalette.peerColorValue != DarkBitchatPalette.peerColorValue + LightBitchatPalette.peerColors != DarkBitchatPalette.peerColors ) } From e9bdf8aefc1fe83f14474789994b18f9966e3ce2 Mon Sep 17 00:00:00 2001 From: jack Date: Sun, 12 Jul 2026 10:29:30 -0400 Subject: [PATCH 4/7] Bound pre-auth compressed payload expansion --- .../android/protocol/BinaryProtocol.kt | 56 ++- .../android/protocol/CompressionUtil.kt | 121 ++++-- .../android/protocol/BinaryProtocolTest.kt | 359 ++++++++++++++++++ docs/file_transfer.md | 20 + 4 files changed, 509 insertions(+), 47 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt b/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt index a952d5fa..6b1b5325 100644 --- a/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt +++ b/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt @@ -324,21 +324,36 @@ object BinaryProtocol { } } - fun decode(data: ByteArray): BitchatPacket? { + fun decode(data: ByteArray): BitchatPacket? = + decode(data, CompressionUtil::decompress) + + /** Test seam used to prove rejected expansion sizes never reach inflation. */ + internal fun decodeForTesting( + data: ByteArray, + decompress: (ByteArray, Int) -> ByteArray? + ): BitchatPacket? = decode(data, decompress) + + private fun decode( + data: ByteArray, + decompress: (ByteArray, Int) -> ByteArray? + ): BitchatPacket? { // Try decode as-is first (robust when padding wasn't applied) - iOS fix - decodeCore(data)?.let { return it } + decodeCore(data, decompress)?.let { return it } // If that fails, try after removing padding val unpadded = MessagePadding.unpad(data) if (unpadded.contentEquals(data)) return null // No padding was removed, already failed - return decodeCore(unpadded) + return decodeCore(unpadded, decompress) } /** * Core decoding implementation used by decode() with and without padding removal - iOS fix */ - private fun decodeCore(raw: ByteArray): BitchatPacket? { + private fun decodeCore( + raw: ByteArray, + decompress: (ByteArray, Int) -> ByteArray? + ): BitchatPacket? { try { if (raw.size < HEADER_SIZE_V1 + SENDER_ID_SIZE) return null @@ -435,23 +450,42 @@ object BinaryProtocol { } else { buffer.getShort().toUShort().toInt() } + + val maxExpandedSize = com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH + if (originalSize <= 0 || originalSize > maxExpandedSize) { + Log.w( + "BinaryProtocol", + "Expanded payload size $originalSize is outside the allowed range 1..$maxExpandedSize" + ) + return null + } // Compressed payload val compressedSize = payloadLength.toInt() - lengthFieldBytes + if (compressedSize == 0) { + Log.w("BinaryProtocol", "Compressed payload has no deflate bytes") + return null + } val compressedPayload = ByteArray(compressedSize) buffer.get(compressedPayload) // Security check: Compression bomb protection - if (compressedSize > 0) { - val ratio = originalSize.toDouble() / compressedSize.toDouble() - if (ratio > 50_000.0) { - Log.w("BinaryProtocol", "🚫 Suspicious compression ratio: ${ratio}:1") - return null - } + val ratio = originalSize.toDouble() / compressedSize.toDouble() + if (ratio > 50_000.0) { + Log.w("BinaryProtocol", "🚫 Suspicious compression ratio: ${ratio}:1") + return null } // Decompress - CompressionUtil.decompress(compressedPayload, originalSize) ?: return null + val expandedPayload = decompress(compressedPayload, originalSize) ?: return null + if (expandedPayload.size != originalSize) { + Log.w( + "BinaryProtocol", + "Expanded payload size ${expandedPayload.size} did not match declared size $originalSize" + ) + return null + } + expandedPayload } else { val payloadBytes = ByteArray(payloadLength.toInt()) buffer.get(payloadBytes) diff --git a/app/src/main/java/com/bitchat/android/protocol/CompressionUtil.kt b/app/src/main/java/com/bitchat/android/protocol/CompressionUtil.kt index e8b59254..34e583fb 100644 --- a/app/src/main/java/com/bitchat/android/protocol/CompressionUtil.kt +++ b/app/src/main/java/com/bitchat/android/protocol/CompressionUtil.kt @@ -2,6 +2,7 @@ package com.bitchat.android.protocol import android.util.Log import java.io.ByteArrayOutputStream +import java.util.zip.DataFormatException import java.util.zip.Deflater import java.util.zip.Inflater @@ -11,6 +12,10 @@ import java.util.zip.Inflater */ object CompressionUtil { private const val COMPRESSION_THRESHOLD = com.bitchat.android.util.AppConstants.Protocol.COMPRESSION_THRESHOLD_BYTES // bytes - same as iOS + + // Inflation allocates the full declared output buffer. Keep that allocation single-flight so + // concurrent packets cannot multiply the bounded per-packet memory cost. + private val decompressionLock = Any() /** * Helper to check if compression is worth it - exact same logic as iOS @@ -73,49 +78,93 @@ object CompressionUtil { * iOS COMPRESSION_ZLIB produces raw deflate data (no headers) */ fun decompress(compressedData: ByteArray, originalSize: Int): ByteArray? { - // iOS COMPRESSION_ZLIB produces raw deflate format (no headers) - try { - val inflater = Inflater(true) // true = raw deflate, no headers - inflater.setInput(compressedData) - - val decompressedBuffer = ByteArray(originalSize) - val actualSize = inflater.inflate(decompressedBuffer) - inflater.end() - - // Verify decompressed size matches expected (same validation as iOS) - return if (actualSize == originalSize) { - decompressedBuffer - } else if (actualSize > 0) { - // Handle case where actual size is different - decompressedBuffer.copyOfRange(0, actualSize) - } else { + val maxExpandedSize = com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH + if (compressedData.isEmpty()) { + Log.w("CompressionUtil", "Refusing an empty compressed payload") + return null + } + if (originalSize <= 0 || originalSize > maxExpandedSize) { + Log.w( + "CompressionUtil", + "Refusing expanded payload size $originalSize outside 1..$maxExpandedSize" + ) + return null + } + + return synchronized(decompressionLock) { + // iOS COMPRESSION_ZLIB produces raw deflate format (no headers). + val rawResult = try { + inflateExact(compressedData, originalSize, nowrap = true) + } catch (e: Exception) { + Log.d( + "CompressionUtil", + "Raw deflate decompression failed: ${e.message}" + ) null } - } catch (e: Exception) { - Log.d("CompressionUtil", "Raw deflate decompression failed: ${e.message}, trying with zlib headers...") - - // Fallback: try with zlib headers in case of mixed usage - try { - val inflater = Inflater(false) // false = expect zlib headers - inflater.setInput(compressedData) - - val decompressedBuffer = ByteArray(originalSize) - val actualSize = inflater.inflate(decompressedBuffer) - inflater.end() - - return if (actualSize == originalSize) { - decompressedBuffer - } else if (actualSize > 0) { - decompressedBuffer.copyOfRange(0, actualSize) - } else { + + if (rawResult != null) { + rawResult + } else { + // Fallback after either a format error or an incomplete/wrong-sized raw stream: + // accept the zlib-wrapped form used by some older/mixed clients, but only when it + // independently satisfies the same exact-size and complete-stream checks. + try { + inflateExact(compressedData, originalSize, nowrap = false) + } catch (fallbackException: Exception) { + Log.e( + "CompressionUtil", + "Both raw deflate and zlib decompression failed: ${fallbackException.message}" + ) null } - } catch (fallbackException: Exception) { - Log.e("CompressionUtil", "Both raw deflate and zlib decompression failed: ${fallbackException.message}") - return null } } } + + /** + * Inflate one complete stream into exactly [originalSize] bytes. + * + * A full output buffer alone is not success: an attacker can under-declare a larger stream so + * the first inflate call fills the buffer while [Inflater.finished] remains false. Conversely, + * a truncated or over-declared stream can produce a non-empty prefix. Both forms are rejected, + * as are trailing bytes after the compressed stream. + * + * [DataFormatException] is deliberately allowed to escape so the caller can try the legacy + * zlib-wrapped format. Size/completion mismatches return null; the fallback must then prove the + * same bytes are a complete, exact-sized zlib stream before they can be accepted. + */ + @Throws(DataFormatException::class) + private fun inflateExact( + compressedData: ByteArray, + originalSize: Int, + nowrap: Boolean + ): ByteArray? { + val inflater = Inflater(nowrap) + return try { + inflater.setInput(compressedData) + val output = ByteArray(originalSize) + var written = 0 + + while (written < originalSize) { + val count = inflater.inflate(output, written, originalSize - written) + if (count == 0) break + written += count + } + + if (written != originalSize) return null + + // Give Inflater one byte of room to consume the end marker. Any produced byte proves + // the declared size was smaller than the actual expansion. + val overflowProbe = ByteArray(1) + if (inflater.inflate(overflowProbe) != 0) return null + + if (!inflater.finished() || inflater.remaining != 0) return null + output + } finally { + inflater.end() + } + } /** * Test function to verify deflate compression works correctly diff --git a/app/src/test/java/com/bitchat/android/protocol/BinaryProtocolTest.kt b/app/src/test/java/com/bitchat/android/protocol/BinaryProtocolTest.kt index c1b2327b..fb6e03bb 100644 --- a/app/src/test/java/com/bitchat/android/protocol/BinaryProtocolTest.kt +++ b/app/src/test/java/com/bitchat/android/protocol/BinaryProtocolTest.kt @@ -1,5 +1,6 @@ package com.bitchat.android.protocol +import com.bitchat.android.model.BitchatFilePacket import org.junit.Assert.assertEquals import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertFalse @@ -7,9 +8,11 @@ import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test +import java.io.ByteArrayOutputStream import java.nio.ByteBuffer import java.nio.ByteOrder import java.util.Random +import java.util.zip.Deflater class BinaryProtocolTest { @@ -987,6 +990,309 @@ class BinaryProtocolTest { assertNull("v2 compression bomb (ratio > 50,000:1) must be rejected", result) } + @Test + fun `v2 expanded payload at exact maximum passes bound without allocating output`() { + val max = com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH + val raw = compressedPacket( + version = 2u, + originalSize = max, + compressedData = ByteArray(256) { it.toByte() } + ) + var calls = 0 + + val result = BinaryProtocol.decodeForTesting(raw) { _, requestedSize -> + calls += 1 + assertEquals(max, requestedSize) + null // Prove the boundary reached this seam without allocating a 10 MiB result. + } + + assertNull("The test decompressor deliberately returns no payload", result) + assertEquals(1, calls) + } + + @Test + fun `v2 expanded payload above maximum never reaches decompressor`() { + val max = com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH + val raw = compressedPacket( + version = 2u, + originalSize = max + 1, + compressedData = ByteArray(256) { it.toByte() } + ) + var calls = 0 + + val result = BinaryProtocol.decodeForTesting(raw) { _, _ -> + calls += 1 + byteArrayOf(0x42) + } + + assertNull("An oversized expansion must be rejected before inflation", result) + assertEquals("The decompressor must not be invoked", 0, calls) + } + + @Test + fun `v2 negative expanded payload never reaches decompressor`() { + val raw = compressedPacket( + version = 2u, + originalSize = -1, + compressedData = byteArrayOf(0x03) + ) + var calls = 0 + + val result = BinaryProtocol.decodeForTesting(raw) { _, _ -> + calls += 1 + byteArrayOf(0x42) + } + + assertNull("A negative expansion must be rejected before inflation", result) + assertEquals("The decompressor must not be invoked", 0, calls) + } + + @Test + fun `zero expanded payload never reaches decompressor`() { + val raw = compressedPacket( + version = 2u, + originalSize = 0, + compressedData = rawDeflate(ByteArray(0)) + ) + var calls = 0 + + val result = BinaryProtocol.decodeForTesting(raw) { _, _ -> + calls += 1 + ByteArray(0) + } + + assertNull("Compressed zero-length payloads are non-canonical and must be rejected", result) + assertEquals("The decompressor must not be invoked", 0, calls) + } + + @Test + fun `empty compressed body never reaches decompressor`() { + val raw = compressedPacket( + version = 2u, + originalSize = 128, + compressedData = ByteArray(0) + ) + var calls = 0 + + val result = BinaryProtocol.decodeForTesting(raw) { _, _ -> + calls += 1 + ByteArray(128) + } + + assertNull("A compressed payload must contain deflate bytes", result) + assertEquals("The decompressor must not be invoked", 0, calls) + } + + @Test + fun `v1 unsigned maximum expanded size reaches decompressor`() { + val originalSize = 0xFFFF + val raw = compressedPacket( + version = 1u, + originalSize = originalSize, + compressedData = byteArrayOf(0x01, 0x02) + ) + var calls = 0 + + val result = BinaryProtocol.decodeForTesting(raw) { _, requestedSize -> + calls += 1 + assertEquals(originalSize, requestedSize) + null + } + + assertNull("The test decompressor deliberately returns no payload", result) + assertEquals(1, calls) + } + + @Test + fun `compression utility rejects invalid expansion sizes directly`() { + val max = com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH + + assertNull(CompressionUtil.decompress(ByteArray(0), 1)) + assertNull(CompressionUtil.decompress(rawDeflate(ByteArray(0)), 0)) + assertNull(CompressionUtil.decompress(byteArrayOf(0x03), -1)) + assertNull(CompressionUtil.decompress(byteArrayOf(0x03), max + 1)) + } + + @Test + fun `raw deflate expands only when size and stream completion are exact`() { + val payload = ByteArray(4_096) { index -> (index % 17).toByte() } + val compressed = rawDeflate(payload) + + val decoded = BinaryProtocol.decode( + compressedPacket(version = 2u, originalSize = payload.size, compressedData = compressed) + ) + + assertNotNull(decoded) + assertArrayEquals(payload, decoded!!.payload) + } + + @Test + fun `zlib wrapped payload remains compatible when size and stream completion are exact`() { + val payload = ByteArray(4_096) { index -> (index % 23).toByte() } + val compressed = zlibDeflate(payload) + + val decoded = BinaryProtocol.decode( + compressedPacket(version = 2u, originalSize = payload.size, compressedData = compressed) + ) + + assertNotNull(decoded) + assertArrayEquals(payload, decoded!!.payload) + } + + @Test + fun `under-declared zlib expansion is rejected by fallback`() { + val payload = ByteArray(4_096) { 0x51 } + val compressed = zlibDeflate(payload) + + val decoded = BinaryProtocol.decode( + compressedPacket(version = 2u, originalSize = 128, compressedData = compressed) + ) + + assertNull("Zlib fallback must reject output beyond the declaration", decoded) + } + + @Test + fun `over-declared zlib expansion is rejected by fallback`() { + val payload = ByteArray(128) { 0x52 } + val compressed = zlibDeflate(payload) + + val decoded = BinaryProtocol.decode( + compressedPacket(version = 2u, originalSize = 256, compressedData = compressed) + ) + + assertNull("Zlib fallback must reject output shorter than the declaration", decoded) + } + + @Test + fun `truncated zlib stream is rejected by fallback`() { + val payload = ByteArray(4_096) { index -> (index % 29).toByte() } + val compressed = zlibDeflate(payload) + val truncated = compressed.copyOf(compressed.size - 1) + + val decoded = BinaryProtocol.decode( + compressedPacket(version = 2u, originalSize = payload.size, compressedData = truncated) + ) + + assertNull("Zlib fallback must require the stream end marker and checksum", decoded) + } + + @Test + fun `zlib stream with trailing bytes is rejected by fallback`() { + val payload = ByteArray(4_096) { index -> (index % 13).toByte() } + val compressedWithTrailingByte = zlibDeflate(payload) + byteArrayOf(0x00) + + val decoded = BinaryProtocol.decode( + compressedPacket( + version = 2u, + originalSize = payload.size, + compressedData = compressedWithTrailingByte + ) + ) + + assertNull("Zlib fallback must consume the complete input and nothing more", decoded) + } + + @Test + fun `under-declared raw expansion is rejected even when output buffer fills`() { + val payload = ByteArray(4_096) { 0x41 } + val compressed = rawDeflate(payload) + + val decoded = BinaryProtocol.decode( + compressedPacket(version = 2u, originalSize = 128, compressedData = compressed) + ) + + assertNull("Inflater must be finished, not merely fill the declared buffer", decoded) + } + + @Test + fun `over-declared raw expansion is rejected instead of returning a prefix`() { + val payload = ByteArray(128) { 0x42 } + val compressed = rawDeflate(payload) + + val decoded = BinaryProtocol.decode( + compressedPacket(version = 2u, originalSize = 256, compressedData = compressed) + ) + + assertNull("The expanded byte count must equal the declaration", decoded) + } + + @Test + fun `truncated raw stream is rejected even if all declared bytes were emitted`() { + val payload = ByteArray(4_096) { index -> (index % 31).toByte() } + val compressed = rawDeflate(payload) + val truncated = compressed.copyOf(compressed.size - 1) + + val decoded = BinaryProtocol.decode( + compressedPacket(version = 2u, originalSize = payload.size, compressedData = truncated) + ) + + assertNull("A stream without its end marker must not be accepted", decoded) + } + + @Test + fun `raw stream with trailing bytes is rejected`() { + val payload = ByteArray(4_096) { index -> (index % 19).toByte() } + val compressedWithTrailingByte = rawDeflate(payload) + byteArrayOf(0x00) + + val decoded = BinaryProtocol.decode( + compressedPacket( + version = 2u, + originalSize = payload.size, + compressedData = compressedWithTrailingByte + ) + ) + + assertNull("Trailing bytes after a complete stream must not be accepted", decoded) + } + + @Test + fun `decoder rejects a decompressor result shorter than its declaration`() { + val raw = compressedPacket( + version = 2u, + originalSize = 128, + compressedData = ByteArray(16) { it.toByte() } + ) + + val decoded = BinaryProtocol.decodeForTesting(raw) { _, _ -> byteArrayOf(0x01) } + + assertNull("BinaryProtocol must independently enforce the declared expanded size", decoded) + } + + @Test + fun `legacy 11 MiB public file transfer is explicitly rejected by bounded decoder`() { + val content = ByteArray(11 * 1024 * 1024) { 0x41 } + val filePayload = BitchatFilePacket( + fileName = "legacy-11m.bin", + fileSize = content.size.toLong(), + mimeType = "application/octet-stream", + content = content + ).encode() + assertNotNull(filePayload) + + val encoded = BinaryProtocol.encode( + BitchatPacket( + version = 2u, + type = MessageType.FILE_TRANSFER.value, + senderID = hexToBytes(senderHex), + recipientID = SpecialRecipients.BROADCAST, + timestamp = fixedTimestamp, + payload = filePayload!!, + ttl = 5u + ), + padding = false + ) + assertNotNull("A legacy sender can produce the highly-compressible wire packet", encoded) + assertTrue( + "The compressed wire body remains below the normal 10 MiB input cap", + encoded!!.size < com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH + ) + + assertNull( + "The uniform bound intentionally rejects this legacy expansion until streaming admission exists", + BinaryProtocol.decode(encoded) + ) + } + /** * Compression bomb is rejected * @@ -1163,6 +1469,59 @@ class BinaryProtocolTest { return result } + private fun compressedPacket( + version: UByte, + originalSize: Int, + compressedData: ByteArray, + type: UByte = MessageType.MESSAGE.value + ): ByteArray { + val originalSizeFieldBytes = if (version >= 2u.toUByte()) 4 else 2 + val payloadLength = originalSizeFieldBytes + compressedData.size + val headerSize = if (version >= 2u.toUByte()) 16 else 14 + val buffer = ByteBuffer.allocate(headerSize + 8 + payloadLength).apply { + order(ByteOrder.BIG_ENDIAN) + put(version.toByte()) + put(type.toByte()) + put(5.toByte()) + putLong(fixedTimestamp.toLong()) + put(BinaryProtocol.Flags.IS_COMPRESSED.toByte()) + if (version >= 2u.toUByte()) { + putInt(payloadLength) + } else { + putShort(payloadLength.toShort()) + } + put(hexToBytes(senderHex)) + if (version >= 2u.toUByte()) { + putInt(originalSize) + } else { + putShort(originalSize.toShort()) + } + put(compressedData) + } + return buffer.array() + } + + private fun rawDeflate(data: ByteArray): ByteArray = deflate(data, nowrap = true) + + private fun zlibDeflate(data: ByteArray): ByteArray = deflate(data, nowrap = false) + + private fun deflate(data: ByteArray, nowrap: Boolean): ByteArray { + val deflater = Deflater(Deflater.DEFAULT_COMPRESSION, nowrap) + return try { + deflater.setInput(data) + deflater.finish() + val output = ByteArrayOutputStream() + val buffer = ByteArray(1_024) + while (!deflater.finished()) { + val count = deflater.deflate(buffer) + output.write(buffer, 0, count) + } + output.toByteArray() + } finally { + deflater.end() + } + } + private fun makePacket( version: UByte = 1u, type: UByte = MessageType.MESSAGE.value, diff --git a/docs/file_transfer.md b/docs/file_transfer.md index 333c4d5a..3f44cb2c 100644 --- a/docs/file_transfer.md +++ b/docs/file_transfer.md @@ -107,6 +107,26 @@ source-route metadata. It does not imply multi-gigabyte mesh transfer support. transport threshold; the data portion is at most 469 bytes and becomes smaller when recipient or source-route overhead is present. +#### Compressed expansion rollout gate (draft/HOLD) + +Android's bounded decoder applies the same 10 MiB expanded-payload ceiling to every outer +message type. It also requires a non-empty compressed body, an exact declared output size, and a +complete deflate stream. The `FILE_TRANSFER (0x22)` byte cannot safely grant a larger ceiling: it is +attacker-controlled before packet signature verification, and the current receive pipeline must +inflate before it can perform that verification. + +This intentionally means a legacy Android sender can produce a highly compressible public file +between 10 MiB and the UI's 50 MiB send limit that the bounded decoder rejects. The hardening must +therefore remain a rollout HOLD rather than silently ship as backward compatible. Grandfathering +50 MiB also is not safe after only a type check: inflation allocates the declared payload and +`BitchatFilePacket.decode` currently copies the content again, creating a greater than 100 MiB peak +for a maximum-size transfer. + +Before enabling that legacy range, receive processing needs an authenticated admission decision +made before large allocation plus streaming inflation/TLV parsing into a bounded temporary file (or +another ownership-preserving design that avoids the second full-size copy). The sender limit and a +wire capability/version transition must then be coordinated so old and new clients fail predictably. + ### 1.3 File Transfer TLV payload (BitchatFilePacket) The file payload is a TLV structure with mixed length field sizes to support large contents efficiently. From b251812b9e57bd82ab8e25752bb4cb57f4f67fa6 Mon Sep 17 00:00:00 2001 From: a1denvalu3 <43107113+a1denvalu3@users.noreply.github.com> Date: Sun, 26 Jul 2026 11:52:55 +0200 Subject: [PATCH 5/7] Align compressed payload send and receive bounds (#736) * Align compressed payload send and receive bounds * Preserve ambiguous raw deflate compatibility * Pool decompression by memory budget --- .../android/protocol/BinaryProtocol.kt | 25 ++-- .../android/protocol/CompressionUtil.kt | 83 +++++++++---- .../protocol/DecompressionResourcePool.kt | 73 +++++++++++ .../bitchat/android/ui/MediaSendingManager.kt | 2 +- .../com/bitchat/android/util/AppConstants.kt | 4 +- .../android/protocol/BinaryProtocolTest.kt | 24 ++-- .../protocol/DecompressionResourcePoolTest.kt | 117 ++++++++++++++++++ .../kotlin/com/bitchat/FileTransferTest.kt | 2 +- docs/file_transfer.md | 9 +- 9 files changed, 290 insertions(+), 49 deletions(-) create mode 100644 app/src/main/java/com/bitchat/android/protocol/DecompressionResourcePool.kt create mode 100644 app/src/test/java/com/bitchat/android/protocol/DecompressionResourcePoolTest.kt diff --git a/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt b/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt index 6b1b5325..5b1d7287 100644 --- a/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt +++ b/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt @@ -183,7 +183,6 @@ object BinaryProtocol { private const val SENDER_ID_SIZE = 8 private const val RECIPIENT_ID_SIZE = 8 private const val SIGNATURE_SIZE = 64 - object Flags { const val HAS_RECIPIENT: UByte = 0x01u const val HAS_SIGNATURE: UByte = 0x02u @@ -200,6 +199,15 @@ object BinaryProtocol { fun encode(packet: BitchatPacket, padding: Boolean = true): ByteArray? { try { + val maxPayloadLength = com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH + if (packet.payload.size > maxPayloadLength) { + Log.w( + "BinaryProtocol", + "Cannot encode payload ${packet.payload.size} above receiver limit $maxPayloadLength" + ) + return null + } + // Try to compress payload if beneficial var payload = packet.payload var originalPayloadSize: Int? = null @@ -325,7 +333,7 @@ object BinaryProtocol { } fun decode(data: ByteArray): BitchatPacket? = - decode(data, CompressionUtil::decompress) + decode(data, CompressionUtil::decompressWithResourcesReserved) /** Test seam used to prove rejected expansion sizes never reach inflation. */ internal fun decodeForTesting( @@ -466,9 +474,6 @@ object BinaryProtocol { Log.w("BinaryProtocol", "Compressed payload has no deflate bytes") return null } - val compressedPayload = ByteArray(compressedSize) - buffer.get(compressedPayload) - // Security check: Compression bomb protection val ratio = originalSize.toDouble() / compressedSize.toDouble() if (ratio > 50_000.0) { @@ -476,8 +481,14 @@ object BinaryProtocol { return null } - // Decompress - val expandedPayload = decompress(compressedPayload, originalSize) ?: return null + // Reserve the compressed copy plus expanded output before either allocation. + // Small packets share the memory pool; packets wait only while its budget is full. + val resourceBytes = compressedSize.toLong() + originalSize.toLong() + val expandedPayload = CompressionUtil.withDecompressionResources(resourceBytes) { + val compressedPayload = ByteArray(compressedSize) + buffer.get(compressedPayload) + decompress(compressedPayload, originalSize) + } ?: return null if (expandedPayload.size != originalSize) { Log.w( "BinaryProtocol", diff --git a/app/src/main/java/com/bitchat/android/protocol/CompressionUtil.kt b/app/src/main/java/com/bitchat/android/protocol/CompressionUtil.kt index 34e583fb..5f8e9d5a 100644 --- a/app/src/main/java/com/bitchat/android/protocol/CompressionUtil.kt +++ b/app/src/main/java/com/bitchat/android/protocol/CompressionUtil.kt @@ -13,9 +13,7 @@ import java.util.zip.Inflater object CompressionUtil { private const val COMPRESSION_THRESHOLD = com.bitchat.android.util.AppConstants.Protocol.COMPRESSION_THRESHOLD_BYTES // bytes - same as iOS - // Inflation allocates the full declared output buffer. Keep that allocation single-flight so - // concurrent packets cannot multiply the bounded per-packet memory cost. - private val decompressionLock = Any() + private val decompressionPool = DecompressionResourcePool.forRuntime() /** * Helper to check if compression is worth it - exact same logic as iOS @@ -78,50 +76,83 @@ object CompressionUtil { * iOS COMPRESSION_ZLIB produces raw deflate data (no headers) */ fun decompress(compressedData: ByteArray, originalSize: Int): ByteArray? { + if (!isValidRequest(compressedData, originalSize)) return null + return withDecompressionResources(originalSize.toLong()) { + decompressWithResourcesReserved(compressedData, originalSize) + } + } + + internal fun withDecompressionResources(bytes: Long, block: () -> T): T? = + decompressionPool.withReservation(bytes, block) + + /** + * Inflate after the caller has reserved all packet-specific allocations. + * This avoids nested acquisition when BinaryProtocol reserves both its input copy and output. + */ + internal fun decompressWithResourcesReserved( + compressedData: ByteArray, + originalSize: Int + ): ByteArray? { + if (!isValidRequest(compressedData, originalSize)) return null + return decompressExact(compressedData, originalSize) + } + + private fun isValidRequest(compressedData: ByteArray, originalSize: Int): Boolean { val maxExpandedSize = com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH if (compressedData.isEmpty()) { Log.w("CompressionUtil", "Refusing an empty compressed payload") - return null + return false } if (originalSize <= 0 || originalSize > maxExpandedSize) { Log.w( "CompressionUtil", "Refusing expanded payload size $originalSize outside 1..$maxExpandedSize" ) - return null + return false } + return true + } - return synchronized(decompressionLock) { - // iOS COMPRESSION_ZLIB produces raw deflate format (no headers). - val rawResult = try { - inflateExact(compressedData, originalSize, nowrap = true) - } catch (e: Exception) { - Log.d( - "CompressionUtil", - "Raw deflate decompression failed: ${e.message}" - ) + private fun decompressExact(compressedData: ByteArray, originalSize: Int): ByteArray? { + return if (looksLikeZlib(compressedData)) { + // A raw stream can coincidentally begin with a valid-looking zlib header. The + // header therefore only determines which format to try first; any non-exact zlib + // result must still fall back to raw under the same size/completion bounds. + val zlibResult = try { + inflateExact(compressedData, originalSize, nowrap = false) + } catch (zlibException: DataFormatException) { null } - - if (rawResult != null) { - rawResult + if (zlibResult != null) { + zlibResult } else { - // Fallback after either a format error or an incomplete/wrong-sized raw stream: - // accept the zlib-wrapped form used by some older/mixed clients, but only when it - // independently satisfies the same exact-size and complete-stream checks. try { - inflateExact(compressedData, originalSize, nowrap = false) - } catch (fallbackException: Exception) { - Log.e( - "CompressionUtil", - "Both raw deflate and zlib decompression failed: ${fallbackException.message}" - ) + inflateExact(compressedData, originalSize, nowrap = true) + } catch (rawException: DataFormatException) { + Log.d("CompressionUtil", "Invalid zlib/raw deflate stream") null } } + } else { + try { + inflateExact(compressedData, originalSize, nowrap = true) + } catch (rawException: DataFormatException) { + Log.d("CompressionUtil", "Invalid raw deflate stream") + null + } } } + /** RFC 1950 header check used to avoid speculative double inflation. */ + private fun looksLikeZlib(data: ByteArray): Boolean { + if (data.size < 2) return false + val cmf = data[0].toInt() and 0xFF + val flg = data[1].toInt() and 0xFF + return (cmf and 0x0F) == 8 && + (cmf ushr 4) <= 7 && + ((cmf shl 8) or flg) % 31 == 0 + } + /** * Inflate one complete stream into exactly [originalSize] bytes. * diff --git a/app/src/main/java/com/bitchat/android/protocol/DecompressionResourcePool.kt b/app/src/main/java/com/bitchat/android/protocol/DecompressionResourcePool.kt new file mode 100644 index 00000000..a72c8070 --- /dev/null +++ b/app/src/main/java/com/bitchat/android/protocol/DecompressionResourcePool.kt @@ -0,0 +1,73 @@ +package com.bitchat.android.protocol + +import java.util.concurrent.Semaphore +import java.util.concurrent.TimeUnit +import kotlin.math.ceil + +/** + * Fair, weighted admission control for decompression allocations. + * + * Permits represent memory rather than workers: small packets can proceed concurrently while + * near-limit packets consume most of the budget. Callers must reserve before allocating any + * packet-specific compressed copy or expanded output. + */ +internal class DecompressionResourcePool( + budgetBytes: Long, + private val unitBytes: Int, + private val waitTimeoutMs: Long +) { + private val totalPermits = (budgetBytes / unitBytes).toInt().coerceAtLeast(1) + private val permits = Semaphore(totalPermits, true) + + fun withReservation(bytes: Long, block: () -> T): T? { + val requiredPermits = permitsFor(bytes) + val acquired = try { + permits.tryAcquire(requiredPermits, waitTimeoutMs, TimeUnit.MILLISECONDS) + } catch (_: InterruptedException) { + Thread.currentThread().interrupt() + false + } + if (!acquired) return null + + return try { + block() + } finally { + permits.release(requiredPermits) + } + } + + internal fun permitsFor(bytes: Long): Int = + ceil(bytes.coerceAtLeast(1).toDouble() / unitBytes.toDouble()) + .toInt() + .coerceAtMost(totalPermits) + + internal val availablePermits: Int + get() = permits.availablePermits() + + companion object { + private const val DEFAULT_UNIT_BYTES = 256 * 1024 + private const val DEFAULT_WAIT_TIMEOUT_MS = 1_000L + private const val HEAP_BUDGET_DIVISOR = 8L + private const val MAX_BUDGET_BYTES = 64L * 1024 * 1024 + + fun forRuntime( + maxHeapBytes: Long = Runtime.getRuntime().maxMemory(), + maxPacketResourceBytes: Long = + 2L * com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH + ): DecompressionResourcePool { + val budget = recommendedBudgetBytes(maxHeapBytes, maxPacketResourceBytes) + return DecompressionResourcePool( + budgetBytes = budget, + unitBytes = DEFAULT_UNIT_BYTES, + waitTimeoutMs = DEFAULT_WAIT_TIMEOUT_MS + ) + } + + internal fun recommendedBudgetBytes( + maxHeapBytes: Long, + maxPacketResourceBytes: Long + ): Long = (maxHeapBytes / HEAP_BUDGET_DIVISOR) + .coerceAtLeast(maxPacketResourceBytes) + .coerceAtMost(MAX_BUDGET_BYTES.coerceAtLeast(maxPacketResourceBytes)) + } +} diff --git a/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt b/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt index ec8c064a..0b8f2526 100644 --- a/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt @@ -43,7 +43,7 @@ class MediaSendingManager( get() = getMeshService() companion object { private const val TAG = "MediaSendingManager" - private const val MAX_FILE_SIZE = com.bitchat.android.util.AppConstants.Media.MAX_FILE_SIZE_BYTES // 50MB limit + private const val MAX_FILE_SIZE = com.bitchat.android.util.AppConstants.Media.MAX_FILE_SIZE_BYTES private const val PENDING_PRIVATE_MEDIA_TIMEOUT_MS = 15_000L } diff --git a/app/src/main/java/com/bitchat/android/util/AppConstants.kt b/app/src/main/java/com/bitchat/android/util/AppConstants.kt index 4dec775f..7d115b4a 100644 --- a/app/src/main/java/com/bitchat/android/util/AppConstants.kt +++ b/app/src/main/java/com/bitchat/android/util/AppConstants.kt @@ -131,7 +131,9 @@ object AppConstants { } object Media { - const val MAX_FILE_SIZE_BYTES: Long = 50L * 1024 * 1024 + // A file is currently encoded into one protocol payload before BLE fragmentation. + // Reserve room for maximum filename/MIME TLVs and encryption envelope overhead. + const val MAX_FILE_SIZE_BYTES: Long = (10L * 1024 * 1024) - (132L * 1024) } object Services { diff --git a/app/src/test/java/com/bitchat/android/protocol/BinaryProtocolTest.kt b/app/src/test/java/com/bitchat/android/protocol/BinaryProtocolTest.kt index fb6e03bb..f6a1042d 100644 --- a/app/src/test/java/com/bitchat/android/protocol/BinaryProtocolTest.kt +++ b/app/src/test/java/com/bitchat/android/protocol/BinaryProtocolTest.kt @@ -1139,6 +1139,18 @@ class BinaryProtocolTest { assertArrayEquals(payload, decoded!!.payload) } + @Test + fun `raw deflate with zlib-looking prefix falls back after non-exact zlib parse`() { + val payload = ByteArray(29) { index -> (index + 1).toByte() } + val compressed = byteArrayOf( + 0x08, // non-final raw stored block; also zlib CMF + 0x1d, 0x00, // LEN = 29; 0x08 0x1d passes the RFC 1950 header check + 0xe2.toByte(), 0xff.toByte() // one's complement of LEN + ) + payload + byteArrayOf(0x03, 0x00) // final empty fixed-Huffman block + + assertArrayEquals(payload, CompressionUtil.decompress(compressed, payload.size)) + } + @Test fun `under-declared zlib expansion is rejected by fallback`() { val payload = ByteArray(4_096) { 0x51 } @@ -1259,7 +1271,7 @@ class BinaryProtocolTest { } @Test - fun `legacy 11 MiB public file transfer is explicitly rejected by bounded decoder`() { + fun `new sender refuses legacy 11 MiB public file transfer before transmission`() { val content = ByteArray(11 * 1024 * 1024) { 0x41 } val filePayload = BitchatFilePacket( fileName = "legacy-11m.bin", @@ -1281,15 +1293,9 @@ class BinaryProtocolTest { ), padding = false ) - assertNotNull("A legacy sender can produce the highly-compressible wire packet", encoded) - assertTrue( - "The compressed wire body remains below the normal 10 MiB input cap", - encoded!!.size < com.bitchat.android.util.AppConstants.Protocol.MAX_PAYLOAD_LENGTH - ) - assertNull( - "The uniform bound intentionally rejects this legacy expansion until streaming admission exists", - BinaryProtocol.decode(encoded) + "Sender and receiver must enforce the same expanded-payload ceiling", + encoded ) } diff --git a/app/src/test/java/com/bitchat/android/protocol/DecompressionResourcePoolTest.kt b/app/src/test/java/com/bitchat/android/protocol/DecompressionResourcePoolTest.kt new file mode 100644 index 00000000..843d88a2 --- /dev/null +++ b/app/src/test/java/com/bitchat/android/protocol/DecompressionResourcePoolTest.kt @@ -0,0 +1,117 @@ +package com.bitchat.android.protocol + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit + +class DecompressionResourcePoolTest { + @Test + fun `small reservations run concurrently and next waits only when budget is full`() { + val pool = DecompressionResourcePool( + budgetBytes = 2_000, + unitBytes = 1_000, + waitTimeoutMs = 2_000 + ) + val executor = Executors.newFixedThreadPool(3) + val entered = CountDownLatch(2) + val release = CountDownLatch(1) + val thirdEntered = CountDownLatch(1) + + try { + repeat(2) { + executor.submit { + pool.withReservation(1_000) { + entered.countDown() + release.await() + } + } + } + assertTrue(entered.await(1, TimeUnit.SECONDS)) + + executor.submit { + pool.withReservation(1_000) { + thirdEntered.countDown() + } + } + assertFalse("third reservation must wait while budget is full", thirdEntered.await(100, TimeUnit.MILLISECONDS)) + + release.countDown() + assertTrue("third reservation must proceed after release", thirdEntered.await(1, TimeUnit.SECONDS)) + } finally { + release.countDown() + executor.shutdownNow() + } + } + + @Test + fun `timed admission drops work instead of waiting indefinitely`() { + val pool = DecompressionResourcePool( + budgetBytes = 1_000, + unitBytes = 1_000, + waitTimeoutMs = 50 + ) + val entered = CountDownLatch(1) + val release = CountDownLatch(1) + val executor = Executors.newSingleThreadExecutor() + + try { + executor.submit { + pool.withReservation(1_000) { + entered.countDown() + release.await() + } + } + assertTrue(entered.await(1, TimeUnit.SECONDS)) + assertNull(pool.withReservation(1_000) { "unexpected" }) + } finally { + release.countDown() + executor.shutdownNow() + } + } + + @Test + fun `permits are released when decode throws`() { + val pool = DecompressionResourcePool(2_000, 1_000, 50) + + try { + pool.withReservation(2_000) { error("boom") } + } catch (_: IllegalStateException) { + // Expected. + } + + assertEquals(2, pool.availablePermits) + assertEquals("ok", pool.withReservation(2_000) { "ok" }) + } + + @Test + fun `runtime budget is based on heap memory and always admits one maximum packet`() { + val maxPacketResources = 20L * 1024 * 1024 + + assertEquals( + maxPacketResources, + DecompressionResourcePool.recommendedBudgetBytes( + maxHeapBytes = 64L * 1024 * 1024, + maxPacketResourceBytes = maxPacketResources + ) + ) + assertEquals( + 32L * 1024 * 1024, + DecompressionResourcePool.recommendedBudgetBytes( + maxHeapBytes = 256L * 1024 * 1024, + maxPacketResourceBytes = maxPacketResources + ) + ) + assertEquals( + 64L * 1024 * 1024, + DecompressionResourcePool.recommendedBudgetBytes( + maxHeapBytes = 2L * 1024 * 1024 * 1024, + maxPacketResourceBytes = maxPacketResources + ) + ) + } +} diff --git a/app/src/test/kotlin/com/bitchat/FileTransferTest.kt b/app/src/test/kotlin/com/bitchat/FileTransferTest.kt index 31131540..8a136e69 100644 --- a/app/src/test/kotlin/com/bitchat/FileTransferTest.kt +++ b/app/src/test/kotlin/com/bitchat/FileTransferTest.kt @@ -237,7 +237,7 @@ class FileTransferTest { // Given: Large file size (simulated) val largeFileSize = 100L * 1024 * 1024 // 100MB - val maxAllowedSize = 50L * 1024 * 1024 // 50MB + val maxAllowedSize = com.bitchat.android.util.AppConstants.Media.MAX_FILE_SIZE_BYTES // When: Checking if file can be transferred val isAllowed = largeFileSize <= maxAllowedSize diff --git a/docs/file_transfer.md b/docs/file_transfer.md index 3f44cb2c..0d0274d4 100644 --- a/docs/file_transfer.md +++ b/docs/file_transfer.md @@ -115,10 +115,11 @@ complete deflate stream. The `FILE_TRANSFER (0x22)` byte cannot safely grant a l attacker-controlled before packet signature verification, and the current receive pipeline must inflate before it can perform that verification. -This intentionally means a legacy Android sender can produce a highly compressible public file -between 10 MiB and the UI's 50 MiB send limit that the bounded decoder rejects. The hardening must -therefore remain a rollout HOLD rather than silently ship as backward compatible. Grandfathering -50 MiB also is not safe after only a type check: inflation allocates the declared payload and +New Android senders cap files just below 10 MiB (reserving envelope overhead) and refuse to encode +any payload above the receiver ceiling. +Legacy Android senders can still produce a highly compressible public file between 10 MiB and their +50 MiB UI limit that the bounded decoder rejects. Grandfathering 50 MiB is not safe after only a type +check: inflation allocates the declared payload and `BitchatFilePacket.decode` currently copies the content again, creating a greater than 100 MiB peak for a maximum-size transfer. From c3bc911bcddb0c40ecb2ccc8d03571656173260a Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:35:51 +0200 Subject: [PATCH 6/7] Finish rollout: user-visible size-cap failure, OOM-safe decode catch - MediaSendingManager: surface a chat system message when a picked file exceeds the ~10 MiB send cap instead of silently dropping the send (voice/image/file paths), completing the 'user-visible failure' requirement of the rollout gate - BinaryProtocol: catch Exception instead of Throwable in decodeCore so OutOfMemoryError is never swallowed and masked as a parse failure - docs/file_transfer.md: mark the compressed-expansion rollout gate resolved; support for legacy >10 MiB compressed transfers is explicitly ended --- .../android/protocol/BinaryProtocol.kt | 2 +- .../bitchat/android/ui/MediaSendingManager.kt | 25 ++++++++++++++----- docs/file_transfer.md | 11 ++++---- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt b/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt index 5b1d7287..47ee8d75 100644 --- a/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt +++ b/app/src/main/java/com/bitchat/android/protocol/BinaryProtocol.kt @@ -522,7 +522,7 @@ object BinaryProtocol { route = route ) - } catch (e: Throwable) { + } catch (e: Exception) { Log.e("BinaryProtocol", "Error decoding packet: ${e.message}") return null } diff --git a/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt b/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt index 0b8f2526..41431cd1 100644 --- a/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt @@ -81,6 +81,22 @@ class MediaSendingManager( private var automaticRetryRequestedFor: String? = null private var pendingAutomaticTimeoutRequestId: String? = null + /** + * Enforce the send-size cap with a user-visible failure. + * Returns true if the file is oversized and the send was aborted. + */ + private fun rejectIfOversized(file: java.io.File): Boolean { + val size = file.length() + if (size <= MAX_FILE_SIZE) return false + Log.e(TAG, "❌ File too large: $size bytes (max: $MAX_FILE_SIZE)") + val sizeMb = size / (1024 * 1024) + val maxMb = MAX_FILE_SIZE / (1024 * 1024) + messageManager.addSystemMessage( + "cannot send ${file.name}: file is too large (${sizeMb} MB, max $maxMb MB)" + ) + return true + } + /** * Send a voice note (audio file) */ @@ -103,8 +119,7 @@ class MediaSendingManager( return@withContext null } - if (file.length() > MAX_FILE_SIZE) { - Log.e(TAG, "File too large: ${file.length()} bytes (max: $MAX_FILE_SIZE)") + if (rejectIfOversized(file)) { return@withContext null } @@ -148,8 +163,7 @@ class MediaSendingManager( return@withContext null } - if (file.length() > MAX_FILE_SIZE) { - Log.e(TAG, "File too large: ${file.length()} bytes (max: $MAX_FILE_SIZE)") + if (rejectIfOversized(file)) { return@withContext null } @@ -193,8 +207,7 @@ class MediaSendingManager( return@withContext null } - if (file.length() > MAX_FILE_SIZE) { - Log.e(TAG, "File too large: ${file.length()} bytes (max: $MAX_FILE_SIZE)") + if (rejectIfOversized(file)) { return@withContext null } diff --git a/docs/file_transfer.md b/docs/file_transfer.md index 0d0274d4..8422b77a 100644 --- a/docs/file_transfer.md +++ b/docs/file_transfer.md @@ -107,7 +107,7 @@ source-route metadata. It does not imply multi-gigabyte mesh transfer support. transport threshold; the data portion is at most 469 bytes and becomes smaller when recipient or source-route overhead is present. -#### Compressed expansion rollout gate (draft/HOLD) +#### Compressed expansion rollout gate (resolved) Android's bounded decoder applies the same 10 MiB expanded-payload ceiling to every outer message type. It also requires a non-empty compressed body, an exact declared output size, and a @@ -116,10 +116,11 @@ attacker-controlled before packet signature verification, and the current receiv inflate before it can perform that verification. New Android senders cap files just below 10 MiB (reserving envelope overhead) and refuse to encode -any payload above the receiver ceiling. -Legacy Android senders can still produce a highly compressible public file between 10 MiB and their -50 MiB UI limit that the bounded decoder rejects. Grandfathering 50 MiB is not safe after only a type -check: inflation allocates the declared payload and +any payload above the receiver ceiling; exceeding the cap surfaces a user-visible error in chat. +Support for legacy >10 MiB compressed transfers is explicitly ended: legacy Android senders can +still produce a highly compressible public file between 10 MiB and their 50 MiB UI limit that the +bounded decoder rejects. Grandfathering 50 MiB is not safe after only a type check: inflation +allocates the declared payload and `BitchatFilePacket.decode` currently copies the content again, creating a greater than 100 MiB peak for a maximum-size transfer. From 0721c39f89415fb3e4935b01cc720906aaf7d018 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:24:02 +0200 Subject: [PATCH 7/7] Route oversize send failure to the active conversation Addresses review feedback: the size-cap error was posted to the main mesh timeline, so a user sending from a private chat or channel never saw it. rejectIfOversized now posts to the private conversation (addPrivateMessageNoUnread) or channel (addChannelMessage) the send originated from, falling back to the main timeline for public sends. Adds regression tests for both routings. --- .../bitchat/android/ui/MediaSendingManager.kt | 42 +++++++++++++++---- .../ui/MediaSendingManagerMigrationTest.kt | 37 ++++++++++++++++ 2 files changed, 70 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt b/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt index 41431cd1..68797e08 100644 --- a/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt +++ b/app/src/main/java/com/bitchat/android/ui/MediaSendingManager.kt @@ -82,18 +82,42 @@ class MediaSendingManager( private var pendingAutomaticTimeoutRequestId: String? = null /** - * Enforce the send-size cap with a user-visible failure. - * Returns true if the file is oversized and the send was aborted. + * Enforce the send-size cap with a user-visible failure posted to the + * conversation the user is sending from. Returns true if the file is + * oversized and the send was aborted. */ - private fun rejectIfOversized(file: java.io.File): Boolean { + private fun rejectIfOversized( + file: java.io.File, + toPeerIDOrNull: String?, + channelOrNull: String? + ): Boolean { val size = file.length() if (size <= MAX_FILE_SIZE) return false Log.e(TAG, "❌ File too large: $size bytes (max: $MAX_FILE_SIZE)") val sizeMb = size / (1024 * 1024) val maxMb = MAX_FILE_SIZE / (1024 * 1024) - messageManager.addSystemMessage( - "cannot send ${file.name}: file is too large (${sizeMb} MB, max $maxMb MB)" - ) + val text = "cannot send ${file.name}: file is too large (${sizeMb} MB, max $maxMb MB)" + when { + toPeerIDOrNull != null -> { + val sys = BitchatMessage( + sender = "system", + content = text, + timestamp = Date(), + isRelay = false + ) + messageManager.addPrivateMessageNoUnread(toPeerIDOrNull, sys) + } + channelOrNull != null -> { + val sys = BitchatMessage( + sender = "system", + content = text, + timestamp = Date(), + isRelay = false + ) + messageManager.addChannelMessage(channelOrNull, sys) + } + else -> messageManager.addSystemMessage(text) + } return true } @@ -119,7 +143,7 @@ class MediaSendingManager( return@withContext null } - if (rejectIfOversized(file)) { + if (rejectIfOversized(file, toPeerIDOrNull, channelOrNull)) { return@withContext null } @@ -163,7 +187,7 @@ class MediaSendingManager( return@withContext null } - if (rejectIfOversized(file)) { + if (rejectIfOversized(file, toPeerIDOrNull, channelOrNull)) { return@withContext null } @@ -207,7 +231,7 @@ class MediaSendingManager( return@withContext null } - if (rejectIfOversized(file)) { + if (rejectIfOversized(file, toPeerIDOrNull, channelOrNull)) { return@withContext null } diff --git a/app/src/test/kotlin/com/bitchat/android/ui/MediaSendingManagerMigrationTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/MediaSendingManagerMigrationTest.kt index ebe5f744..60c57198 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/MediaSendingManagerMigrationTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/MediaSendingManagerMigrationTest.kt @@ -287,6 +287,43 @@ class MediaSendingManagerMigrationTest { assertTrue(messages.none { it.type == com.bitchat.android.model.BitchatMessageType.Image }) } + @Test + fun `oversized file failure is posted to the private conversation and nothing is sent`() { + val bigFile = kotlin.io.path.createTempFile("oversized-private", ".jpg").toFile() + try { + bigFile.writeBytes(ByteArray(11 * 1024 * 1024) { 0x42 }) + + manager.sendImageNote(peerID, null, bigFile.absolutePath) + + val messages = state.privateChats.value[peerID].orEmpty() + assertEquals(1, messages.size) + assertTrue(messages.single().content.contains("too large")) + assertTrue(messages.none { it.type == com.bitchat.android.model.BitchatMessageType.Image }) + assertTrue(state.getMessagesValue().none { it.content.contains("too large") }) + verify(mesh, never()).prepareFilePrivate(any(), any(), any(), any()) + } finally { + bigFile.delete() + } + } + + @Test + fun `oversized file failure is posted to the channel and nothing is sent`() { + val bigFile = kotlin.io.path.createTempFile("oversized-channel", ".jpg").toFile() + try { + bigFile.writeBytes(ByteArray(11 * 1024 * 1024) { 0x42 }) + + manager.sendImageNote(null, "#test", bigFile.absolutePath) + + val channelMessages = state.getChannelMessagesValue()["#test"].orEmpty() + assertEquals(1, channelMessages.size) + assertTrue(channelMessages.single().content.contains("too large")) + assertTrue(state.getMessagesValue().none { it.content.contains("too large") }) + verify(mesh, never()).prepareFilePrivate(any(), any(), any(), any()) + } finally { + bigFile.delete() + } + } + @Test fun `cancelled consent cannot later send or echo`() { whenever(mesh.prepareFilePrivate(eq(peerID), any(), any(), eq(false)))