diff --git a/README.md b/README.md index 601ab5a161..5583fad0b5 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ SimpleX logo -Invest in SimpleX Chat. [Register now](https://simplexchat.typeform.com/crowdfunding). +Invest in SimpleX Chat. [Learn more on Wefunder](https://wefunder.com/simplexchat). # SimpleX - the first messaging platform that has no user identifiers of any kind - 100% private by design! diff --git a/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/views/database/DatabaseEncryptionView.android.kt b/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/views/database/DatabaseEncryptionView.android.kt index 83677f3318..1826b2114e 100644 --- a/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/views/database/DatabaseEncryptionView.android.kt +++ b/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/views/database/DatabaseEncryptionView.android.kt @@ -42,10 +42,9 @@ actual fun SavePassphraseSetting( } Text( stringResource(MR.strings.save_passphrase_in_keychain), - Modifier.padding(end = 24.dp), + Modifier.weight(1f).padding(end = 24.dp), color = Color.Unspecified ) - Spacer(Modifier.fillMaxWidth().weight(1f)) DefaultSwitch( checked = useKeychain, onCheckedChange = onCheckedChange, diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt index 40d7927264..ff393a3c30 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt @@ -497,8 +497,8 @@ fun ComposeView( } } - fun clearCurrentDraft() { - if (chatModel.draftChatId.value == draftChatId(chat.id, chatScope)) { + fun clearCurrentDraft(forChat: Chat = chat) { + if (chatModel.draftChatId.value == draftChatId(forChat.id, chatScope)) { chatModel.draft.value = null chatModel.draftChatId.value = null } @@ -575,9 +575,10 @@ fun ComposeView( } // TODO [short links] connectCheckLinkPreview - fun checkLinkPreview(): MsgContent { - val msgText = composeState.value.message.text - return when (val composePreview = composeState.value.preview) { + // the state is passed in by a send that must not read the current one - see sendMessageAsync + fun checkLinkPreview(cs: ComposeState = composeState.value): MsgContent { + val msgText = cs.message.text + return when (val composePreview = cs.preview) { is ComposePreview.CLinkPreview -> { val parsedMsg = parseToMarkdown(msgText) val url = getMessageLinks(parsedMsg).first @@ -597,6 +598,10 @@ fun ComposeView( composeState.value = composeState.value.copy(inProgress = true) } + // composeState and its inProgress flag are shared between the chats opened in this view, and sending is not cancelled + // when the chat is switched - a send may only clear or reset the state while it still holds the message that was sent + fun composeHasSentMessage(): Boolean = chatModel.chatId.value == chat.id && composeState.value.inProgress + suspend fun sendMemberContactInvitation() { val mc = checkLinkPreview() sending() @@ -604,10 +609,10 @@ fun ComposeView( if (contact != null) { withContext(Dispatchers.Main) { chatsCtx.updateContact(chat.remoteHostId, contact) - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } @@ -624,10 +629,10 @@ fun ComposeView( if (contact != null) { withContext(Dispatchers.Main) { chatsCtx.updateContact(chat.remoteHostId, contact) - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } @@ -669,15 +674,19 @@ fun ComposeView( chatModel.channelRelayHostnames.remove(groupInfo.groupId) chatModel.groupMembers.value = relayResults.map { it.relayMember } chatModel.populateGroupMembersIndexes() - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } - suspend fun sendMessageAsync(text: String?, live: Boolean, ttl: Int?, sign: Boolean = false): List? { - val cs = composeState.value + // toChat is the chat the message was composed in - it differs from the one this view shows only for the live message + // committed by a chat switch, which has no context item, so the forwarding, editing and reporting branches below + // cannot run with a different chat. cs is that send's state, captured before the switch replaced it. + suspend fun sendMessageAsync(text: String?, live: Boolean, ttl: Int?, sign: Boolean = false, toChat: Chat = chat, cs: ComposeState = composeState.value): List? { + // a send for another chat may not write to composeState, even after that chat is opened again - it was handed over + fun composeIsForSend(): Boolean = toChat.id == chat.id var sent: List? var lastMessageFailedToSend: ComposeState? = null val msgText = text ?: cs.message.text @@ -727,8 +736,8 @@ fun ComposeView( fun updateMsgContent(msgContent: MsgContent): MsgContent { return when (msgContent) { - is MsgContent.MCText -> checkLinkPreview() - is MsgContent.MCLink -> checkLinkPreview() + is MsgContent.MCText -> checkLinkPreview(cs) + is MsgContent.MCLink -> checkLinkPreview(cs) is MsgContent.MCImage -> MsgContent.MCImage(msgText, image = msgContent.image) is MsgContent.MCVideo -> MsgContent.MCVideo(msgText, image = msgContent.image, duration = msgContent.duration) is MsgContent.MCVoice -> MsgContent.MCVoice(msgText, duration = msgContent.duration) @@ -785,12 +794,12 @@ fun ComposeView( } val liveMessage = cs.liveMessage - if (!live) { + if (!live && composeIsForSend()) { if (liveMessage != null) composeState.value = cs.copy(liveMessage = null) sending() } if (!cs.forwarding || chatModel.draft.value?.forwarding == true) { - clearCurrentDraft() + clearCurrentDraft(toChat) } if (cs.contextItem is ComposeContextItem.ForwardingItems) { @@ -801,6 +810,8 @@ fun ComposeView( if (cs.message.text.isNotEmpty()) { sent?.mapIndexed { index, message -> if (index == sent!!.lastIndex) { + // the current state, not cs: forwarding is never reached from the chat switch, and keeps what was typed + // while it was in flight send(chat, checkLinkPreview(), quoted = message.id, live = false, ttl = ttl, mentions = cs.memberMentions, sign = sign) } else { message @@ -814,7 +825,7 @@ fun ComposeView( sent = if (updatedMessage != null) listOf(updatedMessage) else null lastMessageFailedToSend = if (updatedMessage == null) constructFailedMessage(cs) else null } else if (liveMessage != null && liveMessage.sent) { - val updatedMessage = updateMessage(liveMessage.chatItem, chat, live) + val updatedMessage = updateMessage(liveMessage.chatItem, toChat, live) sent = if (updatedMessage != null) listOf(updatedMessage) else null } else if (cs.contextItem is ComposeContextItem.ReportedItem) { sent = sendReport(cs.contextItem.reason, cs.contextItem.chatItem.id) @@ -824,7 +835,7 @@ fun ComposeView( val remoteHost = chatModel.currentRemoteHost.value when (val preview = cs.preview) { ComposePreview.NoPreview -> msgs.add(MsgContent.MCText(msgText)) - is ComposePreview.CLinkPreview -> msgs.add(checkLinkPreview()) + is ComposePreview.CLinkPreview -> msgs.add(checkLinkPreview(cs)) is ComposePreview.ChatLinkPreview -> { val linkStr = preview.chatLink.connLinkStr val text = if (msgText.isEmpty()) linkStr else "$msgText\n$linkStr" @@ -915,7 +926,7 @@ fun ComposeView( localPath = file.filePath ) } - val sendResult = send(chat, content, if (index == 0) quotedItemId else null, file, + val sendResult = send(toChat, content, if (index == 0) quotedItemId else null, file, live = if (content !is MsgContent.MCVoice && index == msgs.lastIndex) live else false, ttl = ttl, mentions = cs.memberMentions, @@ -932,23 +943,47 @@ fun ComposeView( val wasForwarding = cs.forwarding val forwardingFromChatId = (cs.contextItem as? ComposeContextItem.ForwardingItems)?.fromChatInfo?.id val lastFailed = lastMessageFailedToSend - if (lastFailed == null) { - clearState(live) - } else { - composeState.value = lastFailed - } - val draft = chatModel.draft.value - if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { - composeState.value = draft - } else { - clearCurrentDraft() + // composeState is shared between the chats opened in this view, and this runs after the send API call, so the user + // could have switched chats or typed another message in the meantime - only the message that was sent may be + // cleared or restored. On Main, so that these checks and changes are not interleaved with the user switching + // chats or typing. + withContext(Dispatchers.Main) { + val chatIsOpen = composeIsForSend() && chatModel.chatId.value == chat.id + // a live message is held in the compose state of the chat it is sent to, but only while that chat is the one open + val liveSend = live || cs.liveMessage != null + val sentMessageInCompose = chatIsOpen && (liveSend || composeState.value.inProgress) + if (sentMessageInCompose) { + if (lastFailed == null) { + clearState(live) + } else { + composeState.value = lastFailed + } + } + val draft = chatModel.draft.value + if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { + if (sentMessageInCompose) composeState.value = draft + } else { + clearCurrentDraft(toChat) + // liveSend excluded: a failing keystroke send would otherwise write a draft on every attempt + if (!sentMessageInCompose && !liveSend && lastFailed != null) { + // the message was not sent, so it is restored in the chat it was composed in, or kept as its draft if another chat is open + if (chatIsOpen && composeState.value.empty) { + composeState.value = lastFailed + } else if (saveLastDraft) { + chatModel.draft.value = lastFailed + chatModel.draftChatId.value = draftChatId(chat.id, chatScope) + } + } + } } return sent } - fun sendMessage(ttl: Int?, sign: Boolean = false) { + // toChat and composed are for the chat switch, which hands the compose state over to the chat it opened; passing + // toChat without doing that leaves the sent message in the input + fun sendMessage(ttl: Int?, sign: Boolean = false, toChat: Chat = chat, composed: ComposeState? = null) { withLongRunningApi(slow = 120_000) { - sendMessageAsync(null, false, ttl, sign) + sendMessageAsync(null, false, ttl, sign, toChat, composed ?: composeState.value) } } @@ -1313,13 +1348,29 @@ fun ComposeView( KeyChangeEffect(chatModel.chatId.value) { prevChatId -> val cs = composeState.value if (cs.liveMessage != null && (cs.message.text.isNotEmpty() || cs.liveMessage.sent)) { - sendMessage(null) + // the chat is already switched, so the live message goes to the chat with the id it had before the switch + val liveMessageChat = if (prevChatId == null || prevChatId == chat.id) chat else chatsCtx.getChat(prevChatId) + // if that chat is gone there is nowhere to send it, and it must not be sent to the chat opened instead + // cs is captured on this thread, before the compose state is replaced below + if (liveMessageChat != null) sendMessage(null, toChat = liveMessageChat, composed = cs) else clearState() resetLinkPreview() clearPrevDraft(prevChatId) deleteUnusedFiles() + // the sent message belongs to the chat it was composed in; the chat opened next shows its own draft + val draft = chatModel.draft.value + composeState.value = if (draft != null && chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)) draft + else ComposeState(useLinkPreviews = useLinkPreviews) } else if (cs.inProgress) { clearPrevDraft(prevChatId) - composeState.value = cs.copy(inProgress = false, progressByTimeout = false) + // the message being sent must not be kept in the compose state, it is shared with the chat opened next; + // if it fails to send it is restored in this chat or saved as its draft + clearState() + // clearState() does not load the draft of the chat opened next, and without this it is never shown and is + // dropped when that chat is left + val draft = chatModel.draft.value + if (draft != null && chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)) { + composeState.value = draft + } } else if (!cs.empty) { if (cs.preview is ComposePreview.VoicePreview && !cs.preview.finished) { recState.value = RecordingState.NotStarted diff --git a/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/database/DatabaseEncryptionView.desktop.kt b/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/database/DatabaseEncryptionView.desktop.kt index eb93e7c510..4535857696 100644 --- a/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/database/DatabaseEncryptionView.desktop.kt +++ b/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/database/DatabaseEncryptionView.desktop.kt @@ -42,10 +42,9 @@ actual fun SavePassphraseSetting( } Text( stringResource(MR.strings.save_passphrase_in_settings), - Modifier.padding(end = 24.dp), + Modifier.weight(1f).padding(end = 24.dp), color = Color.Unspecified ) - Spacer(Modifier.fillMaxWidth().weight(1f)) DefaultSwitch( checked = useKeychain, onCheckedChange = onCheckedChange, diff --git a/apps/simplex-support-bot/src/messages.ts b/apps/simplex-support-bot/src/messages.ts index c35789d26b..33f7f9ccef 100644 --- a/apps/simplex-support-bot/src/messages.ts +++ b/apps/simplex-support-bot/src/messages.ts @@ -2,7 +2,10 @@ import {isWeekend} from "./util.js" export const welcomeMessage = `Hello! This is a *SimpleX team* support bot - not an AI. *Join public groups* at https://simplex.chat/directory or [via directory bot](https://smp4.simplex.im/a#lXUjJW5vHYQzoLYgmi8GbxkGP41_kjefFvBrdwg-0Ok) -Please ask any questions about SimpleX Chat.` + +We just launched [equity crowdfunding on Wefunder](https://wefunder.com/simplex.chat)! + +Please ask any questions about SimpleX Chat and about our crowdfunding.` export function queueMessage(timezone: string, grokEnabled: boolean): string { const hours = isWeekend(timezone) ? "48" : "24" diff --git a/cabal.project b/cabal.project index dbb26ad0f2..bdd04fe7d6 100644 --- a/cabal.project +++ b/cabal.project @@ -21,7 +21,7 @@ constraints: zip +disable-bzip2 +disable-zstd source-repository-package type: git location: https://github.com/simplex-chat/simplexmq.git - tag: ee4dd0d8ded0f66f70a8890ce09d69e3610aa276 + tag: e3d53428a0c5776f9682264a56436ce97bc3eff8 source-repository-package type: git diff --git a/docs/ABOUT.md b/docs/ABOUT.md index 44809b9c0c..283b945c3f 100644 --- a/docs/ABOUT.md +++ b/docs/ABOUT.md @@ -16,3 +16,5 @@ SimpleX: "Ask SimpleX team" contact in the app or [this address](https://simplex Email: [chat@simplex.chat](mailto:chat@simplex.chat). You can use PGP to encrypt email messages using our key from [keys.openpgp.org](https://keys.openpgp.org/search?q=chat%40simplex.chat) (its fingerprint is `FB44 AF81 A45B DE32 7319 797C 8510 7E35 7D4A 17FC`) and making your key available for a secure reply. You can follow our updates on social media: [X/Twitter](https://x.com/simplexchat), [Reddit](https://www.reddit.com/r/SimpleXChat/), [Mastodon](https://mastodon.social/@simplex) and [Nostr](https://primal.net/p/npub1exv22uulqnmlluszc4yk92jhs2e5ajcs6mu3t00a6avzjcalj9csm7d828). + +Subscribe to our email updates via [this page](https://use.simplex.chat/email-updates) - they are sent via MailChimp as plaintext emails without any tracking. diff --git a/images/github-banner.jpg b/images/github-banner.jpg index f7a0d730b8..ef3cb5e6f5 100644 Binary files a/images/github-banner.jpg and b/images/github-banner.jpg differ diff --git a/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md b/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md new file mode 100644 index 0000000000..7a27fdd9d1 --- /dev/null +++ b/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md @@ -0,0 +1,244 @@ +# Fix: message being sent leaks into another chat's compose/draft, and erases what is typed there + +Branch: `nd/fix-inflight-send-writes-other-chat-compose` (off `origin/stable`) +Date: 2026-07-25 +PR: #7308 + +Line references are against `origin/stable` at `8dc387cb5`, with this fix +applied. Android and desktop only +(`multiplatform/.../views/chat/ComposeView.kt`); iOS has the same defect +but is not addressed here. + +## Problem + +Two reported symptoms, one cause. Both need a send that is still in +flight when the chat is switched (slow network, large file, or the send +just hanging with the progress circle showing): + +1. **The message ends up in another chat's draft.** Reply to a message + (or just type), press send, switch to another chat while it is + sending: the text *and the reply context* appear in that chat's input, + and leaving it saves them as that chat's draft. No forwarding + involved. +2. **A late send erases what you typed.** Press send, the progress circle + keeps spinning, switch to another chat and back, type a new message — + when the original send finally succeeds, the newly typed message is + erased. + +## Cause + +The compose state is shared, and the send outlives the chat: + +- `ChatView.kt:134` — one `MutableState` per `ChatView` + instance, `rememberSaveable` with no keys, reused for every chat that + the view displays. +- `Utils.kt:43-46` — `withLongRunningApi` launches on + `CoroutineScope(Dispatchers.Default)`, a standalone scope with no tie + to the composition or to the chat, and `sendMessage` + (`ComposeView.kt:972-976`) uses it. Leaving the chat never cancels an + in-flight send. + +Two writes then act on the wrong chat: + +- **On the chat switch** — `ComposeView.kt:1343-1347`: the `cs.inProgress` + branch used to keep the message in the shared compose state + (`composeState.value = cs.copy(inProgress = false, progressByTimeout = false)`) + and only cleared the *previous* chat's saved draft. The text and the + quote were therefore sitting in the input of the chat opened next, and + `ComposeView.kt:1348-1358` (`!cs.empty`) then saved them as *that* + chat's draft on the next switch. Symptom 1. +- **When the send completes** — `ComposeView.kt:943-968`, running in the + detached coroutine after the switch: `clearState(live)` on success, or + `composeState.value = lastFailed` on failure, where `lastFailed = + cs.copy(inProgress = false, preview = preview)` + (`ComposeView.kt:729`) **keeps `contextItem`, i.e. the reply**. On + success this wipes whatever is in the input now — including a message + typed after coming back (symptom 2); on failure it dumps the old + message into whichever chat is open (symptom 1 again). + +The same function was already inconsistent about which chat it acts on: +its draft bookkeeping (`clearCurrentDraft()`, and the forwarding +condition) uses the **captured** `chat` — the chat the message was +composed in — while its `composeState` writes hit whatever chat is +displayed at that moment. + +## Fix + +Two changes, both in `ComposeView.kt`. + +**1. Do not keep the message being sent in the shared compose state** +(`ComposeView.kt:1343-1347`). On switching away with a send in flight the +compose state is cleared, so nothing leaks into the chat opened next: + +```kotlin +} else if (cs.inProgress) { + clearPrevDraft(prevChatId) + // the message being sent must not be kept in the compose state, it is shared with the chat opened next; + // if it fails to send it is restored in this chat or saved as its draft + clearState() +} +``` + +`clearState()` is used rather than assigning an empty `ComposeState` so that +the link preview state is reset too (`pendingLinkUrl` still points at the +sent message's link, and its fetch would otherwise set a preview on the +input of the chat opened next), and so that the attachment size limit is +carried over the same way as everywhere else. + +In-flight content is deliberately **not** saved as a draft here: the +message has been submitted and will most likely be sent, and a draft is +for messages that are not sent yet. + +`clearState()` alone would leave the chat opened next with an empty input +even when it has a draft: this branch, like the live message one above it, +returns before the branch that loads a draft +(`chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)`), +so that draft was never shown - and, being still in the slot but not in +any compose state, it was then dropped by `clearPrevDraft` on the next +chat switch. It is loaded here instead. This is not the one-slot +limitation below: nothing else is competing for the slot, the draft is +simply lost. + +**2. Only touch the compose state if it still holds the message that was +sent** (`ComposeView.kt:936-968`): + +```kotlin +withContext(Dispatchers.Main) { + val chatIsOpen = chatModel.chatId.value == chat.id + val liveSend = live || cs.liveMessage != null + val sentMessageInCompose = chatIsOpen && (liveSend || composeState.value.inProgress) + if (sentMessageInCompose) { + if (lastFailed == null) { + clearState(live) + } else { + composeState.value = lastFailed + } + } + val draft = chatModel.draft.value + if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { + if (sentMessageInCompose) composeState.value = draft + } else { + clearCurrentDraft() + if (!sentMessageInCompose && lastFailed != null) { + // the message was not sent, so it is restored in the chat it was composed in, or kept as its draft if another chat is open + if (chatIsOpen && composeState.value.empty) { + composeState.value = lastFailed + } else if (saveLastDraft) { + chatModel.draft.value = lastFailed + chatModel.draftChatId.value = draftChatId(chat.id, chatScope) + } + } + } +} +``` + +Both the checks and the changes run on `Dispatchers.Main` (the block has +no suspension points), so they cannot be interleaved with the user +switching chats or typing - `KeyChangeEffect`, which does change 1, runs +there too. + +`inProgress` is the marker that the compose state is still the submitted +message: it is set by `sending()` (`ComposeView.kt:596-598`), preserved by +`copy` while sending (the only other write during a send is +`progressByTimeout` at `ComposeView.kt:1610-1617`), reset when switching +away (change 1), and never set by typing a new message. So a chat switch +*or* newly typed text both make the guard false. + +A **failed** send is different from an in-flight one - the message was not +sent, so it is an unsent message. It is put back into the input if that +chat is open and nothing else is being composed there, and kept as that +chat's draft otherwise, so it never appears in another chat (see the +limitations below for when it is still dropped). Staying in the chat is +unaffected: the guard is true there +and the failed message is restored into the input as before, keeping +"preserving long message when failed to send" (`e61babdc8`) working. + +Deliberately unchanged: + +- The condition of the forwarding branch. Gating the whole branch would + send a forward that completed after the user left to `clearCurrentDraft()` + instead, **deleting** the destination chat's draft that the branch + exists to preserve - only the compose write inside it is gated. +- Live message sends (`live`, or `cs.liveMessage != null` for the send + that finalises a live message when leaving the chat, `ComposeView.kt:1338-1342`), + as long as their chat is the one open. They never call `sending()`, so a + guard based on `inProgress` would change their behaviour: failed live + sends would stop restoring and would write a draft on every failing + keystroke send. That is why `liveSend` is an alternative to `inProgress` + inside the guard, and why it is excluded from the restore/draft branch - + not gating it there would produce exactly that draft-per-keystroke. + + What they are **not** exempt from is `chatIsOpen`. An earlier revision + had `live || cs.liveMessage != null` outside it, which holds only while + a live message is always sent to the chat that is open. #7323 removes + that: the live message committed by a chat switch is sent to the chat it + was composed in, while this view already shows another one, so an + unguarded clause here would clear *that* chat's compose state - the leak + this fix exists to prevent. Standalone this changes nothing except a + live send that completes after its chat was left, which now leaves the + opened chat alone. `sendMessageAsync` reads `composeState` inside the + coroutine (`ComposeView.kt:684`), so that branch cannot clear the state + itself without racing the send; #7323 adds the `composed` parameter that + makes the captured state explicit. + +**3. The same check where the flag is shared** (`ComposeView.kt:600-602` +and the three senders that connect a prepared chat). They call the same +`sending()`, so an unguarded `clearState()` or `inProgress` reset from +one of them corrupts the state of a send started in the chat opened next. + +## Behaviour after the fix + +| situation | before | after | +| --- | --- | --- | +| send, stay in chat, succeeds | input cleared | input cleared (unchanged) | +| send, stay in chat, fails | message restored in input | message restored in input (unchanged) | +| send, switch chats, succeeds | message left in the other chat's input, saved as its draft | other chat untouched | +| send, switch chats, fails | message dumped into the other chat's input | message restored in the chat it was composed in, or kept as its draft | +| send hangs, switch away and back, type, then it succeeds | typed message erased | typed message kept | +| forward send, still in destination chat | destination chat's draft restored | unchanged | +| live message sent on leaving the chat | compose state cleared by the send | unchanged | + +## Limitations + +Kept deliberately, to not grow the change: + +- A message that failed to send is dropped, rather than kept, when the + "Message draft" privacy setting is off, when the destination chat of a + failed forward already has a draft (its own draft is preserved + instead), and when the single draft slot is later taken by another + chat - drafts are one global slot, so the last write wins. +- The three senders that connect a prepared chat share the same + `sending()` flag, so they use the same check (`ComposeView.kt:604-616`, + `618-640`, `659-685`). Without it a connect completing after the chat + was switched would clear `inProgress` for a send started in the chat + opened next, and that sent message would then stay in the input. They + have no failed-message restore, so their typed message is dropped when + the chat is switched instead of being carried into the next chat. +- Typing in the same chat while its own send is in flight is still + cleared when the send completes: `inProgress` is preserved by `copy`, + so the guard stays true. Unchanged from before, and different from the + reported symptom, which needs the chat to be switched. +## Verification + +- `./gradlew :common:compileKotlinDesktop` — passes. +- Manual (needs a slow or failing send — e.g. airplane mode, or a large + file). On desktop any chat switch exercises it; on Android only an + in-place switch does (member info → open chat), because leaving to the + chat list destroys the view: + 1. Reply + type in A, send, switch to B while sending. B's input must + stay empty; leaving B must not create a draft in B. If the send + failed, A must hold the message (with the reply) as its draft. + 2. Send in A with the network off so the circle keeps spinning, switch + to B and back to A, type a new message, restore the network. The + typed message must survive the old send completing. + 3. Regression: ordinary send in A (input clears), failed send while + staying in A (message comes back in the input), forward into a chat + that has a draft (draft restored after sending). + +Rebased onto the scope-aware draft ids introduced by #7309: the draft +written here for a message that failed to send uses +`draftChatId(chat.id, chatScope)`, like every other draft write. + +Related: `plans/2026-07-25-fix-forward-moves-draft-to-target-chat.md` +(PR #7307) — different cause (stale `chat` captured by the desktop +`onDispose`), same shared-compose-state design. diff --git a/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md b/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md new file mode 100644 index 0000000000..bd37361dc5 --- /dev/null +++ b/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md @@ -0,0 +1,248 @@ +# Fix: live message is sent to the chat opened after switching chats + +Branch: `nd/fix-live-message-sent-to-wrong-chat` (off `origin/stable`) +Date: 2026-07-29 + +Line references are against `origin/stable` at `970ef8932`, with this fix +applied. Android and desktop (`commonMain/ComposeView.kt`). + +## Problem + +Typing a live message and switching to another chat sends that message to +the chat that was opened, without the user sending anything. Reported on +desktop, where every chat switch reuses the same view. + +## Cause + +A live message is committed when the chat is switched +(`ComposeView.kt:1353-1369`), which before this change was: + +``` + if (cs.liveMessage != null && (cs.message.text.isNotEmpty() || cs.liveMessage.sent)) { + sendMessage(null) +``` + +`KeyChangeEffect` is `LaunchedEffect(key1) { block(prev) }` +(`Utils.kt:683-698`), so when the key changes, `remember(key1)` rebuilds +it from the lambda of the composition that is running *now* - and by then +`chatModel.chatId` is already the new chat, `ChatView` has recomposed +`ComposeView` with the new `chat`, and the block that runs captured that +one. + +`sendMessage(null)` → `sendMessageAsync` → `send(chat, ...)` uses the +captured `chat`, while the message content comes from `composeState`, +which is shared between the chats opened in this view. So the content of +the chat that was left is sent to the chat that was opened: + +- `liveMessage.sent == false` - a new message is created in the wrong + chat, which is what is seen; +- `liveMessage.sent == true` - `apiUpdateChatItem` is called with the new + chat's type and id and the item id from the previous chat, which the + backend cannot resolve. + +The same mismatch made the post-send `clearCurrentDraft()` clear the +draft of the chat opened after the switch, deleting a draft that was +never sent. + +## Fix + +`sendMessageAsync` and `sendMessage` take the chat the message was +composed in, defaulting to the chat this view shows +(`ComposeView.kt:685-694`, `988-993`). Only what a live message can reach +uses it: the message send, the update of an already sent live message, +and the two places that clear the draft after sending. Live messages have +no context item (`SendMsgView.kt:156-165` only offers the button when the +compose is empty and has none), so the forwarding, editing and reporting +branches cannot run with a chat other than the view's and keep using +`chat` - the parameter is not threaded through them. + +The chat switch resolves the chat by the id it had before the switch: + +```kotlin +val liveMessageChat = if (prevChatId == null || prevChatId == chat.id) chat else chatsCtx.getChat(prevChatId) +// if that chat is gone there is nowhere to send it, and it must not be sent to the chat opened instead +if (liveMessageChat != null) sendMessage(null, toChat = liveMessageChat, composed = cs) else clearState() +``` + +`prevChatId == chat.id` keeps the view's own chat, which is what secondary +(member support) chat views need - they share the group's chat id, and +only their `chat` carries the scope. + +If the previous chat can no longer be found the message is not sent at +all, and the compose state is cleared so it does not leak into the chat +that was opened. Sending it to the chat that is open now is the defect +being fixed, so it is not used as a fallback. + +### Handing the compose state over to the opened chat + +Sending to the right chat is not enough on its own: `composeState` is +shared between the chats opened in this view, and this is the only branch +of `KeyChangeEffect` that neither resets it nor loads the opened chat's +draft - the branch that loads a draft (`else if (chatModel.draftChatId +.value == draftChatId(chatModel.chatId.value, chatScope) ...)`) is later +in the same `if` chain and cannot be reached. So the live message stayed +in the compose state of a view that now shows another chat, and that +chat's draft was never read. + +`sendMessageAsync` then made it visible. It runs on `Dispatchers.Default`, +so its writes land after the switch: + +```kotlin +val liveMessage = cs.liveMessage +if (!live) { + if (liveMessage != null) composeState.value = cs.copy(liveMessage = null) // the whole composed state + sending() // and its spinner +} +``` + +The opened chat's input showed the text composed in the previous one until +the send completed and `clearState()` emptied it; the draft it should have +shown was still in the model, and the next switch away dropped it. This +predates this fix - without it the same writes happen, and there +`clearCurrentDraft()` resolves to the opened chat and deletes its draft +outright. + +Four changes, all following from "this send no longer owns the compose +state": + +- `sendMessageAsync` takes its `cs` as a parameter defaulting to + `composeState.value`, and `sendMessage` takes `composed: ComposeState? = + null`, so only the chat switch passes a state and every other sender + still reads it inside the coroutine, exactly where the send read it + before. The chat switch + captures it on the main thread before replacing it - without that the + send would read the compose state of the chat that was opened and send + *its draft* to the previous chat. +- `checkLinkPreview` takes that state too. It re-read `composeState` + rather than what was passed in, and it is reached by every text live + message through `updateMsgContent`, so with the compose state handed + over it would have rebuilt the message from the opened chat's draft, or + from nothing - overwriting the live message instead of committing it. + Only the calls a live message can reach pass the state. The forwarding + call site keeps reading the current one - it is unreachable from the + chat switch, and `forwardItem` suspends before it, so passing the + captured state there would drop what was typed while the forward was in + flight. The three senders that connect a prepared chat keep reading the + current one too. +- Every `composeState` write in `sendMessageAsync` is guarded by + `composeIsForSend()` (`toChat.id == chat.id`): directly for the two at + the start, and through `chatIsOpen` for the clear/restore at the end, + which #7308 already routes through `sentMessageInCompose`. It compares + the two chats rather than checking which one is open, so the send made + by a chat switch never takes the compose state back, not even if that + chat is opened again before the send completes. + `clearCurrentDraft(toChat)` is already keyed on the chat and needs no + guard. Whether the *view's own* send may still write when its chat has + been switched away is #7308's question, not this one's. +- The chat-switch branch then resets `composeState` to the opened chat's + draft, or to an empty state, like the branches below it do. + +## Blast radius + +`toChat` defaults to the chat this view shows, so every other send passes +no chat: the send button (`SendMsgView.kt`), the live updates while typing +(`sendMessageAsync(live = true)`), forwarding, editing and reporting. For +all of them `composeIsForSend()` is true, so every guard added here is a +no-op and they behave exactly as before. Only the send started by the chat +switch passes a different chat, and only the branches it can reach were +changed. + +The one change not behind that guard is `checkLinkPreview` reading the +state passed in. It matters only where the two can differ, which is after +a suspension: the forwarding branch waits on `forwardItem`, so that call +site deliberately keeps reading the current state (it is unreachable from +the chat switch anyway). The other call sites are reached with nothing +suspending since the state was captured. + +The live message update loop is not affected: it is started once +(`SendMsgView.kt:523-559`) with the `::updateLiveMessage` reference of the +composition in which live mode started, so its updates already go to the +chat the message belongs to. It exits because the chat switch replaces the +compose state with the opened chat's, which has no `liveMessage` - on the +main thread, as the chat is switched, rather than when the send completes +as before. Only that send was created fresh on every composition, which is +why it was the one going to the wrong chat. + +Not covered, and unchanged: a live message in a member support chat that +is closed without changing the chat id is never committed - the effect +that commits it is keyed on the chat id, which does not change when that +view is closed. + +`chatsCtx.getChat` searches the context's own list, and a secondary +context (member support, reports) is built with an empty one, so there it +can only return null. That branch is not reached from a support chat in +practice - it shares the group's chat id, so `prevChatId == chat.id` holds +and the view's own `chat` is used - and if it ever were, the message is +discarded rather than sent to the chat that was opened, which is the +behaviour intended for "the chat is gone" anyway. + +## Verification + +- `./gradlew :common:compileKotlinDesktop` — passes. +- Manual: + 1. Start a live message in **A**, type, and switch to **B** while + typing. The message must appear in **A**; nothing is sent in **B**, + and B's input and draft are untouched. + 2. Repeat with a draft already saved in **B** - it must still be there + after the switch. This is the case that was found failing: B showed + the text composed in A, then emptied when the send completed, and B's + draft was dropped on the next switch. Watch B's input from the moment + of the switch, not only after the send finishes. + 3. Slow or failing send (network off) while doing 1 and 2, so the window + between the switch and the send completing is long enough to type in + **B** - what is typed there must survive the send completing. + 4. The live message must carry a **link preview**: type a URL in **A**, + let the preview load, then switch. The message committed to A must be + the text that was composed - not the opened chat's draft, and not + empty. Every text live message is rebuilt through + `updateMsgContent` -> `checkLinkPreview`, so this is what breaks if + that one stops reading the state it was given. + 5. Switch **back**: live message in A, switch to B, return to A and type + something new before the send completes. What is typed in A must + survive - the send handed the compose state over at the switch and + must not take it back. + 6. Regressions: an ordinary send goes to the chat it was typed in; + forwarding still targets the chat it was forwarded to, and text typed + while a forward is in flight is still appended to it; reporting a + message still reports it in the chat it belongs to; sending in a + member support chat still goes to that scope. + +## Merged with #7308 + +#7308 (a send that is still in flight when the chat is switched) landed in +`stable` first, so this branch was merged with it. Both changed the end of +`sendMessageAsync`, and the two guards are **not** the same rule - the +merge keeps both: + +- here, `composeIsForSend()` = `toChat.id == chat.id` - is this send for + the chat this view shows, or for another one; +- in #7308, `chatIsOpen` = `chatModel.chatId.value == chat.id` - is the + chat this view shows still the one open. + +`chatIsOpen` becomes the conjunction, +`composeIsForSend() && chatModel.chatId.value == chat.id`. Where `toChat` +is `chat` - every send but the one made by a chat switch - that reduces to +#7308's own check, so its behaviour is unchanged. + +Nothing else in that block had to move. #7308 already routes both compose +writes through `sentMessageInCompose`, which derives from `chatIsOpen`, so +guarding `chatIsOpen` guards them; the rest of the change there is one +call site taking `toChat`, `clearCurrentDraft`. The draft id a failed +message is saved under keeps using `chat`: that branch is behind +`!liveSend`, which the send made by a chat switch never satisfies, so +`toChat` is always `chat` where it is read. + +An earlier revision of this note said that #7308's `cs.liveMessage != null` +clause "already covers the send made by the chat switch". **It did not.** +At the time that clause sat outside the `chatIsOpen` check: + +```kotlin +val sentMessageInCompose = live || cs.liveMessage != null || (chatIsOpen && composeState.value.inProgress) +``` + +which is correct only while a live message is always sent to the chat that +is open - the assumption this fix removes. Read as written, the clause +*exempts* the chat-switch send from the very guard that protects the +opened chat, and a merge that followed it reintroduced the leak described +above. #7308 shipped with the live clauses moved inside `chatIsOpen`, +which was a no-op on its own branch and is what makes this merge work. diff --git a/plans/2026-07-31-fix-db-passphrase-toggle-clipped.md b/plans/2026-07-31-fix-db-passphrase-toggle-clipped.md new file mode 100644 index 0000000000..16767d62aa --- /dev/null +++ b/plans/2026-07-31-fix-db-passphrase-toggle-clipped.md @@ -0,0 +1,124 @@ +# Fix "Save passphrase in settings" toggle unreachable on desktop + +## Symptom + +On 7.0 desktop (reproduced on the Linux AppImage and on Windows), in +Chat data → Database passphrase & export → Database passphrase, the +"Save passphrase in settings" toggle cannot be switched off. The reporter sees +the switch sitting slightly past the right edge of the section card. Because the +setting stays on, the passphrase remains stored in `settings.properties` and the +app never prompts for it on start — on desktop that file holds the passphrase in +clear text, since `Cryptor.desktop.kt` is an identity implementation. + +This is distinct from the case where the switch is *rendered disabled* +(`DatabaseEncryptionView.kt:127`, `enabled = (!initialRandomDBPassphrase && !progressIndicator) || migration`), +which is intended behaviour for a database still using the initial random +passphrase. The reports here are from users who set their own passphrase, so +`initialRandomDBPassphrase == false` and the switch is enabled — just not +reachable. + +## Root cause + +1. `SavePassphraseSetting` is hand-rolled in both platform actuals + (`DatabaseEncryptionView.desktop.kt:43-53`, `.android.kt:43-53`) and is the + only toggle row in the app whose label carries no `weight`: + + ```kotlin + Text(stringResource(MR.strings.save_passphrase_in_settings), Modifier.padding(end = 24.dp)) + Spacer(Modifier.fillMaxWidth().weight(1f)) + DefaultSwitch(checked = useKeychain, onCheckedChange = onCheckedChange, enabled = enabled) + ``` + + `Row` measures unweighted children first against the full available width, then + divides what is left among weighted ones. A label that does not comfortably fit + consumes the remainder, the weighted `Spacer` collapses to zero, and + `DefaultSwitch` is placed past the row's right edge. + +2. Every other toggle row goes through `SettingsActionItemWithContent` + (`SettingsView.kt:380`), which gives the label `Modifier.weight(1f)`. There the + label truncates and the trailing control keeps its size and position, so the + same string length is harmless. + +3. Before #6777 this row had enough slack and no clipping. `SectionView` was a + plain `Column` with no horizontal inset, and `SectionItemView` used + `DEFAULT_PADDING` (20.dp) per side — 348.dp of content width on the desktop + start pane (`DEFAULT_START_MODAL_WIDTH` = 388.dp). Any overflow still drew and + still received pointer events. + +4. #6777 introduced `LocalCardScreen` / `CardColumnLayout` (`Section.kt`), which + wraps section content in `Modifier.padding(horizontal = CARD_PADDING /* 18.dp */)` + … `.clip(SectionCardShape)`, and switches `itemHPadding` from `DEFAULT_PADDING` + to `CARD_PADDING`. `DatabaseEncryptionView` is opened with `cardScreen = true` + (`DatabaseView.kt:235`), so its row content width drops 348.dp → **316.dp**. + +5. `Modifier.clip` clips pointer input as well as drawing. The displaced switch is + therefore both cut off visually and unhittable — the toggle stops working rather + than merely looking wrong. + +Budget arithmetic on the desktop start pane: fixed cost in the row is ~96.dp +(24 icon + 8 spacer + 24 label end-padding + ~40 switch), leaving ~220.dp for a +27-character label at 16.sp, which needs ~215.dp in English. Borderline at 100% +font scale and over budget as soon as the label is longer — a longer localization, +or a larger font size, since the label scales with `fontSizeSqrtMultiplier` while +`CARD_PADDING` does not. + +The widths above are derived from the layout constants, not measured against a +running client; the reporter's observation that the switch sits slightly past the +card edge is what confirms the row overflows in practice. + +## Fix + +Move the weight onto the label and drop the weighted spacer, in both actuals: + +```kotlin +Text(stringResource(MR.strings.save_passphrase_in_settings), Modifier.weight(1f).padding(end = 24.dp)) +DefaultSwitch(checked = useKeychain, onCheckedChange = onCheckedChange, enabled = enabled) +``` + +The label now truncates instead of displacing the switch, matching what +`SettingsActionItemWithContent` does for every other toggle row. + +The spacer has to go: leaving both the label and the spacer weighted would split +the remaining space between them and starve the label instead, which trades one +layout bug for another. + +## Why this fix and not alternatives + +- **Widening the card or shrinking `CARD_PADDING`** would buy back the ~32.dp lost + in #6777, but only until the next longer localization or font-size step. The row + would stay the one place in the app where a long label can push a control out of + reach. +- **Removing `clip` from `CardColumnLayout`** would restore clickability of + overflowing content, but the clip is what gives section cards their rounded + corners; dropping it would regress the design and leave the switch drawn outside + its card. +- **Shortening the string** is a translation-wide problem, not a fix, and does not + help at larger font sizes. + +## Impact + +- Desktop and Android only. Both actuals carry the identical defect; Android's row + is in fact narrower still (~288.dp on a 360.dp-wide screen), so it is affected at + least as much — it simply has not been reported. +- iOS is unaffected: `DatabaseEncryptionView.swift` uses a SwiftUI `Toggle` inside + `settingsRow`, where the label truncates and the toggle cannot be displaced. The + `initialRandomDBPassphrase` disabled-state logic is the same on iOS + (`DatabaseEncryptionView.swift:80`) and is unchanged by this fix. +- Users already stuck in the bad state have `StoreDBPassphrase=true` in + `settings.properties` with the passphrase stored alongside it. After this fix + they can turn the setting off in the UI, which removes the stored passphrase via + `removePassphraseFromKeyChain` and restores the prompt on start. +- No behaviour change beyond the row layout: no logic, preference, or string was + touched. + +## Verification + +- `bash ~/build/linux.sh` on this branch: cold `dist-newstyle`, `libsimplex.so` + rebuilt from master's sources, `:common:compileKotlinDesktop` executed, + `BUILD SUCCESSFUL`, AppImage produced. +- `bash ~/build/android.sh` on this branch: `BUILD SUCCESSFUL`, arm64-v8a debug APK + produced (native libs are the prebuilt ones, so this exercises the Kotlin change + only). +- Not done: the rendered row has not been checked in a running client. Worth + confirming at a raised font size and in a locale with a longer label, which is + the case that made the overflow visible in the first place. diff --git a/scripts/nix/sha256map.nix b/scripts/nix/sha256map.nix index 7e422c51fe..2a39a6baf4 100644 --- a/scripts/nix/sha256map.nix +++ b/scripts/nix/sha256map.nix @@ -1,5 +1,5 @@ { - "https://github.com/simplex-chat/simplexmq.git"."ee4dd0d8ded0f66f70a8890ce09d69e3610aa276" = "0zahz4011adavfz680wkcqvcrl2f9zjdx6dly6yqkcin4bpr6v3k"; + "https://github.com/simplex-chat/simplexmq.git"."e3d53428a0c5776f9682264a56436ce97bc3eff8" = "1i3x4q6sc8w6hndrmmrsgc15di0bz6w29r4y5cr985rvs9c2mx7d"; "https://github.com/simplex-chat/hs-socks.git"."a30cc7a79a08d8108316094f8f2f82a0c5e1ac51" = "0yasvnr7g91k76mjkamvzab2kvlb1g5pspjyjn2fr6v83swjhj38"; "https://github.com/simplex-chat/direct-sqlcipher.git"."f814ee68b16a9447fbb467ccc8f29bdd3546bfd9" = "1ql13f4kfwkbaq7nygkxgw84213i0zm7c1a8hwvramayxl38dq5d"; "https://github.com/simplex-chat/sqlcipher-simple.git"."a46bd361a19376c5211f1058908fc0ae6bf42446" = "1z0r78d8f0812kxbgsm735qf6xx8lvaz27k1a0b4a2m0sshpd5gl"; diff --git a/src/Simplex/Chat/Controller.hs b/src/Simplex/Chat/Controller.hs index af20d97bee..e8392b8c42 100644 --- a/src/Simplex/Chat/Controller.hs +++ b/src/Simplex/Chat/Controller.hs @@ -70,6 +70,7 @@ import Simplex.Chat.Types.Shared import Simplex.Chat.Types.UITheme import Simplex.Chat.Util (liftIOEither) import Simplex.FileTransfer.Description (FileDescriptionURI) +import Simplex.Messaging.Server.Information (ServerPublicInfo) import Simplex.Messaging.Agent (AgentClient, DatabaseDiff, SubscriptionsInfo) import Simplex.Messaging.Agent.Client (AgentLocks, AgentQueuesInfo (..), AgentWorkersDetails (..), AgentWorkersSummary (..), ProtocolTestFailure, SMPServerSubs, ServerQueueInfo, UserNetworkInfo) import Simplex.Messaging.Agent.Env.SQLite (AgentConfig, NetworkConfig, ServerCfg, Worker) @@ -781,7 +782,7 @@ data ChatResponse | CRChatItems {user :: User, chatName_ :: Maybe ChatName, chatItems :: [AChatItem]} | CRChatItemInfo {user :: User, chatItem :: AChatItem, chatItemInfo :: ChatItemInfo} | CRChatItemId User (Maybe ChatItemId) - | CRServerTestResult {user :: User, testServer :: AProtoServerWithAuth, testFailure :: Maybe ProtocolTestFailure} + | CRServerTestResult {user :: User, testServer :: AProtoServerWithAuth, testFailure :: Maybe ProtocolTestFailure, serverInfo :: Maybe (Either String ServerPublicInfo)} | CRChatRelayTestResult {user :: User, relayProfile :: Maybe RelayProfile, relayTestFailure :: Maybe RelayTestFailure} | CRServerOperatorConditions {conditions :: ServerOperatorConditions} | CRUserServers {user :: User, userServers :: [UserOperatorServers]} diff --git a/src/Simplex/Chat/Files.hs b/src/Simplex/Chat/Files.hs index 0c04b22e28..791f34c6ff 100644 --- a/src/Simplex/Chat/Files.hs +++ b/src/Simplex/Chat/Files.hs @@ -5,14 +5,20 @@ module Simplex.Chat.Files where import Simplex.Chat.Controller import Simplex.Messaging.Util (ifM) -import System.FilePath (combine, splitExtensions) +import System.FilePath (combine, makeValid, splitExtensions, takeFileName) import UnliftIO.Directory (doesDirectoryExist, doesFileExist, getHomeDirectory, getTemporaryDirectory) +safeFileNameStr :: String -> String +safeFileNameStr = notDots . makeValid . takeFileName + where + notDots n = if n == "." || n == ".." then "_" else n + +-- | The file name is sanitized, so the combined path cannot escape the folder. uniqueCombine :: FilePath -> String -> IO FilePath uniqueCombine fPath fName = tryCombine (0 :: Int) where tryCombine n = - let (name, ext) = splitExtensions fName + let (name, ext) = splitExtensions $ safeFileNameStr fName suffix = if n == 0 then "" else "_" <> show n f = fPath `combine` (name <> suffix <> ext) in ifM (doesFileExist f) (tryCombine $ n + 1) (pure f) diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 88cb4532ef..8a41e7c655 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -1675,8 +1675,9 @@ processChatCommand cxt nm = \case aUserServer (AProtoServerWithAuth p' srv) = case testEquality p p' of Just Refl -> pure $ AUS SDBNew $ newUserServer srv Nothing -> throwCmdError $ "incorrect server protocol: " <> B.unpack (strEncode srv) - APITestProtoServer userId srv@(AProtoServerWithAuth _ server) -> withUserId userId $ \user -> - lift $ CRServerTestResult user srv <$> withAgent' (\a -> testProtocolServer a nm (aUserId user) server) + APITestProtoServer userId srv@(AProtoServerWithAuth _ server) -> withUserId userId $ \user -> do + r <- lift $ withAgent' $ \a -> testProtocolServer a nm (aUserId user) server + pure $ uncurry (CRServerTestResult user srv) $ either ((,Nothing) . Just) (Nothing,) r TestProtoServer srv -> withUser $ \User {userId} -> processChatCommand cxt nm $ APITestProtoServer userId srv APITestChatRelay userId address -> withUserId userId $ \user -> do @@ -4054,11 +4055,12 @@ processChatCommand cxt nm = \case lift . when (directOrUsed ct') $ createSndFeatureItems user ct ct' pure $ CRContactPrefsUpdated user ct ct' runUpdateGroupProfile :: User -> GroupInfo -> GroupProfile -> Bool -> CM ChatResponse - runUpdateGroupProfile user gInfo@GroupInfo {businessChat, groupProfile = p@GroupProfile {displayName = n}} p'@GroupProfile {displayName = n', image = img'} domainVerified = do + runUpdateGroupProfile user gInfo@GroupInfo {businessChat, groupProfile = p@GroupProfile {displayName = n}} p'@GroupProfile {displayName = n', image = img', memberAdmission = ma'} domainVerified = do assertUserGroupRole gInfo GROwner when (n /= n') $ checkValidName n' checkProfileImageSize img' checkGroupProfileSize p' + when (useRelays' gInfo && isJust (ma' >>= review)) $ throwCmdError "Admission review is not supported in channels" -- updateGroupProfile clears domain verification; re-set it when the caller already re-resolved the name gInfo' <- withStore $ \db -> do g <- updateGroupProfile db user gInfo p' @@ -4210,10 +4212,11 @@ processChatCommand cxt nm = \case groupMemberId <- getGroupMemberIdByName db user groupId groupMemberName pure (groupId, groupMemberId) newGroup :: User -> IncognitoEnabled -> GroupProfile -> Bool -> MemberId -> Maybe GroupKeys -> Maybe Int64 -> CM GroupInfo - newGroup user incognito gProfile@GroupProfile {displayName, image} useRelays memberId groupKeys_ publicMemberCount_ = do + newGroup user incognito gProfile@GroupProfile {displayName, image, memberAdmission} useRelays memberId groupKeys_ publicMemberCount_ = do checkValidName displayName checkProfileImageSize image checkGroupProfileSize gProfile + when (useRelays && isJust (memberAdmission >>= review)) $ throwCmdError "Admission review is not supported in channels" -- [incognito] generate incognito profile for group membership incognitoProfile <- if incognito then Just <$> liftIO generateRandomProfile else pure Nothing withFastStore $ \db -> createNewGroup db cxt user gProfile incognitoProfile useRelays memberId groupKeys_ publicMemberCount_ diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index b3f3a0fbe6..6e798f7ab2 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -48,7 +48,7 @@ import Data.Word (Word32) import Simplex.Chat.Call import Simplex.Chat.Controller import Simplex.Chat.Delivery -import Simplex.Chat.Files (getChatTempDirectory) +import Simplex.Chat.Files (getChatTempDirectory, safeFileNameStr) import Simplex.Chat.Library.Internal import Simplex.Chat.Web (channelContentChanged, channelProfileUpdated, channelRemoved) import Simplex.Chat.Messages @@ -101,7 +101,6 @@ import qualified Simplex.Messaging.TMap as TM import Simplex.Messaging.Transport (TransportError (..)) import Simplex.Messaging.Util import Simplex.Messaging.Version -import qualified System.FilePath as FP import System.Mem.Weak (Weak) import Text.Read (readMaybe) import UnliftIO.Concurrent (ThreadId, forkIO, mkWeakThreadId) @@ -916,7 +915,10 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = if useRelays' gInfo'' then do introduceInChannel cxt user gInfo'' m' - when (groupFeatureAllowed SGFHistory gInfo'') $ sendHistory user gInfo'' m' + case mStatus of + GSMemPendingApproval -> pure () + GSMemPendingReview -> pure () + _ -> when (groupFeatureAllowed SGFHistory gInfo'') $ sendHistory user gInfo'' m' else case mStatus of GSMemPendingApproval -> pure () GSMemPendingReview -> introduceToModerators cxt user gInfo'' m' @@ -1963,7 +1965,7 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = pure (ft', CIFile {fileId, fileName, fileSize, fileSource, fileStatus, fileProtocol}) mkValidFileInvitation :: FileInvitation -> FileInvitation - mkValidFileInvitation fInv@FileInvitation {fileName} = fInv {fileName = FP.makeValid $ FP.takeFileName fileName} + mkValidFileInvitation fInv@FileInvitation {fileName} = fInv {fileName = safeFileNameStr fileName} validateFileInvitation :: FileInvitation -> CM FileInvitation validateFileInvitation fInv@FileInvitation {fileName, fileSize} @@ -3885,7 +3887,7 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = Just author -> action author Nothing -> messageError $ "x.grp.msg.forward: event " <> tshow tag <> " requires author" - withVerifiedMsg :: MsgEncodingI e => GroupInfo -> Maybe GroupChatScopeInfo -> GroupMember -> ParsedMsg e -> UTCTime -> (VerifiedMsg e -> CM a) -> CM (Maybe a) + withVerifiedMsg :: GroupInfo -> Maybe GroupChatScopeInfo -> GroupMember -> ParsedMsg e -> UTCTime -> (VerifiedMsg e -> CM a) -> CM (Maybe a) withVerifiedMsg gInfo@GroupInfo {membership} scopeInfo member (ParsedMsg _ signedMsg_ chatMsg@ChatMessage {chatMsgEvent}) ts action = case verified of Just verifiedMsg -> Just <$> action verifiedMsg diff --git a/src/Simplex/Chat/Remote.hs b/src/Simplex/Chat/Remote.hs index 39405bd1ba..0e23cc795c 100644 --- a/src/Simplex/Chat/Remote.hs +++ b/src/Simplex/Chat/Remote.hs @@ -65,10 +65,10 @@ import Simplex.Messaging.Util import Simplex.RemoteControl.Client import Simplex.RemoteControl.Invitation (RCInvitation (..), RCSignedInvitation (..), RCVerifiedInvitation (..), verifySignedInvitation) import Simplex.RemoteControl.Types -import System.FilePath (takeFileName, ()) +import System.FilePath (takeDirectory, takeFileName, ()) import UnliftIO import UnliftIO.Concurrent (forkIO) -import UnliftIO.Directory (copyFile, createDirectoryIfMissing, doesDirectoryExist, removeDirectoryRecursive, renameFile) +import UnliftIO.Directory (canonicalizePath, copyFile, createDirectoryIfMissing, doesDirectoryExist, removeDirectoryRecursive, renameFile) remoteFilesFolder :: String remoteFilesFolder = "simplex_v1_files" @@ -574,10 +574,19 @@ handleStoreFile rfKN fileName fileSize fileDigest getChunk = Nothing -> storeFileTo =<< getDefaultFilesFolder storeFileTo :: FilePath -> CM' (Either RemoteProtocolError FilePath) storeFileTo dir = liftIO . tryAllErrors' $ do + unless (validRemoteFileName fileName) $ throwError $ RPEInvalidBody "invalid file name" filePath <- liftIO $ dir `uniqueCombine` fileName + -- resolves symlinks, so it also catches a final component linking outside the folder + canonPath <- liftIO $ canonicalizePath filePath + inDir <- liftIO $ (takeDirectory canonPath ==) <$> canonicalizePath dir + unless inDir $ throwError $ RPEInvalidBody "file path outside of files folder" receiveEncryptedFile rfKN getChunk fileSize fileDigest filePath pure filePath +-- The controller only ever sends a bare file name (see storeRemoteFile), so a path is a protocol violation. +validRemoteFileName :: FilePath -> Bool +validRemoteFileName fName = fName == takeFileName fName && fName `notElem` (["", ".", ".."] :: [FilePath]) + handleGetFile :: User -> RemoteFile -> Respond -> CM () handleGetFile User {userId} RemoteFile {userId = commandUserId, fileId, sent, fileSource = cf'@CryptoFile {filePath}} reply = do logDebug $ "GetFile: " <> tshow filePath diff --git a/src/Simplex/Chat/View.hs b/src/Simplex/Chat/View.hs index 5894d12fd3..72631dc3f8 100644 --- a/src/Simplex/Chat/View.hs +++ b/src/Simplex/Chat/View.hs @@ -126,7 +126,11 @@ chatResponseToView hu cfg@ChatConfig {logLevel, showReactions, showFullLinks, te CRApiChat u chat _ -> ttyUser u $ if testView then testViewChat chat else [viewJSON chat] CRChatContentTypes cts -> [plain $ "Chat content types: " <> T.intercalate ", " (map (safeDecodeUtf8 . strEncode) cts)] CRChatTags u tags -> ttyUser u [viewJSON tags] - CRServerTestResult u srv testFailure -> ttyUser u $ viewServerTestResult srv testFailure + CRServerTestResult u srv testFailure info -> ttyUser u $ viewServerTestResult srv testFailure <> maybe [] viewServerInfo info + where + viewServerInfo = \case + Left e -> [plain $ "Server Info Error: " <> T.pack e] + Right i -> [plain $ "Server Info: " <> tshow i] CRChatRelayTestResult u relayProfile_ relayTestFailure_ -> ttyUser u $ viewRelayTestResult relayProfile_ relayTestFailure_ CRServerOperatorConditions (ServerOperatorConditions ops _ ca) -> viewServerOperators ops ca CRUserServers u uss -> ttyUser u $ concatMap viewUserServers uss <> (if testView then [] else serversUserHelp) diff --git a/tests/RemoteTests.hs b/tests/RemoteTests.hs index e96d531805..1bb6ac34eb 100644 --- a/tests/RemoteTests.hs +++ b/tests/RemoteTests.hs @@ -11,22 +11,25 @@ import ChatTests.DBUtils import ChatTests.Utils import Control.Logger.Simple import Control.Monad +import Control.Monad.Except (runExceptT) import qualified Data.Aeson as J import qualified Data.ByteString as B import qualified Data.ByteString.Lazy.Char8 as LB import Data.List (find, isPrefixOf) import qualified Data.Map.Strict as M import Simplex.Chat.Controller (ChatCommand (..), ChatConfig (..), versionNumber) +import Simplex.Chat.Files (safeFileNameStr) import Simplex.Chat.Library.Commands (parseChatCommand) import qualified Simplex.Chat.Controller as Controller import Simplex.Chat.Mobile.File -import Simplex.Chat.Remote (remoteFilesFolder) +import Simplex.Chat.Remote (remoteFilesFolder, validRemoteFileName) +import Simplex.Chat.Remote.Protocol (remoteStoreFile) import Simplex.Chat.Remote.Types import Simplex.Messaging.Crypto.File (CryptoFileArgs (..)) import Simplex.Messaging.Encoding.String (strEncode) import Simplex.Messaging.Util import Simplex.RemoteControl.Types (RCCtrlAddress (..)) -import System.FilePath (()) +import System.FilePath (takeFileName, ()) import Test.Hspec hiding (it) import UnliftIO import UnliftIO.Concurrent @@ -40,10 +43,26 @@ remoteTests = describe "Remote" $ do `shouldSatisfy` \case Right (StartRemoteHost Nothing (Just (RCCtrlAddress _ "Ethernet 2")) (Just 12345)) -> True _ -> False + describe "stored file name" $ do + it "rejects names with directory components" $ \_ -> + filter validRemoteFileName ["../x", "../../etc/passwd", "/etc/cron.d/x", "a/b", "x/", "", ".", ".."] + `shouldBe` [] + it "accepts bare file names" $ \_ -> + filter (not . validRemoteFileName) ["test.pdf", "test_1.pdf", ".hidden", "a b.tar.gz"] + `shouldBe` [] + it "sanitizes any name to a real file name" $ \_ -> + filter (not . sanitized) fileNames `shouldBe` [] + it "sanitizes to a name with no directory components" $ \_ -> + filter (not . bareName) fileNames `shouldBe` [] xdescribe "No compression" $ aroundWith (. ((False, False),)) runRemoteTests xdescribe "Mobile offers compression" $ aroundWith (. ((True, False),)) runRemoteTests xdescribe "Desktop offers compression" $ aroundWith (. ((False, True),)) runRemoteTests describe "With compression" $ aroundWith (. ((True, True),)) runRemoteTests + where + fileNames :: [FilePath] + fileNames = ["", ".", "..", "...", "../x", "../../etc/passwd", "/etc/cron.d/x", "a/b", "x/", "test.pdf", ".hidden", "a b.tar.gz"] + sanitized n = let n' = safeFileNameStr n in n' /= "" && n' /= "." && n' /= ".." + bareName n = let n' = safeFileNameStr n in n' == takeFileName n' runRemoteTests :: SpecWith ((Bool, Bool), TestParams) runRemoteTests = do @@ -243,8 +262,9 @@ remoteStoreFileTest = contactBob desktop bob rhs <- readTVarIO (Controller.remoteHostSessions $ chatController desktop) - desktopHostStore <- case M.lookup (RHId 1) rhs of - Just (_, RHSessionConnected {storePath}) -> pure $ desktopHostFiles storePath remoteFilesFolder + (rhClient, desktopHostStore) <- case M.lookup (RHId 1) rhs of + Just (_, RHSessionConnected {rhClient, storePath}) -> + pure (rhClient, desktopHostFiles storePath remoteFilesFolder) _ -> fail "Host session 1 should be started" desktop ##> "/store remote file 1 tests/fixtures/test.pdf" desktop <## "file test.pdf stored on remote host 1" @@ -261,6 +281,17 @@ remoteStoreFileTest = chatReadFile (mobileFiles "test_2.pdf") (strEncode key) (strEncode nonce) `shouldReturn` Right (LB.fromStrict src) chatReadFile (desktopHostStore "test_2.pdf") (strEncode key) (strEncode nonce) `shouldReturn` Right (LB.fromStrict src) + -- the host rejects a traversal name before draining the attachment; only calling the protocol + -- directly can put such a name on the wire, as /store remote file sanitizes it controller-side + runExceptT (remoteStoreFile rhClient "tests/fixtures/test.pdf" "../x") >>= \case + Left (RPEInvalidBody _) -> pure () + r -> fail $ "expected RPEInvalidBody, got " <> show r + doesFileExist "./tests/tmp/x" `shouldReturn` False + -- the undrained attachment did not break the session + desktop ##> "/store remote file 1 tests/fixtures/test.pdf" + desktop <## "file test_3.pdf stored on remote host 1" + B.readFile (mobileFiles "test_3.pdf") `shouldReturn` src + removeFile (desktopHostStore "test_1.pdf") removeFile (desktopHostStore "test_2.pdf") diff --git a/website/langs/en.json b/website/langs/en.json index 3cbcb12ee2..482fe50042 100644 --- a/website/langs/en.json +++ b/website/langs/en.json @@ -264,6 +264,8 @@ "index-hero-h1": "Be
Free", "index-hero-h2": "In Your Network", "index-hero-p1": "The first network without user IDs.
You own your contacts, groups and channels.", + "index-hero-invest": "Invest in SimpleX Chat.", + "index-hero-invest-cta": "Learn more on Wefunder.", "index-hero-download-desktop-btn-title": "Download SimpleX Desktop App", "index-testflight-title": "SimpleX iOS beta-release on TestFlight", "index-f-droid-title": "SimpleX app via F-Droid", diff --git a/website/src/css/design3.css b/website/src/css/design3.css index ea6f20273b..4b645804eb 100644 --- a/website/src/css/design3.css +++ b/website/src/css/design3.css @@ -434,6 +434,16 @@ section.cover div.content p { max-width: calc(var(--sec-vwu) * 53); } +section.cover div.content p.invest { + font-weight: 300; +} + +section.cover div.content p.invest a { + font-weight: 500; + text-decoration: none; + white-space: nowrap; +} + .publications-btns { position: absolute; bottom: 24px; @@ -960,7 +970,7 @@ main .section-bg { @media (max-width: 959px) { section.cover div.content { gap: calc(var(--sec-vhu) * 2.5); - transform: translateY(calc(var(--sec-vhu) * 5)); + transform: translateY(calc(var(--sec-vhu) * 3)); } .publications-btns, @@ -994,6 +1004,14 @@ main .section-bg { max-width: calc(var(--sec-vwu) * 93); } + section.cover div.content p.invest { + font-weight: 400; + } + + section.cover div.content p.invest a { + font-weight: 600; + } + /* --- MAIN SECTIONS --- */ .page .text-container { justify-content: flex-end; diff --git a/website/src/index.html b/website/src/index.html index ba4494ab1d..3ce15f6d56 100644 --- a/website/src/index.html +++ b/website/src/index.html @@ -100,6 +100,7 @@ active_home: true

{{ "index-hero-h1" | i18n({}, lang) | safe }}

{{ "index-hero-h2" | i18n({}, lang) | safe }}

{{ "index-hero-p1" | i18n({}, lang) | safe }}

+

{{ "index-hero-invest" | i18n({}, lang) | safe }} {{ "index-hero-invest-cta" | i18n({}, lang) | safe }}