diff --git a/src/proto-p10.c b/src/proto-p10.c index 44a7cb9..28b546f 100644 --- a/src/proto-p10.c +++ b/src/proto-p10.c @@ -388,8 +388,13 @@ GetServerN(const char *numeric) } } -struct userNode* -GetUserN(const char *numeric) /* using numeric */ +/* Shared internal numeric-to-userNode lookup. Wraps both the noisy + * GetUserN (callers expect success — a miss is bug-worthy) and the + * quiet GetUserN_silent (callers know the target can legitimately be + * absent — e.g., snoop/track on transient targets, BX P probing for + * the alias/primary pair). */ +static struct userNode * +GetUserN_impl(const char *numeric, int quiet) { struct userNode *un; struct server *s; @@ -397,26 +402,46 @@ GetUserN(const char *numeric) /* using numeric */ switch (strlen(numeric)) { default: - log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too long!", numeric); + if (!quiet) + log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too long!", numeric); return NULL; case 5: slen = 2; ulen = 3; break; case 4: slen = 1; ulen = 3; break; case 3: slen = 1; ulen = 2; break; case 2: case 1: case 0: - log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too short!", numeric); + if (!quiet) + log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too short!", numeric); return NULL; } if (!(s = servers_num[base64toint(numeric, slen)])) { - log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): couldn't find server (len=%d)!", numeric, slen); + if (!quiet) + log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): couldn't find server (len=%d)!", numeric, slen); return NULL; } n = base64toint(numeric+slen, ulen) & s->num_mask; if (!(un = s->users[n])) { - log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s) couldn't find user!", numeric); + if (!quiet) + log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s) couldn't find user!", numeric); } return un; } +struct userNode* +GetUserN(const char *numeric) /* using numeric */ +{ + return GetUserN_impl(numeric, 0); +} + +/* Same as GetUserN but suppresses all warnings on lookup miss. Use + * for callers that legitimately probe numerics which may not exist + * (snoop / track tracking transient PRIVMSG/NOTICE targets, BX P + * lookup probes during alias reconciliation, etc.). */ +struct userNode* +GetUserN_silent(const char *numeric) +{ + return GetUserN_impl(numeric, 1); +} + extern struct userNode *opserv; static void check_ctcp(struct userNode *user, struct userNode *bot, char *text, UNUSED_ARG(int server_qualified)) @@ -1735,22 +1760,113 @@ static CMD_FUNC(cmd_bouncer_transfer) if (argc < 6) return 0; - old_primary = GetUserN(argv[2]); - new_node = GetUserN(argv[3]); + /* Both lookups can legitimately miss — old_primary may have + * already been cleaned up by a prior event, new_node is + * normally absent (the swap path is the common case) but + * may exist for the in-place-conversion / merge case below. + * Use the silent variant so neither probe spams the snoop + * channel. */ + old_primary = GetUserN_silent(argv[2]); + new_node = GetUserN_silent(argv[3]); if (!old_primary) return 1; /* Already gone, nothing to do */ - /* Numeric swap: move old_primary's numeric routing to the new - * server/slot. The userNode keeps its nick, clients dict entry, - * handle_info, channels — only the P10 numeric changes. + /* Two BX P shapes need handling: + * + * 1. **Numeric swap (new_node absent)** — classic promote-or- + * transfer. Move old_primary's numeric routing to the new + * server/slot. The userNode keeps its nick, clients dict + * entry, handle_info, channels — only the P10 numeric changes. + * This is the case the original handler was written for, and + * is what fires when nefarious never bursts an alias to us. * - * Nefarious never bursts aliases as N tokens (they're introduced - * via BX C only), so X3 won't have a node for the alias numeric. - * If new_node somehow exists (shouldn't happen), log and ignore. */ + * 2. **In-place conversion / merge (new_node exists with same + * account)** — modern fork peers emit BX P for the case where + * an N-introduced client (old) is being absorbed into an + * existing primary (new), e.g. burst-ordering caused us to + * receive N for the would-be-alias before its BX C. Both + * nodes exist on X3. We merge: delete old_primary's + * userNode (channels and dict entry cleaned up via DelUser, + * no QUIT broadcast), keep new_node intact as the surviving + * identity. Gate strictly on same-handle to avoid + * accidentally merging unrelated collisions. + * + * 3. **new_node exists but different/no handle** — genuinely + * unexpected. Keep the original "log and ignore" behaviour + * so we don't silently corrupt state across an account + * mismatch. */ if (new_node) { + if (old_primary->handle_info + && old_primary->handle_info == new_node->handle_info) { + /* Merge: old absorbed into new. Before deleting + * old_primary, transfer its channel memberships onto + * new_node. This mirrors nefarious's own both-exist + * swap semantics (upstream m_bouncer_transfer.c:88-113, + * fork bouncer_session.c:7070-7078): the ghost's + * channels must survive on the merged identity. + * + * Ordering is load-bearing: this MUST run before + * DelUser(). DelUser's own cleanup loop + * (DelChannelUser(..., NULL, 0)) lets an unregistered + * channel self-destruct when its last member leaves + * (see DelChannelUser's tail, hash.c). If old_primary + * were deleted first while still holding memberships + * new_node lacks, any unregistered channel where + * old_primary was the only member in X3's view would + * be destroyed here — even though the surviving + * identity (and the rest of the network) is still in + * it. Transferring first means old_primary's channel + * list is already empty by the time DelUser runs, so + * that loop body never executes. + * + * Walk old_primary->channels the same way DelUser's + * loop does: always pop the last element, since + * AddChannelUser/DelChannelUser mutate both the + * channel's member list and the user's channel list + * out from under us. */ + while (old_primary->channels.used > 0) { + struct modeNode *mn = old_primary->channels.list[old_primary->channels.used - 1]; + struct chanNode *chan = mn->channel; + + if (!GetUserMode(chan, new_node)) { + /* AddChannelUser() only fires irc_join() when + * the joining user IsLocal() (i.e. an X3 + * service bot) — new_node here is always a + * network user, so no JOIN hits the wire. It + * unconditionally runs call_join_funcs() + * though, which fires the same on-join hooks + * (chanserv presence/ban/oplevel bookkeeping) + * a real join would. The numeric-swap path + * above never touches channel membership at + * all, so there's no existing both-exist + * precedent that's hook-free; there's no + * lower-level "add to channel, no hooks" + * primitive to reach for instead. Correctness + * of membership state takes priority, so we + * accept the hook firing here as a known + * side effect rather than leaving the ghost's + * channels to be silently dropped. */ + struct modeNode *new_mn = AddChannelUser(new_node, chan); + new_mn->modes = mn->modes; + new_mn->oplevel = mn->oplevel; + } + + /* No announce (reason NULL), same call shape + * DelUser's own loop uses: this is internal + * bookkeeping, not a real part. */ + DelChannelUser(old_primary, chan, NULL, 0); + } + + /* DelUser with announce=0 suppresses both QUIT and KILL + * emission — this is internal cleanup, the network + * isn't supposed to see the alias's identity leave. */ + DelUser(old_primary, NULL, 0, "Bouncer transfer"); + return 1; + } log_module(MAIN_LOG, LOG_WARNING, - "BX P: new_node %s already exists as %s — ignoring promote", + "BX P: new_node %s already exists as %s with mismatched " + "handle — ignoring promote", argv[3], new_node->nick); return 1; } diff --git a/src/proto.h b/src/proto.h index 75a5e24..cf27204 100644 --- a/src/proto.h +++ b/src/proto.h @@ -84,6 +84,7 @@ struct cManagerNode #ifdef WITH_PROTOCOL_P10 struct server* GetServerN(const char *numeric); struct userNode* GetUserN(const char *numeric); +struct userNode* GetUserN_silent(const char *numeric); #endif /* Basic protocol parsing support. */