diff --git a/apps/ios/Shared/Views/NewChat/QRCodeScan.swift b/apps/ios/Shared/Views/NewChat/QRCodeScan.swift index f7227be20c..e0f68484eb 100644 --- a/apps/ios/Shared/Views/NewChat/QRCodeScan.swift +++ b/apps/ios/Shared/Views/NewChat/QRCodeScan.swift @@ -32,16 +32,18 @@ private func showWrongQRCodeAlert(_ scanned: QRCodeType, expected: QRCodeKind, t )) return } - // Unrecognised: we cannot name the kind. Keep the existing "invalid" wording, expected-aware - // (the server screen keeps its own, more specific "Invalid server address!" title). + // Unrecognised: we cannot name the kind. Keep each screen's own existing "invalid" wording — + // the server screen says "Invalid server address!", the verify screen says "Incorrect security + // code!" (it scans a code, not a link), everything else the generic "not a SimpleX link". if case .unknown = scanned { - let forServer = expected == .serverAddress - AlertManager.shared.showAlert(mkAlert( - title: forServer ? "Invalid server address!" : "Invalid QR code", - message: forServer - ? "Check server address and try again." - : "The code you scanned is not a SimpleX link QR code." - )) + switch expected { + case .serverAddress: + AlertManager.shared.showAlert(mkAlert(title: "Invalid server address!", message: "Check server address and try again.")) + case .securityCode: + AlertManager.shared.showAlert(mkAlert(title: "Incorrect security code!")) + default: + AlertManager.shared.showAlert(mkAlert(title: "Invalid QR code", message: "The code you scanned is not a SimpleX link QR code.")) + } return } // Recognised wrong kind: "\n\n". @@ -74,8 +76,7 @@ private extension QRCodeType { var kindName: String { switch self { case let .connectionLink(_, linkType): - if let lt = linkType { return String(format: NSLocalizedString("This is a %@.", comment: "qr type"), lt.description) } - return NSLocalizedString("This is a SimpleX address.", comment: "qr type") + return String.localizedStringWithFormat(NSLocalizedString("This is a %@.", comment: "qr type"), linkType.description) case .serverAddress: return NSLocalizedString("This is a SimpleX server address.", comment: "qr type") case .migrationLink: return NSLocalizedString("This is a link to migrate to another device.", comment: "qr type") case .desktopAddress: return NSLocalizedString("This is an address for linking a mobile to a SimpleX desktop app.", comment: "qr type") diff --git a/apps/ios/Shared/Views/NewChat/QRCodeType.swift b/apps/ios/Shared/Views/NewChat/QRCodeType.swift index fed3fb5488..821f60a211 100644 --- a/apps/ios/Shared/Views/NewChat/QRCodeType.swift +++ b/apps/ios/Shared/Views/NewChat/QRCodeType.swift @@ -21,7 +21,7 @@ enum QRCodeKind { // One unified type for every kind of QR code (or pasted string) the app can scan. enum QRCodeType { - case connectionLink(text: String, linkType: SimplexLinkType?) + case connectionLink(text: String, linkType: SimplexLinkType) case serverAddress(text: String) case migrationLink(text: String) case desktopAddress(text: String) @@ -62,12 +62,13 @@ func strHasSimplexFileLink(_ text: String) -> Bool { // Classify a scanned/pasted string by composing the existing local parsers in priority // order, returning the first match, else `.unknown`. func parseQRCode(_ raw: String) -> QRCodeType { - let t = raw.trimmingCharacters(in: .whitespaces) + let t = raw.trimmingCharacters(in: .whitespacesAndNewlines) if strHasSimplexFileLink(t) { return .migrationLink(text: t) } if t.hasPrefix(desktopAddressScheme) { return .desktopAddress(text: t) } - // Match only when the WHOLE string is exactly one SimpleX link (parity with Kotlin's - // parseToMarkdown size == 1, and with the prior strIsSimplexLink check) — not merely one - // link embedded among other text, which strHasSingleSimplexLink would accept. + // Match only when the WHOLE string is exactly one SimpleX link — same as `strIsSimplexLink` + // and the master connect scanner — not a link embedded among other text. A SimpleX *name* + // (Format.simplexName) is therefore not a connectionLink here; it falls through to .unknown, + // exactly as the master scanner treats it (names are handled only by the paste path). if let md = parseSimpleXMarkdown(t), md.count == 1, case let .simplexLink(_, linkType, _, _) = md[0].format { return .connectionLink(text: t, linkType: linkType) @@ -77,12 +78,15 @@ func parseQRCode(_ raw: String) -> QRCodeType { return .unknown(text: t) } -// A contact's security / verification code QR encodes the raw code with no scheme. The core -// builds it as `verificationCode = T.pack . unwords . chunks 5 . show . os2ip` — decimal digits -// of a SHA-256 integer, in groups of 5 separated by spaces (e.g. "61889 38426 ... 25"). Recognise -// it by shape: ≥32 ASCII decimal digits once the grouping whitespace is stripped. Checked last, -// so real links/addresses never reach here; the scanned text keeps its spaces, so verifyCode matches. +// A contact's security / verification code QR encodes the raw code with no scheme. The core builds +// it as `verificationCode = T.pack . unwords . chunks 5 . show . os2ip` — decimal digits of a SHA +// integer, in groups (of 5) separated by spaces (e.g. "61889 38426 ... 25"). Recognise it by that +// exact shape: 2+ whitespace-separated groups of ASCII decimal digits, 32+ digits total. Requiring +// the grouping keeps a bare long number from being mistaken for a code. Checked last; the scanned +// text keeps its spaces, so verifyCode still matches. func isSecurityCode(_ t: String) -> Bool { - let digits = t.filter { !$0.isWhitespace } - return digits.count >= 32 && digits.allSatisfy { $0.isNumber && $0.isASCII } + let groups = t.split(whereSeparator: { $0.isWhitespace }) + return groups.count >= 2 + && groups.reduce(0) { $0 + $1.count } >= 32 + && groups.allSatisfy { $0.allSatisfy { $0.isNumber && $0.isASCII } } } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/QRCodeScan.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/QRCodeScan.kt index 9d71706fb9..20ffc4135f 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/QRCodeScan.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/QRCodeScan.kt @@ -37,14 +37,23 @@ private fun showWrongQRCodeAlert(rhId: Long?, scanned: QRCodeType, expected: KCl ) return } - // Unrecognised: we cannot name the kind. Keep the existing "invalid" wording, expected-aware - // (the server screen keeps its own, more specific "Invalid server address!" title). + // Unrecognised: we cannot name the kind. Keep each screen's own existing "invalid" wording — + // the server screen says "Invalid server address!", the verify screen says "Incorrect security + // code!" (it scans a code, not a link), everything else the generic "not a SimpleX link". if (scanned is QRCodeType.Unknown) { - val forServer = expected == QRCodeType.ServerAddress::class - AlertManager.shared.showAlertMsg( - title = generalGetString(if (forServer) MR.strings.smp_servers_invalid_address else MR.strings.invalid_qr_code), - text = generalGetString(if (forServer) MR.strings.smp_servers_check_address else MR.strings.code_you_scanned_is_not_simplex_link_qr_code), - ) + when (expected) { + QRCodeType.ServerAddress::class -> AlertManager.shared.showAlertMsg( + title = generalGetString(MR.strings.smp_servers_invalid_address), + text = generalGetString(MR.strings.smp_servers_check_address), + ) + QRCodeType.SecurityCode::class -> AlertManager.shared.showAlertMsg( + title = generalGetString(MR.strings.incorrect_code), + ) + else -> AlertManager.shared.showAlertMsg( + title = generalGetString(MR.strings.invalid_qr_code), + text = generalGetString(MR.strings.code_you_scanned_is_not_simplex_link_qr_code), + ) + } return } // Recognised wrong kind: "\n\n". @@ -72,7 +81,7 @@ private fun showWrongQRCodeAlert(rhId: Long?, scanned: QRCodeType, expected: KCl // runs into the instruction below). Reuses the existing link-type labels for connection links. private val QRCodeType.description: String get() = when (this) { - is QRCodeType.ConnectionLink -> linkType?.let { String.format(generalGetString(MR.strings.qr_type_connection), it.description) } ?: generalGetString(MR.strings.qr_type_simplex_address) + is QRCodeType.ConnectionLink -> String.format(generalGetString(MR.strings.qr_type_connection), linkType.description) is QRCodeType.ServerAddress -> generalGetString(MR.strings.qr_type_server_address) is QRCodeType.MigrationLink -> generalGetString(MR.strings.qr_type_migration_link) is QRCodeType.DesktopAddress -> generalGetString(MR.strings.qr_type_desktop_address) diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/QRCodeType.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/QRCodeType.kt index 189cee32d2..1563e30c59 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/QRCodeType.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/QRCodeType.kt @@ -11,8 +11,9 @@ import chat.simplex.common.views.migration.strHasSimplexFileLink sealed class QRCodeType { abstract val text: String - // A SimpleX connection link (contact / invitation / group / channel / relay). - data class ConnectionLink(override val text: String, val linkType: SimplexLinkType?): QRCodeType() + // A SimpleX connection link (contact / invitation / group / channel / relay). The link parser + // always yields a non-null linkType (there is no address-name case on master). + data class ConnectionLink(override val text: String, val linkType: SimplexLinkType): QRCodeType() // A SMP/XFTP chat server address (the UserServer is built by the caller that has rhId). data class ServerAddress(override val text: String): QRCodeType() // A database migration file link (move a profile to this device). @@ -53,12 +54,14 @@ fun parseQRCode(raw: String): QRCodeType { } // A contact's security / verification code QR encodes the raw code with no scheme. The core -// builds it as `verificationCode = T.pack . unwords . chunks 5 . show . os2ip` — decimal digits -// of a SHA-256 integer, in groups of 5 separated by spaces (e.g. "61889 38426 ... 25"). Recognise -// it by shape: ≥32 decimal digits once the grouping whitespace is stripped (no hex — `show` only -// emits decimal). Checked last (after every real link/address parser), so those never reach here; -// the scanned text keeps its spaces, so verifyCode still matches. +// builds it as `verificationCode = T.pack . unwords . chunks 5 . show . os2ip` — decimal digits of +// a SHA integer, in groups (of 5) separated by spaces (e.g. "61889 38426 ... 25"). Recognise it by +// that exact shape: 2+ whitespace-separated groups of decimal digits, 32+ digits total. Requiring +// the grouping keeps a bare long number from being mistaken for a code. Checked last (after every +// real link/address parser); the scanned text keeps its spaces, so verifyCode still matches. internal fun isSecurityCode(t: String): Boolean { - val digits = t.filterNot { it.isWhitespace() } - return digits.length >= 32 && digits.all { it in '0'..'9' } + val groups = t.trim().split(Regex("\\s+")) + return groups.size >= 2 && + groups.sumOf { it.length } >= 32 && + groups.all { g -> g.all { it in '0'..'9' } } } diff --git a/apps/multiplatform/common/src/commonMain/resources/MR/base/strings.xml b/apps/multiplatform/common/src/commonMain/resources/MR/base/strings.xml index c2c9b2e90d..1cab8d4545 100644 --- a/apps/multiplatform/common/src/commonMain/resources/MR/base/strings.xml +++ b/apps/multiplatform/common/src/commonMain/resources/MR/base/strings.xml @@ -773,7 +773,6 @@ Wrong QR code This is a %s. - This is a SimpleX address. This is a SimpleX server address. This is a link to migrate to another device. This is an address for linking a mobile to a SimpleX desktop app. diff --git a/apps/multiplatform/common/src/desktopTest/kotlin/chat/simplex/app/ParseQRCodeTest.kt b/apps/multiplatform/common/src/desktopTest/kotlin/chat/simplex/app/ParseQRCodeTest.kt index e7c925a124..4e3fa73be4 100644 --- a/apps/multiplatform/common/src/desktopTest/kotlin/chat/simplex/app/ParseQRCodeTest.kt +++ b/apps/multiplatform/common/src/desktopTest/kotlin/chat/simplex/app/ParseQRCodeTest.kt @@ -56,9 +56,15 @@ class ParseQRCodeTest { assertFalse(isSecurityCode("a3f5".repeat(16))) } + @Test + fun securityCode_ungroupedLongNumber_isNotRecognised() { + // A bare long number (no space grouping) is not the verificationCode shape — must not match. + assertFalse(isSecurityCode("6188938426639340957696390793898412485253")) + } + @Test fun securityCode_shortNumber_isNotRecognised() { - assertFalse(isSecurityCode("12345 67890")) // only 10 digits once spaces are stripped + assertFalse(isSecurityCode("12345 67890")) // grouped, but only 10 digits total } @Test diff --git a/plans/plan-unify-qr-scanning-impl.md b/plans/plan-unify-qr-scanning-impl.md index f6ab9a597b..515b3a228a 100644 --- a/plans/plan-unify-qr-scanning-impl.md +++ b/plans/plan-unify-qr-scanning-impl.md @@ -48,13 +48,17 @@ when in doubt about behaviour, that doc is authoritative. ## As-built deltas vs this plan (branch `nd/qr-scan-clarity`, off `master`) -The plan was verified against `stable`; the branch is off `master`, which is **behind** stable. -The code therefore differs from the plan text in these (intentional, documented) ways: +The branch is rebased onto current `master` (which has caught up to `stable` — it now has +`strConnectTarget`/`ConnectTarget`/SimplexName). The code differs from the original plan text in +these (intentional, documented) ways: -1. **Classifier (master):** no `strConnectTarget`/`ConnectTarget`/SimplexName on master, so - `parseQRCode` uses `parseToMarkdown` (`size == 1`) + `Format.SimplexLink.linkType` — see the - updated C1.0 above. There is **no address-name case** on master, so `ConnectionLink.linkType` - is always non-null and ConnectView gains no "names now accepted" improvement. +1. **Classifier uses `parseToMarkdown`, not `strConnectTarget`.** `parseQRCode` matches a + connection link with `parseToMarkdown` (`size == 1`) + `Format.SimplexLink.linkType` — this is + exactly master's own scanner check (`strIsSimplexLink`). A SimpleX **name** therefore does NOT + match (it is `Format.SimplexName`, not `Format.SimplexLink`), so it classifies as `Unknown`, + the same "not a SimpleX link" the master connect *scanner* shows today (names are handled only + by the **paste** path via `strConnectTarget`, untouched here). Consequently `ConnectionLink` + only ever wraps a real link, so `ConnectionLink.linkType` is **non-null** by construction. 2. **iOS uses a small `enum QRCodeKind`** (`connectionLink/serverAddress/migrationLink/ desktopAddress/code`) for `expected`, not a closure: Swift has no `KClass`, and the alert needs the expected kind for the Unknown-on-server message. This is the Code↔Unknown pairing @@ -104,7 +108,7 @@ is NOT compiled** (no Xcode here) — and the two new Swift files still need add ```kotlin sealed class QRCodeType { abstract val text: String - data class ConnectionLink(override val text: String, val linkType: SimplexLinkType?): QRCodeType() + data class ConnectionLink(override val text: String, val linkType: SimplexLinkType): QRCodeType() // non-null on master data class ServerAddress (override val text: String): QRCodeType() data class MigrationLink (override val text: String): QRCodeType() data class DesktopAddress(override val text: String): QRCodeType() @@ -128,7 +132,7 @@ fun parseQRCode(raw: String): QRCodeType { // no rhId; FFI parse on when { link != null -> QRCodeType.ConnectionLink(t, link.linkType) parseServerAddress(t) != null -> QRCodeType.ServerAddress(t) - isSecurityCode(t) -> QRCodeType.SecurityCode(t) // long hex/decimal digits, no scheme + isSecurityCode(t) -> QRCodeType.SecurityCode(t) // space-grouped decimal digits, no scheme else -> QRCodeType.Unknown(t) } } @@ -137,12 +141,12 @@ fun parseQRCode(raw: String): QRCodeType { // no rhId; FFI parse on // A contact's verification code QR is the raw code, no scheme. Core builds it as // `verificationCode = unwords . chunks 5 . show . os2ip` = DECIMAL digits grouped in 5s separated -// by SPACES, so strip whitespace, then require decimal digits only (show emits no hex). Checked -// last, so real links/addresses never reach here. NB: qr.text keeps the spaces, so verifyCode -// (core noSpaces-normalises) still matches. Canonical fixture: tests/ChatTests/Direct.hs. +// by SPACES. Match that exact shape — 2+ whitespace-separated groups of decimal digits, 32+ total +// — so a bare long number isn't mistaken for a code. Checked last; qr.text keeps the spaces, so +// verifyCode (core noSpaces-normalises) still matches. Canonical fixture: tests/ChatTests/Direct.hs. internal fun isSecurityCode(t: String): Boolean { - val digits = t.filterNot { it.isWhitespace() } - return digits.length >= 32 && digits.all { it in '0'..'9' } + val groups = t.trim().split(Regex("\\s+")) + return groups.size >= 2 && groups.sumOf { it.length } >= 32 && groups.all { g -> g.all { it in '0'..'9' } } } ``` - [x] **C1.1 — unit test `parseQRCode`** (load-bearing). One fixture per kind + garbage, and @@ -173,8 +177,9 @@ suspend fun handleScan( `showWrongQRCodeAlert(rhId, scanned, expected, close)` + the per-kind `description`/`whereToScan` metadata implement the design's message rules (recognised-wrong → `wrong_qr_code` + - `"\n\n"`; relay → `relay_address_alert_title` + `relay_address_alert_message`; Unknown → `invalid_qr_code` + - expected-aware msg; **no-user** → no button + `qr_where_no_user`). Connect button dismisses the + `"\n\n"`; relay → `relay_address_alert_title` + `relay_address_alert_message`; Unknown → + expected-aware: server → `smp_servers_invalid_address`, verify screen → `incorrect_code`, else + `invalid_qr_code`; **no-user** → no button + `qr_where_no_user`). Connect button dismisses the scanner first, then connects: `onConfirm = { close(); withBGApi { planAndConnect(rhId, qr.text, close = null) } }`. Nothing calls these yet. Build. @@ -260,7 +265,6 @@ a full `Alert(…primaryButton…)` for Connect; `ScanCodeView.verify` returns ` ``` wrong_qr_code = "Wrong QR code" qr_type_connection = "This is a %s." # %s = SimplexLinkType.description (e.g. "SimpleX group link") -qr_type_simplex_address = "This is a SimpleX address." qr_type_server_address = "This is a SimpleX server address." qr_type_migration_link = "This is a link to migrate to another device." qr_type_desktop_address = "This is an address for linking a mobile to a SimpleX desktop app." diff --git a/plans/plan-unify-qr-scanning.md b/plans/plan-unify-qr-scanning.md index 6079c946ee..6148217da3 100644 --- a/plans/plan-unify-qr-scanning.md +++ b/plans/plan-unify-qr-scanning.md @@ -6,14 +6,13 @@ verified constraints. Exact code, signatures, string keys/values, and the commit Cx / Appendix A"). When the two disagree, the impl plan wins on code/strings; this doc wins on intent. -> **Branch note (stable vs master).** This design was verified against `stable`; the -> implementation branch is off `master`, which is **behind** stable. On master there is **no** -> `strConnectTarget` / `ConnectTarget` / SimplexName, and short-link classification goes through -> `parseToMarkdown` (size==1 → `Format.SimplexLink`), iOS through `strHasSingleSimplexLink`. So -> wherever the sections below name `strConnectTarget`/`ConnectTarget`/`simplexName` or cite a -> `file:line` (source map, type model, navigation/adversarial reviews), read them as the *stable* -> reference that motivated the design; the **authoritative master mapping is the impl plan's -> "As-built deltas vs this plan"** section. Name-link cases are **N/A on master**. +> **Branch note.** This branch is rebased onto current `master`, which has caught up to `stable` +> (it now has `strConnectTarget` / `ConnectTarget` / SimplexName). The scanner classifier uses +> `parseToMarkdown` (size==1 → `Format.SimplexLink`) — equivalent to master's own scanner check +> `strIsSimplexLink` — so a SimpleX **name** QR classifies as `Unknown` ("not a SimpleX link"), +> exactly as master's connect *scanner* behaves today. Names are recognised only by the **paste** +> path (`strConnectTarget` → "unsupported name" alert), which this change does not touch, so there +> is **no scanner regression**. `file:line` references in the source map / reviews below are approximate. ## Goal @@ -82,7 +81,8 @@ genuinely-unrecognised case (a non-SimpleX QR, or garbage). Six variants, each carrying the scanned `text`: - **ConnectionLink** — also carries the link sub-type (contact / invitation / group / channel / - relay), or none for a SimpleX address-*name*. + relay). On master the parser always yields a non-null sub-type (no address-*name* case — that + exists only on stable), so the field is non-nullable here. - **ServerAddress** — `text` only; the `UserServer` is built later in `onMatch`, so the classifier needs no `rhId`. - **MigrationLink**, **DesktopAddress** — `text` only.