Improve the handling of config X-lines and filters. (#1583)

diff --git a/include/xline.h b/include/xline.h
index f593c1c9728f299a5e915d16a70616fa95c6025d..bc9739f212ccf1a872109a6c71e922d4d7a34bee 100644 (file)
--- a/include/xline.h
+++ b/include/xline.h
@@ -515,8 +515,9 @@ class CoreExport XLineManager
        /** Expire a line given two iterators which identify it in the main map.
         * @param container Iterator to the first level of entries the map
         * @param item Iterator to the second level of entries in the map
+        * @param silent If true, doesn't send an expiry SNOTICE.
         */
-       void ExpireLine(ContainerIter container, LookupIter item);
+       void ExpireLine(ContainerIter container, LookupIter item, bool silent = false);
 
        /** Apply any new lines that are pending to be applied.
         * This will only apply lines in the pending_lines list, to save on
@@ -533,6 +534,6 @@ class CoreExport XLineManager
         */
        void InvokeStats(const std::string& type, unsigned int numeric, Stats::Context& stats);
 
-       /** Clears any XLines which were added by the server configuration. */
-       void ClearConfigLines();
+       /** Expire X-lines which were added by the server configuration and have been removed. */
+       void ExpireRemovedConfigLines(const std::string& type, const insp::flat_set<std::string>& configlines);
 };

diff --git a/src/configreader.cpp b/src/configreader.cpp
index 00880cfff80d88babd47c69d635f644a7d179704..0318dd602cb9f071dd9ca5a09369117046e6f23e 100644 (file)
--- a/src/configreader.cpp
+++ b/src/configreader.cpp
@@ -74,6 +74,8 @@ ServerConfig::~ServerConfig()
 
 static void ReadXLine(ServerConfig* conf, const std::string& tag, const std::string& key, XLineFactory* make)
 {
+       insp::flat_set<std::string> configlines;
+
        ConfigTagList tags = conf->ConfTags(tag);
        for(ConfigIter i = tags.first; i != tags.second; ++i)
        {
@@ -84,9 +86,12 @@ static void ReadXLine(ServerConfig* conf, const std::string& tag, const std::str
                std::string reason = ctag->getString("reason", "<Config>");
                XLine* xl = make->Generate(ServerInstance->Time(), 0, "<Config>", reason, mask);
                xl->from_config = true;
+               configlines.insert(xl->Displayable());
                if (!ServerInstance->XLines->AddLine(xl, NULL))
                        delete xl;
        }
+
+       ServerInstance->XLines->ExpireRemovedConfigLines(make->GetType(), configlines);
 }
 
 typedef std::map<std::string, ConfigTag*> LocalIndex;
@@ -405,7 +410,6 @@ void ServerConfig::Fill()
                        SocketEngine::Close(socktest);
        }
 
-       ServerInstance->XLines->ClearConfigLines();
        ReadXLine(this, "badip", "ipmask", ServerInstance->XLines->GetFactory("Z"));
        ReadXLine(this, "badnick", "nick", ServerInstance->XLines->GetFactory("Q"));
        ReadXLine(this, "badhost", "host", ServerInstance->XLines->GetFactory("K"));

diff --git a/src/modules/m_filter.cpp b/src/modules/m_filter.cpp
index f49694e81744fc47e7636e9541504733f92c7d58..7a7497d1a8a559f5c07a05d47f97fa08510e6adc 100644 (file)
--- a/src/modules/m_filter.cpp
+++ b/src/modules/m_filter.cpp
@@ -193,7 +193,7 @@ class ModuleFilter : public Module, public ServerEventListener, public Stats::Ev
        ModResult OnUserPreMessage(User* user, const MessageTarget& target, MessageDetails& details) CXX11_OVERRIDE;
        FilterResult* FilterMatch(User* user, const std::string &text, int flags);
        bool DeleteFilter(const std::string& freeform, std::string& reason);
-       std::pair<bool, std::string> AddFilter(const std::string& freeform, FilterAction type, const std::string& reason, unsigned long duration, const std::string& flags);
+       std::pair<bool, std::string> AddFilter(const std::string& freeform, FilterAction type, const std::string& reason, unsigned long duration, const std::string& flags, bool config = false);
        void ReadConfig(ConfigStatus& status) CXX11_OVERRIDE;
        Version GetVersion() CXX11_OVERRIDE;
        std::string EncodeFilter(FilterResult* filter);
