From d44e5903365b9b5df19336bcff3faa26a30ed899 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Thu, 24 Feb 2011 19:09:46 +0100 Subject: [PATCH] Make CUser::m_sUserName constant Changing the user name for a CUser instance is a really, really bad idea. There are lots of paths that depend on the user name and only few of them are fixed up when the user name changes. This fixes a problem where admin's "CloneUser from to" caused problems with modules, because all modules where loaded under the old user name and thus they read/write NV data from the wrong directory in ~/.znc/users. Thanks to un1matr1x for reporting this. Signed-off-by: Uli Schlachter --- User.cpp | 28 +++++++++++----------------- User.h | 5 ++--- modules/admin.cpp | 3 +-- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/User.cpp b/User.cpp index d3e5b06d..d234a53b 100644 --- a/User.cpp +++ b/User.cpp @@ -48,10 +48,15 @@ protected: CUser* m_pUser; }; -CUser::CUser(const CString& sUserName) { +CUser::CUser(const CString& sUserName) + : m_sUserName(sUserName), m_sCleanUserName(MakeCleanUserName(sUserName)) +{ + // set paths that depend on the user name: + m_sUserPath = CZNC::Get().GetUserPath() + "/" + m_sUserName; + m_sDLPath = m_sUserPath + "/downloads"; + m_pIRCSock = NULL; m_fTimezoneOffset = 0; - SetUserName(sUserName); m_sNick = m_sCleanUserName; m_sIdent = m_sCleanUserName; m_sRealName = sUserName; @@ -343,13 +348,11 @@ bool CUser::Clone(const CUser& User, CString& sErrorRet, bool bCloneChans) { return false; } + // user names can only specified for the constructor, changing it later + // on breaks too much stuff (e.g. lots of paths depend on the user name) if (GetUserName() != User.GetUserName()) { - if (CZNC::Get().FindUser(User.GetUserName())) { - sErrorRet = "New username already exists"; - return false; - } - - SetUserName(User.GetUserName()); + DEBUG("Ignoring username in CUser::Clone(), old username [" << GetUserName() + << "]; New username [" << User.GetUserName() << "]"); } if (!User.GetPass().empty()) { @@ -1205,15 +1208,6 @@ CString CUser::MakeCleanUserName(const CString& sUserName) { } // Setters -void CUser::SetUserName(const CString& s) { - m_sCleanUserName = CUser::MakeCleanUserName(s); - m_sUserName = s; - - // set paths that depend on the user name: - m_sUserPath = CZNC::Get().GetUserPath() + "/" + m_sUserName; - m_sDLPath = GetUserPath() + "/downloads"; -} - bool CUser::IsChan(const CString& sChan) const { if (sChan.empty()) return false; // There is no way this is a chan diff --git a/User.h b/User.h index 8df0e32b..9d49d2ca 100644 --- a/User.h +++ b/User.h @@ -136,7 +136,6 @@ public: void AddBytesWritten(unsigned long long u) { m_uBytesWritten += u; } // Setters - void SetUserName(const CString& s); void SetNick(const CString& s); void SetAltNick(const CString& s); void SetIdent(const CString& s); @@ -230,8 +229,8 @@ private: void JoinChans(set& sChans); protected: - CString m_sUserName; - CString m_sCleanUserName; + const CString m_sUserName; + const CString m_sCleanUserName; CString m_sNick; CString m_sAltNick; CString m_sIdent; diff --git a/modules/admin.cpp b/modules/admin.cpp index 1a46500e..ffb543cf 100644 --- a/modules/admin.cpp +++ b/modules/admin.cpp @@ -563,14 +563,13 @@ class CAdminMod : public CModule { return; } - CUser* pNewUser = new CUser(sOldUsername); + CUser* pNewUser = new CUser(sNewUsername); CString sError; if (!pNewUser->Clone(*pOldUser, sError)) { delete pNewUser; PutModule("Error: Cloning failed! [" + sError + "]"); return; } - pNewUser->SetUserName(sNewUsername); pNewUser->SetIRCConnectEnabled(false); if (!CZNC::Get().AddUser(pNewUser, sError)) {