From fe6a8c91521a958a08fb81513234b07118be6c4c Mon Sep 17 00:00:00 2001 From: Marcello Barnaba Date: Tue, 21 Apr 2026 08:26:16 +0000 Subject: [PATCH] fix: ChanServ no longer sets TOPIC twice after a netsplit (issue #1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a remote server finished bursting after a netsplit heal and the affected channels had CI_KEEPTOPIC + CI_TOPICLOCK set, ChanServ emitted two back-to-back TOPIC commands for each such channel: 1) chan_handle_SJOIN called restore_topic() the moment the channel was re-created (newChannel == TRUE + synched == TRUE). 2) The immediately-following TOPIC propagated from the same bursting server fell into chan_handle_TOPIC's server-source branch, which — gated only on synched == TRUE — overrode chan->topic with ci->last_topic and emitted a second ChanServ TOPIC with identical content. Services already track per-server burst state via SERVER_FLAG_BURSTING (set in servers_SERVER_create / server_handle_SERVER, cleared at m_gnotice's "has synched to network data." match). The flag just wasn't consulted by the TOPIC path. Fix: * channels.c / chan_handle_SJOIN: skip restore_topic() while the originating server is still BURSTING. * channels.c / chan_handle_TOPIC: in the server-source branch, skip both the TOPICLOCK override (and its send_cmd TOPIC) and the trailing record_topic() while the originating server is BURSTING. chan->topic is still updated via the normal store path so the in-memory state stays consistent; ci->last_topic is preserved so the reconciliation pass has something to compare against. * messages.c / m_gnotice: after clearing SERVER_FLAG_BURSTING, call synch_topics() to resolve any ci->last_topic vs chan->topic mismatch accumulated during the burst with a single ChanServ TOPIC per channel — the same logic already used for services' own initial sync. Result: one TOPIC per channel per netsplit heal, matching rfc1459's original three-point critique on the issue. Fixes #1 --- src/channels.c | 32 ++++++++++++++++++++++++++------ src/messages.c | 6 ++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/channels.c b/src/channels.c index b5d4ff9..78a4c1b 100644 --- a/src/channels.c +++ b/src/channels.c @@ -714,8 +714,16 @@ void chan_handle_SJOIN(CSTR source, const int ac, char **av) { /* Restore the topic if it's a new channel, or if it has been reset, or if someone has just been kicked by services (AutoKick, Restrict, etc) and ChanServ is alone in the channel. */ - if (synched && (newChannel || resetTS || (FlagSet(chan->mode, CMODE_CS) && (chan->userCount == 1)))) - restore_topic(chan); + if (synched && (newChannel || resetTS || (FlagSet(chan->mode, CMODE_CS) && (chan->userCount == 1)))) { + + /* If this SJOIN is part of a remote server's burst (netsplit heal), defer the topic + restore until burst-end — see issue #1. The subsequent TOPIC message from the + same burst would otherwise trigger a second ChanServ TOPIC send. */ + Server *src_server = findserver(source); + + if (IS_NULL(src_server) || FlagUnset(src_server->flags, SERVER_FLAG_BURSTING)) + restore_topic(chan); + } } } } @@ -2481,6 +2489,7 @@ void chan_clear_bans(Channel *chan) { void chan_handle_TOPIC(const char *source, const int ac, char **av) { Channel *chan; + BOOL src_bursting = FALSE; ChannelStats *cs; @@ -2522,12 +2531,17 @@ void chan_handle_TOPIC(const char *source, const int ac, char **av) { /* This is a server topic. We are going to treat server topics just like normal topics during normal operation for now. */ - if (synched == TRUE) { + Server *src_server = findserver(source); + + if (IS_NOT_NULL(src_server) && FlagSet(src_server->flags, SERVER_FLAG_BURSTING)) + src_bursting = TRUE; + + if ((synched == TRUE) && !src_bursting) { ChannelInfo *ci = chan->ci; if (IS_NOT_NULL(ci) && FlagSet(ci->flags, CI_TOPICLOCK)) { - + /* This is a server topic and channel has topiclock on, block this TOPIC and replace it with ours (code taken from check_topiclock()) */ @@ -2547,6 +2561,10 @@ void chan_handle_TOPIC(const char *source, const int ac, char **av) { return; } } + + /* If src_bursting: fall through to the normal store path. The topic saved below reflects + the remote server's current topic; we defer the TOPICLOCK override (and record_topic) + to the burst-end pass so only one ChanServ TOPIC is emitted. See issue #1. */ } TRACE_MAIN(); @@ -2586,9 +2604,11 @@ void chan_handle_TOPIC(const char *source, const int ac, char **av) { TRACE_MAIN(); /* If we aren't synched this is a server topic, we save it in the channel struct but wait - for synch_topics() to record it if it is the case. */ + for synch_topics() to record it if it is the case. Similarly, when the source server is + still bursting (post-netsplit), defer the record until burst-end to preserve ci->last_topic + against the incoming remote topic. See issue #1. */ - if ((synched == TRUE) && IS_NOT_NULL(chan->ci)) + if ((synched == TRUE) && !src_bursting && IS_NOT_NULL(chan->ci)) record_topic(chan); } diff --git a/src/messages.c b/src/messages.c index 194ceac..842fb7e 100644 --- a/src/messages.c +++ b/src/messages.c @@ -705,6 +705,12 @@ static void m_gnotice(CSTR source, const int ac, char **av) { RemoveFlag(server->flags, SERVER_FLAG_BURSTING); LOG_SNOOP(s_Snooper, "Synched with \2%s\2%s [Users: %u]", server->name, FlagSet(server->flags, SERVER_FLAG_UPLINK) ? " [Uplink]" : s_NULL, server->userCount); + + /* Reconcile topics now that this server finished bursting. chan_handle_TOPIC and + chan_handle_SJOIN deferred TOPICLOCK/KEEPTOPIC enforcement while BURSTING was set, + so any mismatch between ci->last_topic and chan->topic is resolved here with a + single ChanServ TOPIC per channel. See issue #1. */ + synch_topics(); } burst_servers(server);