Rework MODE/RPL_CHANMODEIS handling for trailing args (#1661)

Some servers may send a colon even if the last parameter doesn't need it, currently this leads to issues with permission/mode tracking, as the core doesn't handle the colon properly.

This fix replaces reconstructing the parameter string with just passing a vector of the relevant parameters to CChan::SetModes() and adds overrides for CChan::SetModes() and CChan::ModeChange() that accept the vector instead.

Clean up uses of old CModeMessage::GetModes()

src/Chan.cpp

@@ -260,6 +260,11 @@ void CChan::SetModes(const CString& sModes) {
     ModeChange(sModes);
 }
 
+void CChan::SetModes(const CString& modes, const VCString& vsModeParams) {
+    m_mcsModes.clear();
+    ModeChange(modes, vsModeParams);
+}
+
 void CChan::SetAutoClearChanBuffer(bool b) {
     m_bHasAutoClearChanBufferSet = true;
     m_bAutoClearChanBuffer = b;
@@ -295,9 +300,7 @@ void CChan::OnWho(const CString& sNick, const CString& sIdent,
     }
 }
 
-void CChan::ModeChange(const CString& sModes, const CNick* pOpNick) {
-    CString sModeArg = sModes.Token(0);
-    CString sArgs = sModes.Token(1, true);
+void CChan::ModeChange(const CString& sModes, const VCString& vsModes, const CNick* pOpNick) {
     bool bAdd = true;
 
     /* Try to find a CNick* from this channel so that pOpNick->HasPerm()
@@ -309,18 +312,22 @@ void CChan::ModeChange(const CString& sModes, const CNick* pOpNick) {
         if (OpNick) pOpNick = OpNick;
     }
 
-    NETWORKMODULECALL(OnRawMode2(pOpNick, *this, sModeArg, sArgs),
-                      m_pNetwork->GetUser(), m_pNetwork, nullptr, NOTHING);
+    {
+        CString sArgs = CString(" ").Join(vsModes.begin(), vsModes.end());
+        NETWORKMODULECALL(OnRawMode2(pOpNick, *this, sModes, sArgs),
+                          m_pNetwork->GetUser(), m_pNetwork, nullptr, NOTHING);
+    }
 
-    for (unsigned int a = 0; a < sModeArg.size(); a++) {
-        const char& cMode = sModeArg[a];
+    VCString::const_iterator argIter = vsModes.begin();
+    for (unsigned int a = 0; a < sModes.size(); a++) {
+        const char& cMode = sModes[a];
 
         if (cMode == '+') {
             bAdd = true;
         } else if (cMode == '-') {
             bAdd = false;
         } else if (m_pNetwork->GetIRCSock()->IsPermMode(cMode)) {
-            CString sArg = GetModeArg(sArgs);
+            CString sArg = *argIter++;
             CNick* pNick = FindNick(sArg);
             if (pNick) {
                 char cPerm =
@@ -382,16 +389,16 @@ void CChan::ModeChange(const CString& sModes, const CNick* pOpNick) {
             switch (m_pNetwork->GetIRCSock()->GetModeType(cMode)) {
                 case CIRCSock::ListArg:
                     bList = true;
-                    sArg = GetModeArg(sArgs);
+                    sArg = *argIter++;
                     break;
                 case CIRCSock::HasArg:
-                    sArg = GetModeArg(sArgs);
+                    sArg = *argIter++;
                     break;
                 case CIRCSock::NoArg:
                     break;
                 case CIRCSock::ArgWhenSet:
                     if (bAdd) {
-                        sArg = GetModeArg(sArgs);
+                        sArg = *argIter++;
                     }
 
                     break;
@@ -423,6 +430,32 @@ void CChan::ModeChange(const CString& sModes, const CNick* pOpNick) {
     }
 }
 
+void CChan::ModeChange(const CString& sModes, const CNick* pOpNick) {
+    VCString vsModes;
+    CString sModeArg = sModes.Token(0);
+    bool colon = sModeArg.TrimPrefix(":");
+
+    // Only handle parameters if sModes doesn't start with a colon
+    // because if it does, we only have the mode string with no parameters
+    if (!colon) {
+        CString sArgs = sModes.Token(1, true);
+
+        while (!sArgs.empty()) {
+            // Check if this parameter is a trailing parameter
+            // If so, treat the rest of sArgs as one parameter
+            if (sArgs.TrimPrefix(":")) {
+                vsModes.push_back(sArgs);
+                sArgs.clear();
+            } else {
+                vsModes.push_back(sArgs.Token(0));
+                sArgs = sArgs.Token(1, true);
+            }
+        }
+    }
+
+    ModeChange(sModeArg, vsModes, pOpNick);
+}
+
 CString CChan::GetOptions() const {
     VCString vsRet;
 

src/Client.cpp

@@ -1086,9 +1086,8 @@ bool CClient::OnJoinMessage(CJoinMessage& Message) {
 
 bool CClient::OnModeMessage(CModeMessage& Message) {
     CString sTarget = Message.GetTarget();
-    CString sModes = Message.GetModes();
 
-    if (m_pNetwork && m_pNetwork->IsChan(sTarget) && sModes.empty()) {
+    if (m_pNetwork && m_pNetwork->IsChan(sTarget) && !Message.HasModes()) {
         // If we are on that channel and already received a
         // /mode reply from the server, we can answer this
         // request ourself.

src/IRCSock.cpp

@@ -555,24 +555,24 @@ bool CIRCSock::OnKickMessage(CKickMessage& Message) {
 bool CIRCSock::OnModeMessage(CModeMessage& Message) {
     const CNick& Nick = Message.GetNick();
     CString sTarget = Message.GetTarget();
-    CString sModes = Message.GetModes();
+    VCString vsModes = Message.GetModeParams();
+    CString sModes = Message.GetModeList();
 
     CChan* pChan = m_pNetwork->FindChan(sTarget);
     if (pChan) {
-        pChan->ModeChange(sModes, &Nick);
+        pChan->ModeChange(sModes, vsModes, &Nick);
 
         if (pChan->IsDetached()) {
             return true;
         }
     } else if (sTarget == m_Nick.GetNick()) {
-        CString sModeArg = sModes.Token(0);
         bool bAdd = true;
         /* no module call defined (yet?)
                 MODULECALL(OnRawUserMode(*pOpNick, *this, sModeArg, sArgs),
            m_pNetwork->GetUser(), nullptr, );
         */
-        for (unsigned int a = 0; a < sModeArg.size(); a++) {
-            const char& cMode = sModeArg[a];
+        for (unsigned int a = 0; a < sModes.size(); a++) {
+            const char& cMode = sModes[a];
 
             if (cMode == '+') {
                 bAdd = true;
@@ -767,7 +767,7 @@ bool CIRCSock::OnNumericMessage(CNumericMessage& Message) {
             CChan* pChan = m_pNetwork->FindChan(Message.GetParam(1));
 
             if (pChan) {
-                pChan->SetModes(Message.GetParamsColon(2));
+                pChan->SetModes(Message.GetParam(2), Message.GetParamsSplit(3));
 
                 // We don't SetModeKnown(true) here,
                 // because a 329 will follow

src/Message.cpp

@@ -267,3 +267,23 @@ void CMessage::InitType() {
         }
     }
 }
+
+VCString CMessage::GetParamsSplit(unsigned int uIdx, unsigned int uLen) const {
+    VCString splitParams;
+    const VCString &params = GetParams();
+
+    if (params.empty() || uLen == 0 || uIdx >= params.size()) {
+        return splitParams;
+    }
+
+    if (uLen > params.size() - uIdx - 1) {
+        uLen = params.size() - uIdx;
+    }
+
+    VCString::const_iterator startIt = params.begin() + uIdx;
+    VCString::const_iterator endIt = startIt + uLen;
+
+    splitParams.assign(startIt, endIt);
+
+    return splitParams;
+}