Fix duplicate extension config entries

daemon/extension/ScriptConfig.cpp

@@ -74,6 +74,14 @@ bool ScriptConfig::LoadConfig(Options::OptEntries* optEntries)
 
 bool ScriptConfig::SaveConfig(Options::OptEntries* optEntries)
 {
+	struct CaseInsensitiveLess
+	{
+		bool operator()(const CString& left, const CString& right) const
+		{
+			return strcasecmp(*left, *right) < 0;
+		}
+	};
+
 	// save to config file
 	DiskFile infile;
 
@@ -83,7 +91,23 @@ bool ScriptConfig::SaveConfig(Options::OptEntries* optEntries)
 	}
 
 	std::vector<CString> config;
-	std::set<Options::OptEntry*> writtenOptions;
+	std::set<CString, CaseInsensitiveLess> writtenOptions;
+
+	// Options::SetOption overwrites the existing entry while parsing, so when a
+	// name occurs on several lines the last one is the value nzbget runs on.
+	// Collapsing duplicates must preserve that value, otherwise saving a damaged
+	// config silently changes settings.
+	auto findLastValue = [optEntries](const char* name) -> const char*
+	{
+		for (auto it = optEntries->rbegin(); it != optEntries->rend(); ++it)
+		{
+			if (!strcasecmp(it->GetName(), name))
+			{
+				return it->GetValue();
+			}
+		}
+		return nullptr;
+	};
 
 	// read config file into memory array
 	int fileLen = (int)FileSystem::FileSize(g_Options->GetConfigFilename()) + 1;
@@ -110,10 +134,14 @@ bool ScriptConfig::SaveConfig(Options::OptEntries* optEntries)
 			if (g_Options->SplitOptionString(buf, optname, optvalue))
 			{
 				Options::OptEntry* optEntry = optEntries->FindOption(optname);
-				if (optEntry)
+				// write each option only once, dropping duplicate lines accumulated
+				// in the config file by earlier versions (issue #588); keep the
+				// first occurrence's name and position but the last occurrence's
+				// value, so the collapsed line is the one nzbget was running on
+				if (optEntry && writtenOptions.find(optEntry->GetName()) == writtenOptions.end())
 				{
-					infile.Print("%s=%s\n", optEntry->GetName(), optEntry->GetValue());
-					writtenOptions.insert(optEntry);
+					infile.Print("%s=%s\n", optEntry->GetName(), findLastValue(optEntry->GetName()));
+					writtenOptions.insert(optEntry->GetName());
 				}
 			}
 		}
@@ -126,10 +154,11 @@ bool ScriptConfig::SaveConfig(Options::OptEntries* optEntries)
 	// write new options
 	for (Options::OptEntry& optEntry : *optEntries)
 	{
-		std::set<Options::OptEntry*>::iterator fit = writtenOptions.find(&optEntry);
+		std::set<CString, CaseInsensitiveLess>::iterator fit = writtenOptions.find(optEntry.GetName());
 		if (fit == writtenOptions.end())
 		{
-			infile.Print("%s=%s\n", optEntry.GetName(), optEntry.GetValue());
+			infile.Print("%s=%s\n", optEntry.GetName(), findLastValue(optEntry.GetName()));
+			writtenOptions.insert(optEntry.GetName());
 		}
 	}
 

tests/extension/ScriptConfig.cpp

