From efd136c01c70f0240b4e699891fdd9dadafe2f18 Mon Sep 17 00:00:00 2001 From: Alexey Sokolov Date: Sun, 14 Jan 2024 11:12:32 +0000 Subject: [PATCH] Make modpython work with server-dependent caps --- include/znc/Modules.h | 30 +++++++++-- modules/modpython.cpp | 43 ++++++++++++++++ modules/modpython/modpython.i | 4 ++ modules/modpython/module.h | 14 ++++++ src/Modules.cpp | 88 ++++++++++++++++++++++----------- test/integration/tests/core.cpp | 71 ++++++++++++++++++-------- 6 files changed, 195 insertions(+), 55 deletions(-) diff --git a/include/znc/Modules.h b/include/znc/Modules.h index aca782a9..7320912d 100644 --- a/include/znc/Modules.h +++ b/include/znc/Modules.h @@ -1324,11 +1324,12 @@ class CModule { * use, create a subclass of CCapability, and pass to this function; it * will automatically set the module pointer, then call the callbacks to * notify you when server and client accepted support of the capability, or - * stopped supporting it. Note that it's not a strict toggle: e.g. sometimes - * client will disable the cap even when it was already disabled for that - * client. If you want to mix this function with other CAP callbacks in the - * same module, in the overridden functions you'll need to explicitly call - * the function from the base class CModule. + * stopped supporting it. Note that it's not a strict toggle: e.g. + * sometimes client will disable the cap even when it was already disabled + * for that client. + * For python modules, this function accepts 3 parameters: + * name, server callback, client callback; signatures of the callbacks are + * the same as of the virtual functions you'd implement in C++. */ void AddServerDependentCapability(const CString& sName, std::unique_ptr pCap); #endif @@ -1413,6 +1414,25 @@ class CModule { const CString& sContext = "") const; #endif + // Default implementations of several CAP callbacks to make + // AddServerDependentCapability work in modpython/modperl. + bool InternalServerDependentCapsOnServerCap302Available( + const CString& sCap, const CString& sValue); + void InternalServerDependentCapsOnServerCapResult(const CString& sCap, + bool bSuccess); + void InternalServerDependentCapsOnClientCapLs(CClient* pClient, + SCString& ssCaps); + bool InternalServerDependentCapsIsClientCapSupported(CClient* pClient, + const CString& sCap, + bool bState); + void InternalServerDependentCapsOnClientCapRequest(CClient* pClient, + const CString& sCap, + bool bState); + void InternalServerDependentCapsOnClientAttached(); + void InternalServerDependentCapsOnClientDetached(); + void InternalServerDependentCapsOnIRCConnected(); + void InternalServerDependentCapsOnIRCDisconnected(); + protected: CModInfo::EModuleType m_eType; CString m_sDescription; diff --git a/modules/modpython.cpp b/modules/modpython.cpp index ec2a1186..630cfef3 100644 --- a/modules/modpython.cpp +++ b/modules/modpython.cpp @@ -478,6 +478,49 @@ CPySocket::~CPySocket() { Py_CLEAR(m_pyObj); } +CPyCapability::CPyCapability(PyObject* serverCb, PyObject* clientCb) + : m_serverCb(serverCb), m_clientCb(clientCb) { + Py_INCREF(serverCb); + Py_INCREF(clientCb); +} + +CPyCapability::~CPyCapability() { + Py_CLEAR(m_serverCb); + Py_CLEAR(m_clientCb); +} + +void CPyCapability::OnServerChangedSupport(CIRCNetwork* pNetwork, bool bState) { + PyObject* pyArg_Network = + SWIG_NewInstanceObj(pNetwork, SWIG_TypeQuery("CIRCNetwork*"), 0); + PyObject* pyArg_bState = Py_BuildValue("l", (long int)bState); + PyObject* pyRes = PyObject_CallFunctionObjArgs(m_serverCb, pyArg_Network, + pyArg_bState, nullptr); + if (!pyRes) { + CString sPyErr = ((CPyModule*)GetModule())->GetPyExceptionStr(); + DEBUG("modpython: " << GetModule()->GetModName() + << "/OnServerChangedSupport failed: " << sPyErr); + } + Py_CLEAR(pyRes); + Py_CLEAR(pyArg_bState); + Py_CLEAR(pyArg_Network); +} + +void CPyCapability::OnClientChangedSupport(CClient* pClient, bool bState) { + PyObject* pyArg_Client = + SWIG_NewInstanceObj(pClient, SWIG_TypeQuery("CClient*"), 0); + PyObject* pyArg_bState = Py_BuildValue("l", (long int)bState); + PyObject* pyRes = PyObject_CallFunctionObjArgs(m_clientCb, pyArg_Client, + pyArg_bState, nullptr); + if (!pyRes) { + CString sPyErr = ((CPyModule*)GetModule())->GetPyExceptionStr(); + DEBUG("modpython: " << GetModule()->GetModName() + << "/OnClientChangedSupport failed: " << sPyErr); + } + Py_CLEAR(pyRes); + Py_CLEAR(pyArg_bState); + Py_CLEAR(pyArg_Client); +} + CPyModule* CPyModCommand::GetModule() { return this->m_pModule; } diff --git a/modules/modpython/modpython.i b/modules/modpython/modpython.i index 65604da2..38db2a92 100644 --- a/modules/modpython/modpython.i +++ b/modules/modpython/modpython.i @@ -241,6 +241,10 @@ class CPyRetBool { bool ExistsNV(const CString& sName) { return $self->EndNV() != $self->FindNV(sName); } + void AddServerDependentCapability(const CString& sName, PyObject* serverCb, + PyObject* clientCb) { + $self->AddServerDependentCapability(sName, std::make_unique(serverCb, clientCb)); + } } %extend CModules { diff --git a/modules/modpython/module.h b/modules/modpython/module.h index 8d688787..333aa3ee 100644 --- a/modules/modpython/module.h +++ b/modules/modpython/module.h @@ -369,3 +369,17 @@ inline CPyModCommand* CreatePyModCommand(CPyModule* pModule, PyObject* pyObj) { return new CPyModCommand(pModule, sCmd, sArgs, sDesc, pyObj); } + +class ZNC_EXPORT_LIB_EXPORT CPyCapability : public CCapability { + public: + CPyCapability(PyObject* serverCb, PyObject* clientCb); + ~CPyCapability(); + + void OnServerChangedSupport(CIRCNetwork* pNetwork, bool bState) override; + void OnClientChangedSupport(CClient* pClient, bool bState) override; + + private: + PyObject* m_serverCb; + PyObject* m_clientCb; +}; + diff --git a/src/Modules.cpp b/src/Modules.cpp index 8d61aeec..badbfd72 100644 --- a/src/Modules.cpp +++ b/src/Modules.cpp @@ -162,21 +162,18 @@ CModule::CModule(ModHandle pDLL, CUser* pUser, CIRCNetwork* pNetwork, CModule::~CModule() { for (const auto& [sName, pCap] : m_mServerDependentCaps) { + // pCap->OnClientChangedSupport is useless (and even dangerous) to call + // from the destructor, since the derived CModule class is gone already. + // But still need to tell clients via cap-notify that the cap is gone. switch (GetType()) { case CModInfo::NetworkModule: GetNetwork()->NotifyClientsAboutServerDependentCap(sName, false); - for (CClient* pClient : GetNetwork()->GetClients()) { - pCap->OnClientChangedSupport(pClient, false); - } break; case CModInfo::UserModule: for (CIRCNetwork* pNetwork : GetUser()->GetNetworks()) { pNetwork->NotifyClientsAboutServerDependentCap(sName, false); - for (CClient* pClient : pNetwork->GetClients()) { - pCap->OnClientChangedSupport(pClient, false); - } } break; case CModInfo::GlobalModule: @@ -184,9 +181,6 @@ CModule::~CModule() { for (CIRCNetwork* pNetwork : pUser->GetNetworks()) { pNetwork->NotifyClientsAboutServerDependentCap(sName, false); - for (CClient* pClient : pNetwork->GetClients()) { - pCap->OnClientChangedSupport(pClient, false); - } } } } @@ -637,7 +631,9 @@ bool CModule::OnLoad(const CString& sArgs, CString& sMessage) { bool CModule::OnBoot() { return true; } void CModule::OnPreRehash() {} void CModule::OnPostRehash() {} -void CModule::OnIRCDisconnected() { +void CModule::OnIRCDisconnected() {} +void CModule::InternalServerDependentCapsOnIRCDisconnected() { + OnIRCDisconnected(); for (const auto& [sName, pCap] : m_mServerDependentCaps) { GetNetwork()->NotifyClientsAboutServerDependentCap(sName, false); for (CClient* pClient : GetNetwork()->GetClients()) { @@ -645,7 +641,9 @@ void CModule::OnIRCDisconnected() { } } } -void CModule::OnIRCConnected() { +void CModule::OnIRCConnected() {} +void CModule::InternalServerDependentCapsOnIRCConnected() { + OnIRCConnected(); for (const auto& [sName, pCap] : m_mServerDependentCaps) { if (GetNetwork()->IsServerCapAccepted(sName)) { GetNetwork()->NotifyClientsAboutServerDependentCap(sName, true); @@ -1042,7 +1040,9 @@ CModule::EModRet CModule::OnSendToIRC(CString& sLine) { return CONTINUE; } CModule::EModRet CModule::OnSendToIRCMessage(CMessage& Message) { return CONTINUE; } -void CModule::OnClientAttached() { +void CModule::OnClientAttached() {} +void CModule::InternalServerDependentCapsOnClientAttached() { + OnClientAttached(); if (!GetNetwork()) return; for (const auto& [sName, pCap] : m_mServerDependentCaps) { if (GetNetwork()->IsServerCapAccepted(sName)) { @@ -1050,7 +1050,9 @@ void CModule::OnClientAttached() { } } } -void CModule::OnClientDetached() { +void CModule::OnClientDetached() {} +void CModule::InternalServerDependentCapsOnClientDetached() { + OnClientDetached(); for (const auto& [sName, pCap] : m_mServerDependentCaps) { GetClient()->NotifyServerDependentCap(sName, false, ""); pCap->OnClientChangedSupport(GetClient(), false); @@ -1060,8 +1062,13 @@ void CModule::OnClientDetached() { bool CModule::OnServerCapAvailable(const CString& sCap) { return false; } bool CModule::OnServerCap302Available(const CString& sCap, const CString& sValue) { + return OnServerCapAvailable(sCap); +} +bool CModule::InternalServerDependentCapsOnServerCap302Available(const CString& sCap, + const CString& sValue) { auto it = m_mServerDependentCaps.find(sCap); - if (it == m_mServerDependentCaps.end()) return OnServerCapAvailable(sCap); + if (it == m_mServerDependentCaps.end()) + return OnServerCap302Available(sCap, sValue); if (GetNetwork()->IsServerCapAccepted(sCap)) { // This can happen when server sent CAP NEW with another value. GetNetwork()->NotifyClientsAboutServerDependentCap(sCap, true); @@ -1070,7 +1077,10 @@ bool CModule::OnServerCap302Available(const CString& sCap, } return true; } -void CModule::OnServerCapResult(const CString& sCap, bool bSuccess) { +void CModule::OnServerCapResult(const CString& sCap, bool bSuccess) {} +void CModule::InternalServerDependentCapsOnServerCapResult(const CString& sCap, + bool bSuccess) { + OnServerCapResult(sCap, bSuccess); auto it = m_mServerDependentCaps.find(sCap); if (it == m_mServerDependentCaps.end()) return; it->second->OnServerChangedSupport(GetNetwork(), bSuccess); @@ -1151,7 +1161,8 @@ CModule::EModRet CModule::OnUnknownUserRaw(CClient* pClient, CString& sLine) { CModule::EModRet CModule::OnUnknownUserRawMessage(CMessage& Message) { return CONTINUE; } -void CModule::OnClientCapLs(CClient* pClient, SCString& ssCaps) { +void CModule::OnClientCapLs(CClient* pClient, SCString& ssCaps) {} +void CModule::InternalServerDependentCapsOnClientCapLs(CClient* pClient, SCString& ssCaps) { for (const auto& [sName, pCap] : m_mServerDependentCaps) { if (GetNetwork() && GetNetwork()->IsServerCapAccepted(sName)) { if (pClient->HasCap302()) { @@ -1167,16 +1178,24 @@ void CModule::OnClientCapLs(CClient* pClient, SCString& ssCaps) { } } } + OnClientCapLs(pClient, ssCaps); } bool CModule::IsClientCapSupported(CClient* pClient, const CString& sCap, - bool bState) { + bool bState) { return false; } +bool CModule::InternalServerDependentCapsIsClientCapSupported( + CClient* pClient, const CString& sCap, bool bState) { auto it = m_mServerDependentCaps.find(sCap); - if (it == m_mServerDependentCaps.end()) return false; + if (it == m_mServerDependentCaps.end()) + return IsClientCapSupported(pClient, sCap, bState); if (!bState) return true; return GetNetwork() && GetNetwork()->IsServerCapAccepted(sCap); } void CModule::OnClientCapRequest(CClient* pClient, const CString& sCap, - bool bState) { + bool bState) {} +void CModule::InternalServerDependentCapsOnClientCapRequest(CClient* pClient, + const CString& sCap, + bool bState) { + OnClientCapRequest(pClient, sCap, bState); auto it = m_mServerDependentCaps.find(sCap); if (it == m_mServerDependentCaps.end()) return; it->second->OnClientChangedSupport(pClient, bState); @@ -1242,7 +1261,7 @@ bool CModules::OnPostRehash() { return false; } bool CModules::OnIRCConnected() { - MODUNLOADCHK(OnIRCConnected()); + MODUNLOADCHK(InternalServerDependentCapsOnIRCConnected()); return false; } bool CModules::OnIRCConnecting(CIRCSock* pIRCSock) { @@ -1260,7 +1279,7 @@ bool CModules::OnBroadcast(CString& sMessage) { MODHALTCHK(OnBroadcast(sMessage)); } bool CModules::OnIRCDisconnected() { - MODUNLOADCHK(OnIRCDisconnected()); + MODUNLOADCHK(InternalServerDependentCapsOnIRCDisconnected()); return false; } @@ -1601,11 +1620,11 @@ bool CModules::OnModCTCP(const CString& sMessage) { return false; } bool CModules::OnClientAttached() { - MODUNLOADCHK(OnClientAttached()); + MODUNLOADCHK(InternalServerDependentCapsOnClientAttached()); return false; } bool CModules::OnClientDetached() { - MODUNLOADCHK(OnClientDetached()); + MODUNLOADCHK(InternalServerDependentCapsOnClientDetached()); return false; } @@ -1621,12 +1640,16 @@ bool CModules::OnServerCapAvailable(const CString& sCap, const CString& sValue) CIRCNetwork* pOldNetwork = pMod->GetNetwork(); pMod->SetUser(m_pUser); pMod->SetNetwork(m_pNetwork); - bResult |= pMod->OnServerCap302Available(sCap, sValue); + bResult |= + pMod->InternalServerDependentCapsOnServerCap302Available( + sCap, sValue); pMod->SetUser(pOldUser); pMod->SetNetwork(pOldNetwork); } else { // WTF? Is that possible? - bResult |= pMod->OnServerCap302Available(sCap, sValue); + bResult |= + pMod->InternalServerDependentCapsOnServerCap302Available( + sCap, sValue); } pMod->SetClient(pOldClient); } catch (const CModule::EModException& e) { @@ -1639,7 +1662,7 @@ bool CModules::OnServerCapAvailable(const CString& sCap, const CString& sValue) } bool CModules::OnServerCapResult(const CString& sCap, bool bSuccess) { - MODUNLOADCHK(OnServerCapResult(sCap, bSuccess)); + MODUNLOADCHK(InternalServerDependentCapsOnServerCapResult(sCap, bSuccess)); return false; } @@ -1677,7 +1700,7 @@ bool CModules::OnUnknownUserRawMessage(CMessage& Message) { } bool CModules::OnClientCapLs(CClient* pClient, SCString& ssCaps) { - MODUNLOADCHK(OnClientCapLs(pClient, ssCaps)); + MODUNLOADCHK(InternalServerDependentCapsOnClientCapLs(pClient, ssCaps)); return false; } @@ -1694,12 +1717,16 @@ bool CModules::IsClientCapSupported(CClient* pClient, const CString& sCap, CIRCNetwork* pOldNetwork = pMod->GetNetwork(); pMod->SetUser(m_pUser); pMod->SetNetwork(m_pNetwork); - bResult |= pMod->IsClientCapSupported(pClient, sCap, bState); + bResult |= + pMod->InternalServerDependentCapsIsClientCapSupported( + pClient, sCap, bState); pMod->SetUser(pOldUser); pMod->SetNetwork(pOldNetwork); } else { // WTF? Is that possible? - bResult |= pMod->IsClientCapSupported(pClient, sCap, bState); + bResult |= + pMod->InternalServerDependentCapsIsClientCapSupported( + pClient, sCap, bState); } pMod->SetClient(pOldClient); } catch (const CModule::EModException& e) { @@ -1713,7 +1740,8 @@ bool CModules::IsClientCapSupported(CClient* pClient, const CString& sCap, bool CModules::OnClientCapRequest(CClient* pClient, const CString& sCap, bool bState) { - MODUNLOADCHK(OnClientCapRequest(pClient, sCap, bState)); + MODUNLOADCHK( + InternalServerDependentCapsOnClientCapRequest(pClient, sCap, bState)); return false; } diff --git a/test/integration/tests/core.cpp b/test/integration/tests/core.cpp index 2160bfa6..dbf5e044 100644 --- a/test/integration/tests/core.cpp +++ b/test/integration/tests/core.cpp @@ -554,30 +554,61 @@ TEST_F(ZNCTest, CAP302LSValue) { client2.ReadUntil("testcap="); } -TEST_F(ZNCTest, ServerDependentCapInModule) { +class AllLanguages : public ZNCTest, public testing::WithParamInterface {}; + +INSTANTIATE_TEST_CASE_P(LanguagesTests, AllLanguages, testing::Values(1, 2)); + +TEST_P(AllLanguages, ServerDependentCapInModule) { auto znc = Run(); auto ircd = ConnectIRCd(); auto client = LoginClient(); - InstallModule("testmod.cpp", R"( - #include - class TestModule : public CModule { - class TestCap : public CCapability { - public: - using CCapability::CCapability; - void OnServerChangedSupport(CIRCNetwork* pNetwork, bool bState) override { - GetModule()->PutModule("Server changed support: " + CString(bState)); - } - void OnClientChangedSupport(CClient* pClient, bool bState) override { - GetModule()->PutModule("Client changed support: " + CString(bState)); - } - }; - public: - MODCONSTRUCTOR(TestModule) { - AddServerDependentCapability("testcap", std::make_unique()); + switch (GetParam()) { + case 1: + InstallModule("testmod.cpp", R"( + #include + class TestModule : public CModule { + class TestCap : public CCapability { + void OnServerChangedSupport(CIRCNetwork* pNetwork, bool bState) override { + GetModule()->PutModule("Server changed support: " + CString(bState)); + } + void OnClientChangedSupport(CClient* pClient, bool bState) override { + GetModule()->PutModule("Client changed support: " + CString(bState)); + } + }; + public: + MODCONSTRUCTOR(TestModule) { + AddServerDependentCapability("testcap", std::make_unique()); + } + }; + MODULEDEFS(TestModule, "Test") + )"); + break; + case 2: + if (QProcessEnvironment::systemEnvironment().value( + "DISABLED_ZNC_PERL_PYTHON_TEST") == "1") { + return; } - }; - MODULEDEFS(TestModule, "Test") - )"); + znc->CanLeak(); + InstallModule("testmod.py", R"( + import znc + class testmod(znc.Module): + def OnLoad(self, args, ret): + def server_change(net, state): + self.PutModule('Server changed support: ' + ('true' if state else 'false')) + def client_change(client, state): + self.PutModule('Client changed support: ' + ('true' if state else 'false')) + self.AddServerDependentCapability('testcap', server_change, client_change) + return True + )"); + client.Write("znc loadmod modpython"); + break; + case 3: + if (QProcessEnvironment::systemEnvironment().value( + "DISABLED_ZNC_PERL_PYTHON_TEST") == "1") { + return; + } + break; + } client.Write("znc loadmod testmod"); client.ReadUntil("Loaded module testmod"); client.Write("znc addnetwork net2");