@@ -744,7 +744,7 @@ bool ModuleFilter::DeleteFilter(const std::string& freeform, std::string& reason
        return false;
 }
 
-std::pair<bool, std::string> ModuleFilter::AddFilter(const std::string& freeform, FilterAction type, const std::string& reason, unsigned long duration, const std::string& flgs)
+std::pair<bool, std::string> ModuleFilter::AddFilter(const std::string& freeform, FilterAction type, const std::string& reason, unsigned long duration, const std::string& flgs, bool config)
 {
        for (std::vector<FilterResult>::iterator i = filters.begin(); i != filters.end(); i++)
        {
@@ -756,7 +756,7 @@ std::pair<bool, std::string> ModuleFilter::AddFilter(const std::string& freeform
 
        try
        {
-               filters.push_back(FilterResult(RegexEngine, freeform, reason, type, duration, flgs, false));
+               filters.push_back(FilterResult(RegexEngine, freeform, reason, type, duration, flgs, config));
        }
        catch (ModuleException &e)
        {
@@ -807,11 +807,13 @@ std::string ModuleFilter::FilterActionToString(FilterAction fa)
 
 void ModuleFilter::ReadFilters()
 {
+       insp::flat_set<std::string> removedfilters;
+
        for (std::vector<FilterResult>::iterator filter = filters.begin(); filter != filters.end(); )
        {
                if (filter->from_config)
                {
-                       ServerInstance->SNO->WriteGlobalSno('f', "Removing filter '" + filter->freeform + "' due to config rehash.");
+                       removedfilters.insert(filter->freeform);
                        delete filter->regex;
                        filter = filters.erase(filter);
                        continue;
@@ -836,15 +838,17 @@ void ModuleFilter::ReadFilters()
                if (!StringToFilterAction(action, fa))
                        fa = FA_NONE;
 
-               try
-               {
-                       filters.push_back(FilterResult(RegexEngine, pattern, reason, fa, duration, flgs, true));
-                       ServerInstance->Logs->Log(MODNAME, LOG_DEFAULT, "Regular expression %s loaded.", pattern.c_str());
-               }
-               catch (ModuleException &e)
-               {
-                       ServerInstance->Logs->Log(MODNAME, LOG_DEFAULT, "Error in regular expression '%s': %s", pattern.c_str(), e.GetReason().c_str());
-               }
+               std::pair<bool, std::string> result = static_cast<ModuleFilter*>(this)->AddFilter(pattern, fa, reason, duration, flgs, true);
+               if (result.first)
+                       removedfilters.erase(pattern);
+               else
+                       ServerInstance->Logs->Log(MODNAME, LOG_DEFAULT, "Filter '%s' could not be added: %s", pattern.c_str(), result.second.c_str());
+       }
+
+       if (!removedfilters.empty())
+       {
+               for (insp::flat_set<std::string>::const_iterator it = removedfilters.begin(); it != removedfilters.end(); ++it)
+                       ServerInstance->SNO->WriteGlobalSno('f', "Removing filter '" + *(it) + "' due to config rehash.");
        }
 }
 

diff --git a/src/modules/m_xline_db.cpp b/src/modules/m_xline_db.cpp
index 90f9de9d2958f1a98e5b489b7b709478da9f6c1d..a64dc7071185162bd52df98002d2d196defdf0e2 100644 (file)
--- a/src/modules/m_xline_db.cpp
+++ b/src/modules/m_xline_db.cpp
@@ -51,7 +51,8 @@ class ModuleXLineDB : public Module
         */
        void OnAddLine(User* source, XLine* line) CXX11_OVERRIDE
        {
-               dirty = true;
+               if (!line->from_config)
+                       dirty = true;
        }
 
        /** Called whenever an xline is deleted.
@@ -61,7 +62,8 @@ class ModuleXLineDB : public Module
         */
        void OnDelLine(User* source, XLine* line) CXX11_OVERRIDE
        {
-               dirty = true;
+               if (!line->from_config)
+                       dirty = true;
        }
 
        void OnBackgroundTimer(time_t now) CXX11_OVERRIDE
@@ -113,6 +115,9 @@ class ModuleXLineDB : public Module
                        for (LookupIter i = lookup->begin(); i != lookup->end(); ++i)
                        {
                                XLine* line = i->second;
+                               if (line->from_config)
+                                       continue;
+
                                stream << "LINE " << line->type << " " << line->Displayable() << " "
                                        << line->source << " " << line->set_time << " "
                                        << line->duration << " :" << line->reason << std::endl;

diff --git a/src/xline.cpp b/src/xline.cpp
index c5b95208736c428562dcd4de88938e0e2011a049..fbb4f0c8b47aeb6045e77b56f74e979c5b83378d 100644 (file)
--- a/src/xline.cpp
+++ b/src/xline.cpp
@@ -265,12 +265,31 @@ bool XLineManager::AddLine(XLine* line, User* user)
                LookupIter i = x->second.find(line->Displayable());
                if (i != x->second.end())
                {
-                       // XLine propagation bug was here, if the line to be added already exists and
-                       // it's expired then expire it and add the new one instead of returning false
-                       if ((!i->second->duration) || (ServerInstance->Time() < i->second->expiry))
-                               return false;
+                       bool silent = false;
 
-                       ExpireLine(x, i);
+                       // Allow replacing a config line for an updated config line.
+                       if (i->second->from_config && line->from_config)
+                       {
+                               // Nothing changed, skip adding this one.
+                               if (i->second->reason == line->reason)
+                                       return false;
+
+                               silent = true;
+                       }
+                       // Allow replacing a non-config line for a new config line.
+                       else if (!line->from_config)
+                       {
+                               // X-line propagation bug was here, if the line to be added already exists and
+                               // it's expired then expire it and add the new one instead of returning false
+                               if ((!i->second->duration) || (ServerInstance->Time() < i->second->expiry))
+                                       return false;
+                       }
+                       else
+                       {
+                               silent = true;
+                       }
+
+                       ExpireLine(x, i, silent);
                }
        }
 
@@ -403,11 +422,13 @@ XLine* XLineManager::MatchesLine(const std::string &type, const std::string &pat
 }
 
 // removes lines that have expired
-void XLineManager::ExpireLine(ContainerIter container, LookupIter item)
+void XLineManager::ExpireLine(ContainerIter container, LookupIter item, bool silent)
 {
        FOREACH_MOD(OnExpireLine, (item->second));
 
-       item->second->DisplayExpiry();
+       if (!silent)
+               item->second->DisplayExpiry();
+
        item->second->Unset();
 
        /* TODO: Can we skip this loop by having a 'pending' field in the XLine class, which is set when a line
@@ -750,23 +771,23 @@ XLineFactory* XLineManager::GetFactory(const std::string &type)
        return n->second;
 }
 
-void XLineManager::ClearConfigLines()
+void XLineManager::ExpireRemovedConfigLines(const std::string& type, const insp::flat_set<std::string>& configlines)
 {
        // Nothing to do.
        if (lookup_lines.empty())
                return;
 
-       ServerInstance->SNO->WriteToSnoMask('x', "Server rehashing; expiring lines defined in the server config ...");
-       for (ContainerIter type = lookup_lines.begin(); type != lookup_lines.end(); ++type)
+       ContainerIter xlines = lookup_lines.find(type);
+       if (xlines == lookup_lines.end())
+               return;
+
+       for (LookupIter xline = xlines->second.begin(); xline != xlines->second.end(); )
        {
-               for (LookupIter xline = type->second.begin(); xline != type->second.end(); )
-               {
-                       // We cache this to avoid iterator invalidation.
-                       LookupIter cachedxline = xline++;
-                       if (cachedxline->second->from_config)
-                       {
-                               ExpireLine(type, cachedxline);
-                       }
-               }
+               LookupIter cachedxline = xline++;
+               if (!cachedxline->second->from_config)
+                       continue;
+
+               if (!configlines.count(cachedxline->second->Displayable()))
+                       ExpireLine(xlines, cachedxline);
        }
 }