mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-09-19 04:59:59 +00:00
On Android the rendered name IS the announced name, so never strip
Follow-up to the review finding about suffix stripping. I said I would bring the iOS fix across; checking how this app actually builds a row name says the answer here is different, and simpler. iOS appends "#" plus four hex of the peerID in PeerDisplayNameResolver, so a row there genuinely carries a synthetic decoration and the fix is to remove exactly that suffix and no other — which needs the peerID, since the string alone cannot distinguish a decoration from an announced name. Nothing decorates a mesh nickname here. splitSuffix only separates a suffix the peer itself announced, showHashSuffix decides whether to display it, and the one place a #abcd is synthesised is GeohashPeopleList for Nostr people — which isPeerVerified refuses outright. So a mesh row shows what was announced, and the undecorate step I added was a fail-OPEN: pinned "medic", row shows "medic#cafe", suffix removed, seal kept. Exactly the case the review raised, still live on the rendered path after I had "fixed" it. sealAppliesToRendered now defers to sealAppliesToAnnounced. It stays a separate named function so there is one place to change if a row here ever does start carrying a decoration, and so nobody reads one comparison as the two questions having merged — collapsing them is what caused this in the first place. Mutation-checked: put the stripping back and two cases fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
a01fff5aed
commit
8d656242cc
@ -88,24 +88,29 @@ object NicknameBinding {
|
||||
|
||||
/**
|
||||
* Does a seal earned under [pinned] still apply beside [rendered] — a name
|
||||
* as it appears on a row, which the list may have decorated?
|
||||
* as it appears on a row?
|
||||
*
|
||||
* Two peers claiming one nickname are told apart with a `#abcd` the UI
|
||||
* appends, so comparing the decorated string alone would drop the seal of
|
||||
* the peer being impersonated at exactly the moment it matters. A raw match
|
||||
* is tried first, so a name that genuinely ends in a suffix still matches
|
||||
* itself; only then is one suffix removed.
|
||||
* On this platform that is the same question as [sealAppliesToAnnounced],
|
||||
* and the answer is the same comparison, because **nothing decorates a mesh
|
||||
* nickname here.** `splitSuffix` only separates a suffix the peer itself
|
||||
* announced, and `showHashSuffix` decides whether to display it; the one
|
||||
* place a `#abcd` is synthesised is `GeohashPeopleList`, for Nostr people,
|
||||
* and `isPeerVerified` refuses those outright. iOS is the one that appends
|
||||
* `#` plus four hex of the peerID in `PeerDisplayNameResolver`, and it has
|
||||
* to remove exactly that suffix and no other.
|
||||
*
|
||||
* This is deliberately a SECOND function rather than a flag. The first
|
||||
* version of this patch had one, stripping unconditionally, and it was
|
||||
* wrong in both directions at once — which is what happens when one
|
||||
* predicate is asked two questions.
|
||||
* It is kept as a separate function anyway, named for the question it
|
||||
* answers, so that the day a row here does start carrying a decoration
|
||||
* there is one place to change — and so nobody reads the single comparison
|
||||
* as the two questions having merged.
|
||||
*
|
||||
* The first version of this stripped any trailing `#` plus four hex. That
|
||||
* is a fail-OPEN on this platform: a key pinned as `medic` renaming to
|
||||
* `medic#cafe` had the suffix removed and kept its seal, and `#cafe` reads
|
||||
* as the disambiguator the app generates elsewhere, which is a better
|
||||
* disguise than an unrelated name rather than a worse one.
|
||||
*/
|
||||
fun sealAppliesToRendered(pinned: String?, rendered: String?): Boolean {
|
||||
if (pinned.isNullOrEmpty() || rendered.isNullOrEmpty()) return true
|
||||
if (sameName(pinned, rendered)) return true
|
||||
val undecorated = withoutCollisionSuffix(rendered)
|
||||
if (undecorated.isEmpty()) return true
|
||||
return sameName(pinned, undecorated)
|
||||
}
|
||||
fun sealAppliesToRendered(pinned: String?, rendered: String?): Boolean =
|
||||
sealAppliesToAnnounced(pinned, rendered)
|
||||
|
||||
}
|
||||
|
||||
@ -75,7 +75,7 @@ class NicknameBindingTest {
|
||||
// Two peers claiming one nickname render as "medic#a1b2" and
|
||||
// "medic#c3d4". Comparing the decorated string would drop the seal of
|
||||
// the peer being impersonated, at exactly the moment it matters most.
|
||||
assertTrue(NicknameBinding.sealAppliesToRendered("medic", "medic#a1b2"))
|
||||
assertTrue(NicknameBinding.sealAppliesToAnnounced("medic", "medic"))
|
||||
assertEquals("medic", NicknameBinding.withoutCollisionSuffix("medic#a1b2"))
|
||||
}
|
||||
|
||||
@ -99,7 +99,7 @@ class NicknameBindingTest {
|
||||
// mention, wrong for comparing a name, since "ravi@hq" would then
|
||||
// compare unequal to itself.
|
||||
assertTrue(NicknameBinding.sealAppliesToAnnounced("ravi@hq", "ravi@hq"))
|
||||
assertTrue(NicknameBinding.sealAppliesToRendered("ravi@hq", "ravi@hq#a1b2"))
|
||||
assertTrue(NicknameBinding.sealAppliesToAnnounced("ravi@hq", "ravi@hq"))
|
||||
assertFalse(NicknameBinding.sealAppliesToAnnounced("ravi@hq", "ravi"))
|
||||
}
|
||||
|
||||
@ -119,11 +119,10 @@ class NicknameBindingTest {
|
||||
// A row with no name to show yet is not evidence of a rename.
|
||||
assertTrue(NicknameBinding.sealAppliesToAnnounced("medic", null))
|
||||
assertTrue(NicknameBinding.sealAppliesToAnnounced("medic", ""))
|
||||
// A row whose name is nothing BUT a decoration leaves nothing to
|
||||
// compare, so it fails open too. Note this is a RENDERED-name case: a
|
||||
// peer that actually announces "#a1b2" has renamed, and says so.
|
||||
assertTrue(NicknameBinding.sealAppliesToRendered("medic", "#a1b2"))
|
||||
// A peer that announces "#a1b2" has renamed, and says so — on either
|
||||
// path, since a mesh row shows what was announced.
|
||||
assertFalse(NicknameBinding.sealAppliesToAnnounced("medic", "#a1b2"))
|
||||
assertFalse(NicknameBinding.sealAppliesToRendered("medic", "#a1b2"))
|
||||
}
|
||||
|
||||
// ---- announced vs rendered: the split that was missing -----------------
|
||||
@ -144,11 +143,12 @@ class NicknameBindingTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a rendered row tries the raw name before undecorating it`() {
|
||||
// On a row the list may have appended #abcd to tell two namesakes
|
||||
// apart, so that has to come off — but only after the raw name has had
|
||||
// its chance, or a name that genuinely ends in a suffix loses its seal.
|
||||
assertTrue(NicknameBinding.sealAppliesToRendered("medic", "medic#a1b2"))
|
||||
fun `a rendered mesh row carries no decoration to remove`() {
|
||||
// Nothing decorates a mesh nickname on this platform: splitSuffix only
|
||||
// separates a suffix the peer announced, and the one synthesised #abcd
|
||||
// is for Nostr people, which isPeerVerified refuses outright. So a row
|
||||
// name IS the announced name, and stripping would be a fail-open.
|
||||
assertFalse(NicknameBinding.sealAppliesToRendered("medic", "medic#cafe"))
|
||||
assertTrue(NicknameBinding.sealAppliesToRendered("medic#cafe", "medic#cafe"))
|
||||
assertFalse(NicknameBinding.sealAppliesToRendered("medic", "zebra#a1b2"))
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user