diff --git a/CHANGELOG b/CHANGELOG index efcfe310a2..16be1ff1f7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,25 @@ This is the changelog file for the POCO C++ Libraries. +Release 1.15.3 (Unreleased) +=========================== + +API Changes: + +- GH #5322 PropertyFileConfiguration: the optional parent-configuration + parameter (added in 1.15.1 via #5253) is now an AbstractConfiguration* + raw non-owning pointer instead of AbstractConfiguration::Ptr. Callers + must keep the parent alive for the child's lifetime. This fixes a + circular reference between LayeredConfiguration and child + PropertyFileConfiguration instances in Application::loadConfiguration(). + The Ptr-taking overloads are retained as deprecated forwarders to + surface the lifetime contract at compile time; they will be removed + in a future release. Passing a temporary Ptr would leave _pParentConfig + dangling and should be replaced with a named variable whose lifetime + outlives the child. All three PropertyFileConfiguration constructors + are now explicit to prevent accidental implicit conversions. + + Release 1.15.2 (2026-04-16) =========================== diff --git a/Util/include/Poco/Util/PropertyFileConfiguration.h b/Util/include/Poco/Util/PropertyFileConfiguration.h index 8443cd5e2f..2f79b25d7c 100644 --- a/Util/include/Poco/Util/PropertyFileConfiguration.h +++ b/Util/include/Poco/Util/PropertyFileConfiguration.h @@ -66,22 +66,41 @@ class Util_API PropertyFileConfiguration: public MapConfiguration /// a colon ':' nor an equal sign '=' character. { public: - PropertyFileConfiguration(AbstractConfiguration::Ptr pParentConfig = nullptr); + explicit PropertyFileConfiguration(AbstractConfiguration* pParentConfig = nullptr); /// Creates an empty PropertyFileConfiguration. /// If pParentConfig is not null, it is used to expand ${variable} /// references in !include directive paths. + /// The parent configuration is not owned by this object; + /// the caller must ensure it outlives this configuration. - PropertyFileConfiguration(std::istream& istr, AbstractConfiguration::Ptr pParentConfig = nullptr); + explicit PropertyFileConfiguration(std::istream& istr, AbstractConfiguration* pParentConfig = nullptr); /// Creates an PropertyFileConfiguration and loads the configuration data /// from the given stream, which must be in properties file format. /// If pParentConfig is not null, it is used to expand ${variable} /// references in !include directive paths. + /// The parent configuration is not owned by this object; + /// the caller must ensure it outlives this configuration. - PropertyFileConfiguration(const std::string& path, AbstractConfiguration::Ptr pParentConfig = nullptr); + explicit PropertyFileConfiguration(const std::string& path, AbstractConfiguration* pParentConfig = nullptr); /// Creates an PropertyFileConfiguration and loads the configuration data /// from the given file, which must be in properties file format. /// If pParentConfig is not null, it is used to expand ${variable} /// references in !include directive paths. + /// The parent configuration is not owned by this object; + /// the caller must ensure it outlives this configuration. + + POCO_DEPRECATED("Pass a raw AbstractConfiguration*; PropertyFileConfiguration does not take ownership of the parent") + explicit PropertyFileConfiguration(AbstractConfiguration::Ptr pParentConfig); + /// Deprecated. Use the AbstractConfiguration* overload instead. + /// Forwards to the raw-pointer constructor. + + POCO_DEPRECATED("Pass a raw AbstractConfiguration*; PropertyFileConfiguration does not take ownership of the parent") + PropertyFileConfiguration(std::istream& istr, AbstractConfiguration::Ptr pParentConfig); + /// Deprecated. Use the AbstractConfiguration* overload instead. + + POCO_DEPRECATED("Pass a raw AbstractConfiguration*; PropertyFileConfiguration does not take ownership of the parent") + PropertyFileConfiguration(const std::string& path, AbstractConfiguration::Ptr pParentConfig); + /// Deprecated. Use the AbstractConfiguration* overload instead. void load(std::istream& istr); /// Loads the configuration data from the given stream, which @@ -173,7 +192,12 @@ class Util_API PropertyFileConfiguration: public MapConfiguration static int readChar(std::istream& istr); static std::string escapeValue(const std::string& value); - AbstractConfiguration::Ptr _pParentConfig; + AbstractConfiguration* _pParentConfig = nullptr; + /// Non-owning back-pointer to the parent configuration used for + /// ${variable} expansion in !include directive paths. + /// This is intentionally a raw pointer to avoid a circular reference: + /// the parent (typically LayeredConfiguration) owns this + /// PropertyFileConfiguration, so the parent always outlives the child. std::map _sourceMap; std::string _rootFile; }; diff --git a/Util/src/Application.cpp b/Util/src/Application.cpp index f6dce1a943..69f57bb868 100644 --- a/Util/src/Application.cpp +++ b/Util/src/Application.cpp @@ -203,7 +203,7 @@ int Application::loadConfiguration(int priority) Path confPath; if (findAppConfigFile(appPath.getBaseName(), "properties"s, confPath)) { - _pConfig->add(new PropertyFileConfiguration(confPath.toString(), AbstractConfiguration::Ptr(_pConfig, true)), priority, false); + _pConfig->add(new PropertyFileConfiguration(confPath.toString(), _pConfig.get()), priority, false); ++n; } #ifndef POCO_UTIL_NO_INIFILECONFIGURATION @@ -245,7 +245,7 @@ void Application::loadConfiguration(const std::string& path, int priority) std::string ext = confPath.getExtension(); if (icompare(ext, "properties") == 0) { - _pConfig->add(new PropertyFileConfiguration(confPath.toString(), AbstractConfiguration::Ptr(_pConfig, true)), priority, false); + _pConfig->add(new PropertyFileConfiguration(confPath.toString(), _pConfig.get()), priority, false); ++n; } #ifndef POCO_UTIL_NO_INIFILECONFIGURATION diff --git a/Util/src/PropertyFileConfiguration.cpp b/Util/src/PropertyFileConfiguration.cpp index edab95a2d2..5f75eaf74f 100644 --- a/Util/src/PropertyFileConfiguration.cpp +++ b/Util/src/PropertyFileConfiguration.cpp @@ -34,23 +34,41 @@ namespace Poco::Util { PropertyFileConfiguration::~PropertyFileConfiguration() = default; +PropertyFileConfiguration::PropertyFileConfiguration(AbstractConfiguration* pParentConfig): + _pParentConfig(pParentConfig) +{ +} + + +PropertyFileConfiguration::PropertyFileConfiguration(std::istream& istr, AbstractConfiguration* pParentConfig): + _pParentConfig(pParentConfig) +{ + load(istr); +} + + +PropertyFileConfiguration::PropertyFileConfiguration(const std::string& path, AbstractConfiguration* pParentConfig): + _pParentConfig(pParentConfig) +{ + load(path); +} + + PropertyFileConfiguration::PropertyFileConfiguration(AbstractConfiguration::Ptr pParentConfig): - _pParentConfig(std::move(pParentConfig)) + PropertyFileConfiguration(pParentConfig.get()) { } PropertyFileConfiguration::PropertyFileConfiguration(std::istream& istr, AbstractConfiguration::Ptr pParentConfig): - _pParentConfig(std::move(pParentConfig)) + PropertyFileConfiguration(istr, pParentConfig.get()) { - load(istr); } PropertyFileConfiguration::PropertyFileConfiguration(const std::string& path, AbstractConfiguration::Ptr pParentConfig): - _pParentConfig(std::move(pParentConfig)) + PropertyFileConfiguration(path, pParentConfig.get()) { - load(path); } diff --git a/Util/testsuite/src/PropertyFileConfigurationTest.cpp b/Util/testsuite/src/PropertyFileConfigurationTest.cpp index 54d5f3524f..12132bab65 100644 --- a/Util/testsuite/src/PropertyFileConfigurationTest.cpp +++ b/Util/testsuite/src/PropertyFileConfigurationTest.cpp @@ -339,7 +339,7 @@ void PropertyFileConfigurationTest::testInclude() ostr << "!include ${extDir}" << includedParentName << "\n"; } - AutoPtr pConfParent = new PropertyFileConfiguration(mainFileParent.path(), pParent); + AutoPtr pConfParent = new PropertyFileConfiguration(mainFileParent.path(), pParent.get()); assertTrue (pConfParent->getString("main.prop") == "mainValue"); assertTrue (pConfParent->getString("parent.prop") == "parentValue"); } @@ -812,6 +812,23 @@ void PropertyFileConfigurationTest::testIncludeManagement() } +void PropertyFileConfigurationTest::testNoCircularReference() +{ + using Poco::Util::MapConfiguration; + + AutoPtr pParent = new MapConfiguration; + pParent->setString("base.dir", "/tmp/poco-test"); + assertTrue (pParent->referenceCount() == 1); + + { + AutoPtr pChild = new PropertyFileConfiguration(pParent.get()); + assertTrue (pParent->referenceCount() == 1); + (void) pChild; + } + assertTrue (pParent->referenceCount() == 1); +} + + AbstractConfiguration::Ptr PropertyFileConfigurationTest::allocConfiguration() const { return new PropertyFileConfiguration; @@ -841,6 +858,7 @@ CppUnit::Test* PropertyFileConfigurationTest::suite() CppUnit_addTest(pSuite, PropertyFileConfigurationTest, testClearResetsProvenance); CppUnit_addTest(pSuite, PropertyFileConfigurationTest, testGetSourceFilesCoversAllKeys); CppUnit_addTest(pSuite, PropertyFileConfigurationTest, testIncludeManagement); + CppUnit_addTest(pSuite, PropertyFileConfigurationTest, testNoCircularReference); return pSuite; } diff --git a/Util/testsuite/src/PropertyFileConfigurationTest.h b/Util/testsuite/src/PropertyFileConfigurationTest.h index 84503f3d22..16fb914394 100644 --- a/Util/testsuite/src/PropertyFileConfigurationTest.h +++ b/Util/testsuite/src/PropertyFileConfigurationTest.h @@ -32,6 +32,7 @@ class PropertyFileConfigurationTest: public AbstractConfigurationTest void testClearResetsProvenance(); void testGetSourceFilesCoversAllKeys(); void testIncludeManagement(); + void testNoCircularReference(); void setUp(); void tearDown();