Adjust config merge

When the override config has an appSettings/add element that the base config does not have, the base config will copy over the override config element. Before this commit, the override appSettings/add element would be ignored if the base config did not also have the same element.
DigitalRuby · Jun 16, 2022 · e3754e6 · e3754e6
1 parent 87872a4
commit e3754e6
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 80 deletions.
diff --git a/IPBanCore/Core/IPBan/IPBanConfig.cs b/IPBanCore/Core/IPBan/IPBanConfig.cs
@@ -171,7 +171,12 @@ static IPBanConfig()
         private IPBanConfig(XmlDocument doc, IDnsLookup dns = null, IDnsServerList dnsList = null, IHttpRequestMaker httpRequestMaker = null)
         {
             // deserialize with XmlDocument for fine grained control
-            foreach (XmlNode node in doc.SelectNodes("/configuration/appSettings/add"))
+            var appSettingsNodes = doc.SelectNodes("/configuration/appSettings/add");
+            if (appSettingsNodes is null || appSettingsNodes.Count == 0)
+            {
+                throw new InvalidDataException("Configuration is missing or has empty /configuration/appSettings element. This element name is case sensitive. Please check your config.");
+            }
+            foreach (XmlNode node in appSettingsNodes)
             {
                 appSettings[node.Attributes["key"].Value] = node.Attributes["value"].Value;
             }
@@ -761,6 +766,106 @@ public static string ChangeConfigAppSetting(string config, string key, string ne
             return doc.OuterXml;
         }
 
+        /// <summary>
+        /// Merge two configurations
+        /// </summary>
+        /// <param name="xmlBase">Base configuration</param>
+        /// <param name="xmlOverride">Override configuration</param>
+        /// <returns>Merged configuration</returns>
+        /// <exception cref="ArgumentException">Base xml is null or white space</exception>
+        public static XmlDocument MergeXml(string xmlBase, string xmlOverride)
+        {
+            if (string.IsNullOrWhiteSpace(xmlBase))
+            {
+                throw new ArgumentException("Cannot merge null base xml");
+            }
+
+            XmlDocument docBase = new();
+            docBase.LoadXml(xmlBase);
+
+            if (string.IsNullOrWhiteSpace(xmlOverride))
+            {
+                return docBase;
+            }
+
+            XmlDocument docOverride = new();
+            docOverride.LoadXml(xmlOverride);
+
+            XmlNode logFilesOverride = docOverride.SelectSingleNode("/configuration/LogFilesToParse/LogFiles");
+            XmlNode logFilesBase = docBase.SelectSingleNode("/configuration/LogFilesToParse/LogFiles") ?? logFilesOverride;
+            if (logFilesBase is not null &&
+                logFilesOverride is not null &&
+                logFilesBase != logFilesOverride)
+            {
+                foreach (XmlNode overrideNode in logFilesOverride)
+                {
+                    if (overrideNode.NodeType == XmlNodeType.Element)
+                    {
+                        logFilesBase.AppendChild(docBase.ImportNode(overrideNode, true));
+                    }
+                }
+            }
+
+            XmlNode expressionsBlockOverride = docOverride.SelectSingleNode("/configuration/ExpressionsToBlock/Groups");
+            XmlNode expressionsBlockBase = docBase.SelectSingleNode("/configuration/ExpressionsToBlock/Groups") ?? expressionsBlockOverride;
+            if (expressionsBlockBase is not null &&
+                expressionsBlockOverride is not null &&
+                expressionsBlockBase != expressionsBlockOverride)
+            {
+                foreach (XmlNode overrideNode in expressionsBlockOverride)
+                {
+                    if (overrideNode.NodeType == XmlNodeType.Element)
+                    {
+                        expressionsBlockBase.AppendChild(docBase.ImportNode(overrideNode, true));
+                    }
+                }
+            }
+
+            XmlNode expressionsNotifyOverride = docOverride.SelectSingleNode("/configuration/ExpressionsToNotify/Groups");
+            XmlNode expressionsNotifyBase = docBase.SelectSingleNode("/configuration/ExpressionsToNotify/Groups") ?? expressionsNotifyOverride;
+            if (expressionsNotifyBase is not null &&
+                expressionsNotifyOverride is not null &&
+                expressionsNotifyBase != expressionsNotifyOverride)
+            {
+                foreach (XmlNode overrideNode in expressionsNotifyOverride)
+                {
+                    if (overrideNode.NodeType == XmlNodeType.Element)
+                    {
+                        expressionsNotifyBase.AppendChild(docBase.ImportNode(overrideNode, true));
+                    }
+                }
+            }
+
+            XmlNode appSettingsOverride = docOverride.SelectSingleNode("/configuration/appSettings");
+            XmlNode appSettingsBase = docBase.SelectSingleNode("/configuration/appSettings") ?? appSettingsOverride;
+            if (appSettingsBase is not null &&
+                appSettingsOverride is not null &&
+                appSettingsBase != appSettingsOverride)
+            {
+                foreach (XmlNode overrideNode in appSettingsOverride)
+                {
+                    if (overrideNode.NodeType == XmlNodeType.Element)
+                    {
+                        string xpath = $"/configuration/appSettings/add[@key='{overrideNode.Attributes["key"].Value}']";
+                        XmlNode existing = appSettingsBase.SelectSingleNode(xpath);
+                        if (existing is null)
+                        {
+                            // create a new node
+                            appSettingsBase.AppendChild(docBase.ImportNode(overrideNode, true));
+                        }
+                        else
+                        {
+                            // replace existing node
+                            string overrideValue = overrideNode.Attributes["value"]?.Value ?? string.Empty;
+                            existing.Attributes["value"].Value = overrideValue;
+                        }
+                    }
+                }
+            }
+
+            return docBase;
+        }
+
         /// <inheritdoc />
         public bool IsWhitelisted(string entry) => whitelistFilter.IsFiltered(entry);
 

diff --git a/IPBanCore/Core/IPBan/IPBanService_Private.cs b/IPBanCore/Core/IPBan/IPBanService_Private.cs
@@ -56,84 +56,6 @@ private void RunTask(Action action)
             }
         }
 
