From a0d58c8fed3b71e5302b0560133c7e3ae4e82f0a Mon Sep 17 00:00:00 2001 From: RealKindOne Date: Tue, 21 Jul 2026 03:24:38 -0400 Subject: [PATCH] Add IsParting() flag to track PART commands waiting IRCd response. Fixes potential desync where ZNC marks channel as parted before server confirms the PART, which could cause issues if the server never replies. Add unittests and integration tests. --- include/znc/Chan.h | 3 ++ src/Chan.cpp | 4 +- src/Client.cpp | 4 +- src/IRCSock.cpp | 16 ++++++ test/IRCSockTest.cpp | 41 +++++++++++++++ test/integration/tests/core.cpp | 91 ++++++++++++++++++++++++++++++++- 6 files changed, 154 insertions(+), 5 deletions(-) diff --git a/include/znc/Chan.h b/include/znc/Chan.h index 31fa8e34..831d50ae 100644 --- a/include/znc/Chan.h +++ b/include/znc/Chan.h @@ -168,6 +168,7 @@ class CChan : private CCoreTranslationMixin { void Enable(); void IncJoinTries() { m_uJoinTries++; } void ResetJoinTries() { m_uJoinTries = 0; } + void SetParting(bool b) { m_bParting = b; } // !Setters // Getters @@ -199,6 +200,7 @@ class CChan : private CCoreTranslationMixin { bool HasAutoClearChanBufferSet() const { return m_bHasAutoClearChanBufferSet; } + bool IsParting() const { return m_bParting; } // !Getters private: protected: @@ -223,6 +225,7 @@ class CChan : private CCoreTranslationMixin { CBuffer m_Buffer; bool m_bModeKnown; + bool m_bParting; std::map m_mcsModes; }; diff --git a/src/Chan.cpp b/src/Chan.cpp index 23f949a8..fd370f2a 100644 --- a/src/Chan.cpp +++ b/src/Chan.cpp @@ -30,6 +30,7 @@ CChan::CChan(const CString& sName, CIRCNetwork* pNetwork, bool bInConfig, CConfig* pConfig) : m_bDetached(false), m_bIsOn(false), + m_bParting(false), m_bAutoClearChanBuffer(pNetwork->GetUser()->AutoClearChanBuffer()), m_bInConfig(bInConfig), m_bDisabled(false), @@ -84,6 +85,7 @@ CChan::~CChan() { ClearNicks(); } void CChan::Reset() { m_bIsOn = false; + m_bParting = false; m_bModeKnown = false; m_mcsModes.clear(); m_sTopic = ""; @@ -143,7 +145,7 @@ void CChan::JoinUser(const CString& sKey) { if (!IsOn() && !sKey.empty()) { SetKey(sKey); } - if (m_pNetwork->IsIRCConnected() && !IsOn()) { + if (m_pNetwork->IsIRCConnected() && (!IsOn() || IsParting())) { m_pNetwork->PutIRC("JOIN " + GetName() + " " + GetKey()); } } diff --git a/src/Client.cpp b/src/Client.cpp index f51e8371..a5020dd8 100644 --- a/src/Client.cpp +++ b/src/Client.cpp @@ -1469,7 +1469,7 @@ bool CClient::OnJoinMessage(CJoinMessage& Message) { if (pChan) { if (pChan->IsDetached()) pChan->AttachUser(this); - else + else if (!pChan->IsOn() || pChan->IsParting()) pChan->JoinUser(sKey); continue; } else if (!sChannel.empty()) { @@ -1592,7 +1592,7 @@ bool CClient::OnPartMessage(CPartMessage& Message) { PutStatusNotice(t_f("Removing channel {1}")(sChan)); m_pNetwork->DelChan(sChan); } else { - if (pChan) pChan->SetIsOn(false); + if (pChan) pChan->SetParting(true); sChans += (sChans.empty()) ? sChan : CString("," + sChan); } } diff --git a/src/IRCSock.cpp b/src/IRCSock.cpp index 6695b240..82a9fc3e 100644 --- a/src/IRCSock.cpp +++ b/src/IRCSock.cpp @@ -692,6 +692,7 @@ bool CIRCSock::OnJoinMessage(CJoinMessage& Message) { if (pChan) { pChan->Enable(); pChan->SetIsOn(true); + pChan->SetParting(false); PutIRC("MODE " + sChan); } } else { @@ -727,6 +728,7 @@ bool CIRCSock::OnKickMessage(CKickMessage& Message) { if (GetNick().Equals(sKickedNick) && pChan) { pChan->SetIsOn(false); + pChan->SetParting(false); // Don't try to rejoin! pChan->Disable(); @@ -1119,6 +1121,19 @@ bool CIRCSock::OnNumericMessage(CNumericMessage& Message) { m_pNetwork->AddMotdBuffer(BufferMessage(Message)); } break; + case 403: // ERR_NOSUCHCHANNEL + case 442: // ERR_NOTONCHANNEL + { + CString sChan = Message.GetParam(1); + CChan* pChan = m_pNetwork->FindChan(sChan); + if (pChan && pChan->IsParting()) { + pChan->SetIsOn(false); + pChan->SetParting(false); + m_pNetwork->PutStatus( + t_f("PART failed for channel {1}")(sChan)); + } + } + break; case 437: // :irc.server.net 437 * badnick :Nick/channel is temporarily unavailable // :irc.server.net 437 mynick badnick :Nick/channel is temporarily unavailable @@ -1189,6 +1204,7 @@ bool CIRCSock::OnPartMessage(CPartMessage& Message) { } if (Nick.NickEquals(GetNick())) { + if (pChan) pChan->SetParting(false); m_pNetwork->DelChan(sChan); } diff --git a/test/IRCSockTest.cpp b/test/IRCSockTest.cpp index f6d38134..97b6314b 100644 --- a/test/IRCSockTest.cpp +++ b/test/IRCSockTest.cpp @@ -558,3 +558,44 @@ TEST_F(IRCSockTest, ChanMode) { ":are supported by this server"); m_pTestSock->ReadLine(":irc.znc.in 324 me #chan +ntf "); } + +TEST_F(IRCSockTest, PartingFlag) { + // Set channel as joined + m_pTestChan->SetIsOn(true); + + CMessage clientPart("PART #chan"); + m_pTestClient->ReadLine(clientPart.ToString()); + + EXPECT_THAT(m_pTestSock->vsLines, ElementsAre("PART #chan")); + EXPECT_TRUE(m_pTestChan->IsParting()); + + m_pTestSock->Reset(); + + CMessage serverPart(":me PART #chan"); + m_pTestSock->ReadLine(serverPart.ToString()); + + // IsParting() should be free + EXPECT_FALSE(m_pTestChan->IsParting()); + + // Verify channel was deleted + EXPECT_EQ(m_pTestNetwork->FindChan("#chan"), nullptr); +} + +TEST_F(IRCSockTest, PartingFlagOnError) { + // Set channel as joined + m_pTestChan->SetIsOn(true); + + CMessage clientPart("PART #chan"); + m_pTestClient->ReadLine(clientPart.ToString()); + + m_pTestSock->Reset(); + + CMessage errorMsg(":server 442 me #chan :You're not on that channel"); + m_pTestSock->ReadLine(errorMsg.ToString()); + + // IsParting() should be free + EXPECT_FALSE(m_pTestChan->IsParting()); + + // Verify channel was deleted + EXPECT_NE(m_pTestNetwork->FindChan("#chan"), nullptr); +} \ No newline at end of file diff --git a/test/integration/tests/core.cpp b/test/integration/tests/core.cpp index 4169b687..3d8f5a51 100644 --- a/test/integration/tests/core.cpp +++ b/test/integration/tests/core.cpp @@ -1318,7 +1318,7 @@ TEST_F(ZNCTest, JoinDetachedChannelMultiClient) { // Commit ad7bd6d7eed84648638e1b6fd69546b9fe496576 // prevented rejoining when a client cycles a channel. -TEST_F(ZNCTest, ClientCycleChannels) { +TEST_F(ZNCTest, ClientCycleChannel) { auto znc = Run(); auto ircd = ConnectIRCd(); auto client = LoginClient(); @@ -1333,11 +1333,98 @@ TEST_F(ZNCTest, ClientCycleChannels) { client.ReadUntil("End of /NAMES"); // Clients have '/hop' or '/cycle' command that sends - // 'PART #channel' and 'JOIN #channel'. Verify ZNC rejoins.. + // 'PART #channel' and 'JOIN #channel'. Verify ZNC rejoins. client.Write(":nick PART #test"); + + // Verify PART is forwarded to server + QByteArray partMsg; + ircd.ReadUntilAndGet("PART", partMsg); + EXPECT_THAT(partMsg.toStdString(), HasSubstr("PART #test")); + + ircd.Write(":nick PART #test"); + client.ReadUntil(":nick PART #test"); + client.Write(":nick JOIN #test"); ircd.ReadUntil("JOIN #test"); } +TEST_F(ZNCTest, PartWithError403) { + auto znc = Run(); + auto ircd = ConnectIRCd(); + auto client = LoginClient(); + + // Join a channel first + client.Write("JOIN #test"); + client.Close(); + + ircd.Write(":server 001 nick :Hello"); + ircd.ReadUntil("JOIN #test"); + ircd.Write(":nick JOIN :#test"); + ircd.Write(":server 353 nick #test :nick"); + ircd.Write(":server 366 nick #test :End of /NAMES list"); + + // Reconnect client and send PART + client = LoginClient(); + client.ReadUntil(":nick JOIN :#test"); + client.Write("PART #test"); + + QByteArray partMsg; + ircd.ReadUntilAndGet("PART", partMsg); + EXPECT_THAT(partMsg.toStdString(), HasSubstr("PART #test")); + + // Server returns 403 error (ERR_NOSUCHCHANNEL) instead of confirming + ircd.Write(":server 403 nick #test :No such channel"); + + // Verify client receives the status message about PART failure + client.ReadUntil("PART failed for channel #test"); + + // Verify channel still exists by trying to rejoin + client.Write("JOIN #test"); + ircd.ReadUntil("JOIN #test"); + ircd.Write(":nick JOIN :#test"); + ircd.Write(":server 353 nick #test :nick"); + ircd.Write(":server 366 nick #test :End of /NAMES list"); + client.ReadUntil(":nick JOIN :#test"); +} + +TEST_F(ZNCTest, PartWithError442) { + auto znc = Run(); + auto ircd = ConnectIRCd(); + auto client = LoginClient(); + + // Join a channel first + client.Write("JOIN #test"); + client.Close(); + + ircd.Write(":server 001 nick :Hello"); + ircd.ReadUntil("JOIN #test"); + ircd.Write(":nick JOIN :#test"); + ircd.Write(":server 353 nick #test :nick"); + ircd.Write(":server 366 nick #test :End of /NAMES list"); + + // Reconnect client and send PART + client = LoginClient(); + client.ReadUntil(":nick JOIN :#test"); + client.Write("PART #test :leaving"); + + QByteArray partMsg; + ircd.ReadUntilAndGet("PART", partMsg); + EXPECT_THAT(partMsg.toStdString(), HasSubstr("PART #test")); + + // Server returns 442 error (ERR_NOTONCHANNEL) instead of confirming + ircd.Write(":server 442 nick #test :You're not on that channel"); + + // Verify client receives the status message about PART failure + client.ReadUntil("PART failed for channel #test"); + + // Verify channel still exists by trying to rejoin + client.Write("JOIN #test"); + ircd.ReadUntil("JOIN #test"); + ircd.Write(":nick JOIN :#test"); + ircd.Write(":server 353 nick #test :nick"); + ircd.Write(":server 366 nick #test :End of /NAMES list"); + client.ReadUntil(":nick JOIN :#test"); +} + } // namespace } // namespace znc_inttest