@@ -0,0 +1,163 @@
+/*
+ *  This file is part of nzbget. See <https://nzbget.com>.
+ *
+ *  Copyright (C) 2026 Denis <denis@nzbget.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ */
+
+#include "nzbget.h"
+
+#include <boost/test/unit_test.hpp>
+#include <fstream>
+#include <sstream>
+#include "Options.h"
+#include "ScriptConfig.h"
+
+BOOST_AUTO_TEST_SUITE(ExtensionTest)
+
+struct TempConfigFile
+{
+	fs::path path = fs::temp_directory_path() / fs::make_unique_filename();
+
+	~TempConfigFile()
+	{
+		fs::error_code error;
+		fs::remove(path, error);
+	}
+};
+
+struct OptionsGuard
+{
+	Options* oldOptions = g_Options;
+
+	~OptionsGuard()
+	{
+		g_Options = oldOptions;
+	}
+};
+
+BOOST_AUTO_TEST_CASE(SaveConfigDoesNotAppendDuplicateOptionNames)
+{
+	TempConfigFile configFile;
+	{
+		std::ofstream output(configFile.path);
+		output << "# existing config\n";
+	}
+
+	OptionsGuard optionsGuard;
+	Options options("nzbget", configFile.path.string().c_str(), true, nullptr, nullptr);
+	g_Options = &options;
+
+	Options::OptEntries optEntries;
+	optEntries.emplace_back("Extension.Option", "first");
+	optEntries.emplace_back("extension.option", "second");
+	ScriptConfig scriptConfig;
+
+	BOOST_REQUIRE(scriptConfig.SaveConfig(&optEntries));
+	std::string firstContents;
+	{
+		std::ifstream firstFile(configFile.path);
+		std::stringstream contents;
+		contents << firstFile.rdbuf();
+		firstContents = contents.str();
+	}
+	// one line only, carrying the last entry's value - Options::SetOption
+	// resolves duplicates the same way, so the file matches what nzbget loads
+	BOOST_CHECK_EQUAL(firstContents, "# existing config\nExtension.Option=second\n");
+
+	BOOST_REQUIRE(scriptConfig.SaveConfig(&optEntries));
+	std::string secondContents;
+	{
+		std::ifstream secondFile(configFile.path);
+		std::stringstream contents;
+		contents << secondFile.rdbuf();
+		secondContents = contents.str();
+	}
+	BOOST_CHECK_EQUAL(secondContents, firstContents);
+}
+
+BOOST_AUTO_TEST_CASE(SaveConfigRemovesDuplicateLinesFromDamagedConfig)
+{
+	TempConfigFile configFile;
+	{
+		std::ofstream output(configFile.path);
+		output << "# existing config\n"
+			<< "Server1.Active=yes\n"
+			<< "Server2.Active=yes\n"
+			<< "Server2.Active=yes\n"
+			<< "server2.active=yes\n"
+			<< "Server2.Host=news.example.com\n";
+	}
+
+	OptionsGuard optionsGuard;
+	Options options("nzbget", configFile.path.string().c_str(), true, nullptr, nullptr);
+	g_Options = &options;
+
+	Options::OptEntries optEntries;
+	optEntries.emplace_back("Server1.Active", "yes");
+	optEntries.emplace_back("Server2.Active", "no");
+	optEntries.emplace_back("Server2.Host", "news.example.com");
+	ScriptConfig scriptConfig;
+
+	BOOST_REQUIRE(scriptConfig.SaveConfig(&optEntries));
+	std::string contents;
+	{
+		std::ifstream file(configFile.path);
+		std::stringstream buffer;
+		buffer << file.rdbuf();
+		contents = buffer.str();
+	}
+	BOOST_CHECK_EQUAL(contents,
+		"# existing config\n"
+		"Server1.Active=yes\n"
+		"Server2.Active=no\n"
+		"Server2.Host=news.example.com\n");
+}
+
+// When a config file carries duplicate lines for one option, Options::SetOption
+// overwrites the same entry while parsing, so the LAST line is the value nzbget
+// actually runs on. Collapsing the duplicates must preserve that value,
+// otherwise saving a damaged config silently changes settings (issue #588).
+BOOST_AUTO_TEST_CASE(SaveConfigKeepsLastValueWhenConfigHasDuplicateLines)
+{
+	TempConfigFile configFile;
+	{
+		std::ofstream output(configFile.path);
+		output << "# existing config\n"
+			<< "Server2.Active=yes\n"
+			<< "Server2.Name=xsusenet\n"
+			<< "server2.active=no\n";
+	}
+
+	OptionsGuard optionsGuard;
+	Options options("nzbget", configFile.path.string().c_str(), true, nullptr, nullptr);
+	g_Options = &options;
+
+	// as loadconfig reports it: raw file lines, duplicates included
+	Options::OptEntries optEntries;
+	optEntries.emplace_back("Server2.Active", "yes");
+	optEntries.emplace_back("Server2.Name", "xsusenet");
+	optEntries.emplace_back("server2.active", "no");
+	ScriptConfig scriptConfig;
+
+	BOOST_REQUIRE(scriptConfig.SaveConfig(&optEntries));
+	std::string contents;
+	{
+		std::ifstream file(configFile.path);
+		std::stringstream buffer;
+		buffer << file.rdbuf();
+		contents = buffer.str();
+	}
+	// one line per option, keeping the first occurrence's name and position
+	// but the last occurrence's value - exactly what Options::SetOption does
+	BOOST_CHECK_EQUAL(contents,
+		"# existing config\n"
+		"Server2.Active=no\n"
+		"Server2.Name=xsusenet\n");
+}
+
+BOOST_AUTO_TEST_SUITE_END()

tests/extension/extension.cmake

@@ -4,6 +4,7 @@ list(APPEND TESTS_SRC
 	${CMAKE_CURRENT_SOURCE_DIR}/extension/Extension.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/extension/ExtensionManager.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/extension/ScanScript.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/extension/ScriptConfig.cpp
 )
 
 file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/testdata/extension/manifest DESTINATION ${CMAKE_CURRENT_BINARY_DIR})