-        private static XmlDocument MergeXml(string xmlBase, string xmlOverride)
-        {
-            if (string.IsNullOrWhiteSpace(xmlBase))
-            {
-                throw new ArgumentException("Cannot merge null base xml");
-            }
-
-            XmlDocument docBase = new();
-            docBase.LoadXml(xmlBase);
-
-            if (string.IsNullOrWhiteSpace(xmlOverride))
-            {
-                return docBase;
-            }
-
-            XmlDocument docOverride = new();
-            docOverride.LoadXml(xmlOverride);
-
-            XmlNode logFilesBase = docBase.SelectSingleNode("/configuration/LogFilesToParse/LogFiles");
-            XmlNode logFilesOverride = docOverride.SelectSingleNode("/configuration/LogFilesToParse/LogFiles");
-            if (logFilesBase is not null && logFilesOverride is not null)
-            {
-                foreach (XmlNode overrideNode in logFilesOverride)
-                {
-                    if (overrideNode.NodeType == XmlNodeType.Element)
-                    {
-                        logFilesBase.AppendChild(docBase.ImportNode(overrideNode, true));
-                    }
-                }
-            }
-
-            XmlNode expressionsBlockBase = docBase.SelectSingleNode("/configuration/ExpressionsToBlock/Groups");
-            XmlNode expressionsBlockOverride = docOverride.SelectSingleNode("/configuration/ExpressionsToBlock/Groups");
-            if (expressionsBlockBase is not null && expressionsBlockOverride is not null)
-            {
-                foreach (XmlNode overrideNode in expressionsBlockOverride)
-                {
-                    if (overrideNode.NodeType == XmlNodeType.Element)
-                    {
-                        expressionsBlockBase.AppendChild(docBase.ImportNode(overrideNode, true));
-                    }
-                }
-            }
-
-            XmlNode expressionsNotifyBase = docBase.SelectSingleNode("/configuration/ExpressionsToNotify/Groups");
-            XmlNode expressionsNotifyOverride = docOverride.SelectSingleNode("/configuration/ExpressionsToNotify/Groups");
-            if (expressionsNotifyBase is not null && expressionsNotifyOverride is not null)
-            {
-                foreach (XmlNode overrideNode in expressionsNotifyOverride)
-                {
-                    if (overrideNode.NodeType == XmlNodeType.Element)
-                    {
-                        expressionsNotifyBase.AppendChild(docBase.ImportNode(overrideNode, true));
-                    }
-                }
-            }
-
-            XmlNode appSettingsBase = docBase.SelectSingleNode("/configuration/appSettings");
-            XmlNode appSettingsOverride = docOverride.SelectSingleNode("/configuration/appSettings");
-            if (appSettingsBase is not null && appSettingsOverride is not null)
-            {
-                foreach (XmlNode overrideNode in appSettingsOverride)
-                {
-                    if (overrideNode.NodeType == XmlNodeType.Element)
-                    {
-                        string xpath = "/configuration/appSettings/add[@key='" + overrideNode.Attributes["key"].Value + "']";
-                        XmlNode existing = appSettingsBase.SelectSingleNode(xpath);
-                        if (existing != null)
-                        {
-                            existing.Attributes["value"].Value = overrideNode.Attributes["value"].Value;
-                        }
-                    }
-                }
-            }
-
-            return docBase;
-        }
-
         internal async Task UpdateConfiguration()
         {
             try
@@ -147,7 +69,7 @@ internal async Task UpdateConfiguration()
                     // merge override xml
                     string baseXml = configChange ?? Config?.Xml;
                     string overrideXml = configChangeOverride;
-                    XmlDocument finalXml = MergeXml(baseXml, overrideXml);
+                    XmlDocument finalXml = IPBanConfig.MergeXml(baseXml, overrideXml);
                     IPBanConfig oldConfig = Config;
                     IPBanConfig newConfig = IPBanConfig.LoadFromXml(finalXml, DnsLookup, DnsList, RequestMaker);
                     bool configChanged = oldConfig is null || oldConfig.Xml != newConfig.Xml;

diff --git a/IPBanTests/IPBanConfigTests.cs b/IPBanTests/IPBanConfigTests.cs
@@ -33,6 +33,7 @@ MIT License
 using System.Runtime.InteropServices;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
+using System.Xml;
 
 namespace DigitalRuby.IPBanTests
 {
@@ -234,6 +235,23 @@ public async Task TestDefaultConfig()
             }
         }
 
+        /// <summary>
+        /// Test config merge
+        /// </summary>
+        [Test]
+        public void TestConfigMerge()
+        {
+            string config1 = "<?xml version=\"1.0\"?><configuration><appSettings><add key='Whitelist' value='1.1.1.1' /></appSettings></configuration>";
+            string config2 = "<?xml version=\"1.0\"?><configuration><appSettings><add key='Whitelist' value='2.2.2.2' /><add key='Blacklist' value='3.3.3.3' /></appSettings></configuration>";
+            XmlDocument doc = IPBanConfig.MergeXml(config1, config2);
+            var nodes = doc["configuration"]["appSettings"].ChildNodes;
+            Assert.AreEqual(2, nodes.Count);
+            string value1 = nodes[0].Attributes["value"].Value;
+            string value2 = nodes[1].Attributes["value"].Value;
+            Assert.AreEqual("2.2.2.2", value1);
+            Assert.AreEqual("3.3.3.3", value2);
+        }
+
         /// <summary>
         /// Test that we can parse a blacklist or whitelist with comments
         /// </summary>