From: Razvan Becheriu Date: Fri, 16 May 2025 16:30:02 +0000 (+0300) Subject: backport #3831 to 2_6 X-Git-Tag: Kea-2.6.3~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b8f1831af8acc36575b58298de14d6f7a2b2e22e;p=thirdparty%2Fkea.git backport #3831 to 2_6 --- diff --git a/doc/sphinx/arm/ctrl-channel.rst b/doc/sphinx/arm/ctrl-channel.rst index 227ad94420..c649cf541b 100644 --- a/doc/sphinx/arm/ctrl-channel.rst +++ b/doc/sphinx/arm/ctrl-channel.rst @@ -511,6 +511,12 @@ An example command invocation looks like this: } } +.. note:: + + As of Kea 2.6.3, the config file file may only be written to the same + directory as the config file used when starting Kea (passed as a ``-c`` + argument). + .. isccmd:: leases-reclaim .. _command-leases-reclaim: diff --git a/src/bin/agent/tests/ca_controller_unittests.cc b/src/bin/agent/tests/ca_controller_unittests.cc index 9a18c27a0a..44f2b2e4a2 100644 --- a/src/bin/agent/tests/ca_controller_unittests.cc +++ b/src/bin/agent/tests/ca_controller_unittests.cc @@ -515,6 +515,9 @@ TEST_F(CtrlAgentControllerTest, configWrite) { // Now clean up after ourselves. ctrl->registerCommands(); + // Add a config file. + ctrl->setConfigFile(string(TEST_DATA_BUILDDIR) + string("/config.json")); + // First, build the command: string file = string(TEST_DATA_BUILDDIR) + string("/config-write.json"); string cmd_txt = "{ \"command\": \"config-write\" }"; @@ -550,6 +553,54 @@ TEST_F(CtrlAgentControllerTest, configWrite) { ctrl->deregisterCommands(); } +// Tests that config-write fails with a bad path. +TEST_F(CtrlAgentControllerTest, badConfigWrite) { + ASSERT_NO_THROW(initProcess()); + EXPECT_TRUE(checkProcess()); + + // The framework available makes it very difficult to test the actual + // code as CtrlAgentController is not initialized the same way it is + // in production code. In particular, the way CtrlAgentController + // is initialized in tests does not call registerCommands(). + // This is a crude workaround for this problem. Proper solution should + // be developed sooner rather than later. + const DControllerBasePtr& base = getController(); + const CtrlAgentControllerPtr& ctrl + = boost::dynamic_pointer_cast(base); + ASSERT_TRUE(ctrl); + // Now clean up after ourselves. + ctrl->registerCommands(); + + // Add a config file. + ctrl->setConfigFile(string(TEST_DATA_BUILDDIR) + string("/config.json")); + + // First, build the command: + string file("/tmp/config-write.json"); + string cmd_txt = "{ \"command\": \"config-write\" }"; + ConstElementPtr cmd = Element::fromJSON(cmd_txt); + ConstElementPtr params = Element::fromJSON("{\"filename\": \"" + file + "\" }"); + CtrlAgentCommandMgr& mgr_ = CtrlAgentCommandMgr::instance(); + + // Send the command + ConstElementPtr answer = mgr_.handleCommand("config-write", params, cmd); + + // Check that the command failed. + string expected = "not allowed to write config into "; + expected += file; + expected += ": file "; + expected += file; + expected += " must be in the same directory as the config file ("; + expected += string(TEST_DATA_BUILDDIR) + string("/config.json"); + expected += ")"; + checkAnswer(answer, isc::config::CONTROL_RESULT_ERROR, expected); + + // Remove the file. + ::remove(file.c_str()); + + // Now clean up after ourselves. + ctrl->deregisterCommands(); +} + // Tests if config-reload attempts to reload a file and reports that the // file is missing. TEST_F(CtrlAgentControllerTest, configReloadMissingFile) { diff --git a/src/bin/d2/tests/d2_command_unittest.cc b/src/bin/d2/tests/d2_command_unittest.cc index 8b133b174a..1f3a84bd33 100644 --- a/src/bin/d2/tests/d2_command_unittest.cc +++ b/src/bin/d2/tests/d2_command_unittest.cc @@ -1036,6 +1036,9 @@ TEST_F(CtrlChannelD2Test, writeConfigFilename) { EXPECT_NO_THROW(createUnixChannelServer()); string response; + // This is normally set by the command line -c parameter. + server_->setConfigFile("test1.json"); + sendUnixCommand("{ \"command\": \"config-write\", " "\"arguments\": { \"filename\": \"test2.json\" } }", response); @@ -1043,6 +1046,60 @@ TEST_F(CtrlChannelD2Test, writeConfigFilename) { ::remove("test2.json"); } +// Tests if config-write can be called with a valid full path as parameter. +TEST_F(CtrlChannelD2Test, configWriteFullPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("/tmp/test1.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/test2.json\" } }", + response); + + checkConfigWrite(response, CONTROL_RESULT_SUCCESS, "/tmp/test2.json"); + ::remove("/tmp/test2.json"); +} + +// Tests if config-write raises an error with invalid path as parameter. +TEST_F(CtrlChannelD2Test, configWriteBadPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("test1.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/test2.json\" } }", + response); + + string expected = "not allowed to write config into /tmp/test2.json: "; + expected += "file /tmp/test2.json must be in the same directory "; + expected += "as the config file (test1.json)"; + checkConfigWrite(response, CONTROL_RESULT_ERROR, expected); + ::remove("/tmp/test2.json"); +} + +// Tests if config-write raises an error with invalid full path as parameter. +TEST_F(CtrlChannelD2Test, configWriteBadFullPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("/tmp/kea1/test.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/kea2/test.json\" } }", + response); + + string expected = "not allowed to write config into /tmp/kea2/test.json: "; + expected += "file /tmp/kea2/test.json must be in the same directory "; + expected += "as the config file (/tmp/kea1/test.json)"; + checkConfigWrite(response, CONTROL_RESULT_ERROR, expected); + ::remove("/tmp/kea2/test.json"); +} + // Tests if config-reload attempts to reload a file and reports that the // file is missing. TEST_F(CtrlChannelD2Test, configReloadMissingFile) { diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 1374719d9c..d7c0b93147 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -288,11 +288,19 @@ ControlledDhcpv4Srv::commandConfigWriteHandler(const string&, // filename parameter was not specified, so let's use whatever we remember // from the command-line filename = getConfigFile(); - } - - if (filename.empty()) { - return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename." - "Please specify filename explicitly.")); + if (filename.empty()) { + return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename." + "Please specify filename explicitly.")); + } + } else { + try { + checkWriteConfigFile(filename); + } catch (const isc::Exception& ex) { + std::ostringstream msg; + msg << "not allowed to write config into " << filename + << ": " << ex.what(); + return (createAnswer(CONTROL_RESULT_ERROR, msg.str())); + } } // Ok, it's time to write the file. diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 1c8e6dc8ad..a16e53fbb2 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -179,9 +179,9 @@ private: /// current configuration to disk. This command takes one optional /// parameter called filename. If specified, the current configuration /// will be written to that file. If not specified, the file used during - /// Kea start-up will be used. To avoid any exploits, the path is - /// always relative and .. is not allowed in the filename. This is - /// a security measure against exploiting file writes remotely. + /// Kea start-up will be used. To avoid any exploits, the target + /// directory must be the same as a security measure against + /// exploiting file writes remotely. /// /// @param command (ignored) /// @param args may contain optional string argument filename diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index f4960588ef..d99b7b3233 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -1471,6 +1471,9 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configWriteFilename) { createUnixChannelServer(); std::string response; + // This is normally set by the command line -c parameter. + server_->setConfigFile("test1.json"); + sendUnixCommand("{ \"command\": \"config-write\", " "\"arguments\": { \"filename\": \"test2.json\" } }", response); @@ -1478,6 +1481,57 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configWriteFilename) { ::remove("test2.json"); } +// Tests if config-write can be called with a valid full path as parameter. +TEST_F(CtrlChannelDhcpv4SrvTest, configWriteFullPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("/tmp/test1.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/test2.json\" } }", response); + + checkConfigWrite(response, CONTROL_RESULT_SUCCESS, "/tmp/test2.json"); + ::remove("/tmp/test2.json"); +} + +// Tests if config-write raises an error with invalid path as parameter. +TEST_F(CtrlChannelDhcpv4SrvTest, configWriteBadPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("test1.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/test2.json\" } }", response); + + string expected = "not allowed to write config into /tmp/test2.json: "; + expected += "file /tmp/test2.json must be in the same directory "; + expected += "as the config file (test1.json)"; + checkConfigWrite(response, CONTROL_RESULT_ERROR, expected); + ::remove("/tmp/test2.json"); +} + +// Tests if config-write raises an error with invalid full path as parameter. +TEST_F(CtrlChannelDhcpv4SrvTest, configWriteBadFullPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("/tmp/kea1/test.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/kea2/test.json\" } }", response); + + string expected = "not allowed to write config into /tmp/kea2/test.json: "; + expected += "file /tmp/kea2/test.json must be in the same directory "; + expected += "as the config file (/tmp/kea1/test.json)"; + checkConfigWrite(response, CONTROL_RESULT_ERROR, expected); + ::remove("/tmp/kea2/test.json"); +} + // Tests if config-reload attempts to reload a file and reports that the // file is missing. TEST_F(CtrlChannelDhcpv4SrvTest, configReloadMissingFile) { diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 5792653d3b..868e251c78 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -291,11 +291,19 @@ ControlledDhcpv6Srv::commandConfigWriteHandler(const string&, // filename parameter was not specified, so let's use whatever we remember // from the command-line filename = getConfigFile(); - } - - if (filename.empty()) { - return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename." - "Please specify filename explicitly.")); + if (filename.empty()) { + return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename." + "Please specify filename explicitly.")); + } + } else { + try { + checkWriteConfigFile(filename); + } catch (const isc::Exception& ex) { + std::ostringstream msg; + msg << "not allowed to write config into " << filename + << ": " << ex.what(); + return (createAnswer(CONTROL_RESULT_ERROR, msg.str())); + } } // Ok, it's time to write the file. diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index 3d75bae6f9..77d8f450ca 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -179,9 +179,9 @@ private: /// current configuration to disk. This command takes one optional /// parameter called filename. If specified, the current configuration /// will be written to that file. If not specified, the file used during - /// Kea start-up will be used. To avoid any exploits, the path is - /// always relative and .. is not allowed in the filename. This is - /// a security measure against exploiting file writes remotely. + /// Kea start-up will be used. To avoid any exploits, the target + /// directory must be the same as a security measure against + /// exploiting file writes remotely. /// /// @param command (ignored) /// @param args may contain optional string argument filename diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 87bee8e493..21f8502062 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -1491,6 +1491,9 @@ TEST_F(CtrlChannelDhcpv6SrvTest, configWriteFilename) { createUnixChannelServer(); std::string response; + // This is normally set by the command line -c parameter. + server_->setConfigFile("test1.json"); + sendUnixCommand("{ \"command\": \"config-write\", " "\"arguments\": { \"filename\": \"test2.json\" } }", response); @@ -1498,6 +1501,57 @@ TEST_F(CtrlChannelDhcpv6SrvTest, configWriteFilename) { ::remove("test2.json"); } +// Tests if config-write can be called with a valid full path as parameter. +TEST_F(CtrlChannelDhcpv6SrvTest, configWriteFullPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("/tmp/test1.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/test2.json\" } }", response); + + checkConfigWrite(response, CONTROL_RESULT_SUCCESS, "/tmp/test2.json"); + ::remove("/tmp/test2.json"); +} + +// Tests if config-write raises an error with invalid path as parameter. +TEST_F(CtrlChannelDhcpv6SrvTest, configWriteBadPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("test1.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/test2.json\" } }", response); + + string expected = "not allowed to write config into /tmp/test2.json: "; + expected += "file /tmp/test2.json must be in the same directory "; + expected += "as the config file (test1.json)"; + checkConfigWrite(response, CONTROL_RESULT_ERROR, expected); + ::remove("/tmp/test2.json"); +} + +// Tests if config-write raises an error with invalid full path as parameter. +TEST_F(CtrlChannelDhcpv6SrvTest, configWriteBadFullPath) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("/tmp/kea1/test.json"); + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"/tmp/kea2/test.json\" } }", response); + + string expected = "not allowed to write config into /tmp/kea2/test.json: "; + expected += "file /tmp/kea2/test.json must be in the same directory "; + expected += "as the config file (/tmp/kea1/test.json)"; + checkConfigWrite(response, CONTROL_RESULT_ERROR, expected); + ::remove("/tmp/kea2/test.json"); +} + // Tests if config-reload attempts to reload a file and reports that the // file is missing. TEST_F(CtrlChannelDhcpv6SrvTest, configReloadMissingFile) { diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc index 825390e163..e201abdbed 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc @@ -3437,13 +3437,13 @@ void Lease4CmdsTest::testLease4Write() { "{\n" " \"command\": \"lease4-write\",\n" " \"arguments\": {" - " \"filename\": \"/tmp/myleases.txt\"\n" + " \"filename\": \"/foo-bar/myleases.txt\"\n" " }\n" "}"; std::ostringstream os; os << "'filename' parameter is invalid: invalid path specified:" - << " '/tmp', supported path is '" << CfgMgr::instance().getDataDir() << "'"; + << " '/foo-bar', supported path is '" << CfgMgr::instance().getDataDir() << "'"; testCommand(txt, CONTROL_RESULT_ERROR, os.str()); } diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc index 2fb2d796a2..ea281b30b8 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc @@ -4172,13 +4172,13 @@ void Lease6CmdsTest::testLease6Write() { "{\n" " \"command\": \"lease6-write\",\n" " \"arguments\": {" - " \"filename\": \"/tmp/myleases.txt\"\n" + " \"filename\": \"/foo-bar/myleases.txt\"\n" " }\n" "}"; std::ostringstream os; os << "'filename' parameter is invalid: invalid path specified:" - << " '/tmp', supported path is '" << CfgMgr::instance().getDataDir() << "'"; + << " '/foo-bar', supported path is '" << CfgMgr::instance().getDataDir() << "'"; } TEST_F(Lease6CmdsTest, lease6AddMissingParams) { diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index 860196ccef..8dd9508526 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -279,6 +279,7 @@ public: } void clear() { + data_dir_env_var_.setValue(); CfgMgr::instance().setFamily(AF_INET); resetDataDir(); CfgMgr::instance().clear(); diff --git a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc index 8bc79f370c..94b5cf7f20 100644 --- a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc @@ -116,6 +116,9 @@ public: extra_files_(), data_dir_env_var_("KEA_DHCP_DATA_DIR") { + // Reset the env variable. + data_dir_env_var_.setValue(); + // Save the pre-test data dir and set it to the test directory. CfgMgr::instance().clear(); original_datadir_ = CfgMgr::instance().getDataDir(); diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc index b5f7e14184..214c68f4d7 100644 --- a/src/lib/process/d_controller.cc +++ b/src/lib/process/d_controller.cc @@ -508,13 +508,22 @@ DControllerBase::configWriteHandler(const std::string&, "Unable to determine filename." "Please specify filename explicitly.")); } + } else { + try { + checkWriteConfigFile(filename); + } catch (const isc::Exception& ex) { + std::ostringstream msg; + msg << "not allowed to write config into " << filename + << ": " << ex.what(); + return (createAnswer(CONTROL_RESULT_ERROR, msg.str())); + } } // Ok, it's time to write the file. size_t size = 0; - ElementPtr cfg = process_->getCfgMgr()->getContext()->toElement(); try { + ElementPtr cfg = process_->getCfgMgr()->getContext()->toElement(); size = writeConfigFile(filename, cfg); } catch (const isc::Exception& ex) { return (createAnswer(CONTROL_RESULT_ERROR, diff --git a/src/lib/process/d_controller.h b/src/lib/process/d_controller.h index 91b777b786..417e21e149 100644 --- a/src/lib/process/d_controller.h +++ b/src/lib/process/d_controller.h @@ -283,9 +283,9 @@ public: /// current configuration to disk. This command takes one optional /// parameter called filename. If specified, the current configuration /// will be written to that file. If not specified, the file used during - /// Kea start-up will be used. To avoid any exploits, the path is - /// always relative and .. is not allowed in the filename. This is - /// a security measure against exploiting file writes remotely. + /// Kea start-up will be used. To avoid any exploits, the target + /// directory must be the same as a security measure against + /// exploiting file writes remotely. /// /// @param command (ignored) /// @param args may contain optional string argument filename diff --git a/src/lib/process/daemon.cc b/src/lib/process/daemon.cc index e201f5e8a5..3f9aae0334 100644 --- a/src/lib/process/daemon.cc +++ b/src/lib/process/daemon.cc @@ -125,6 +125,28 @@ Daemon::checkConfigFile() const { } } +void +Daemon::checkWriteConfigFile(std::string& file) { + Path path(file); + // from checkConfigFile(). + if (path.stem().empty()) { + isc_throw(isc::BadValue, "config file:" << file + << " is missing file name"); + } + Path current(config_file_); + if (current.parentDirectory() == path.parentDirectory()) { + // Same parent directories! + return; + } + if (path.parentDirectory().empty()) { + // Note the current parent directory can't be empty here. + file = current.parentDirectory() + file; + return; + } + isc_throw(isc::BadValue, "file " << file << " must be in the same " + << "directory as the config file (" << config_file_ << ")"); +} + std::string Daemon::getProcName() { return (proc_name_); diff --git a/src/lib/process/daemon.h b/src/lib/process/daemon.h index d64445a77c..83db87e9af 100644 --- a/src/lib/process/daemon.h +++ b/src/lib/process/daemon.h @@ -139,6 +139,15 @@ public: /// @throw BadValue when the configuration file name is bad. void checkConfigFile() const; + /// @brief Checks the to-be-written configuration file name. + /// + /// @note As a side effect prepend the current config file path + /// when the name does not contain a slash. + /// + /// @param[in][out] file Reference to the TBW configuration file name. + /// @throw BadValue when not in the same directory. + void checkWriteConfigFile(std::string& file); + /// @brief Writes current configuration to specified file /// /// This method writes the current configuration to specified file. diff --git a/src/lib/process/tests/daemon_unittest.cc b/src/lib/process/tests/daemon_unittest.cc index ca0bcd1f31..038f6f4905 100644 --- a/src/lib/process/tests/daemon_unittest.cc +++ b/src/lib/process/tests/daemon_unittest.cc @@ -40,8 +40,8 @@ std::string DaemonImpl::getVersion(bool extended) { } } -}; -}; +} +} namespace { @@ -77,7 +77,6 @@ private: std::string env_copy_; }; - // Very simple test. Checks whether Daemon can be instantiated and its // default parameters are sane TEST_F(DaemonTest, constructor) { @@ -113,6 +112,47 @@ TEST_F(DaemonTest, checkConfigFile) { EXPECT_NO_THROW(instance.checkConfigFile()); } +// Verify write config file checker. +TEST_F(DaemonTest, checkWriteConfigFile) { + Daemon instance; + + std::string file(""); + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "/tmp/"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "tmp/"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + instance.setConfigFile("/tmp/foo"); + file = "/foo/bar"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "/tmp/foo/bar"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "/tmp/bar"; + EXPECT_NO_THROW(instance.checkWriteConfigFile(file)); + EXPECT_EQ("/tmp/bar", file); + file = "bar"; + EXPECT_NO_THROW(instance.checkWriteConfigFile(file)); + EXPECT_EQ("/tmp/bar", file); + instance.setConfigFile("tmp/foo"); + file = "/tmp/bar"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "/tmp/foo/bar"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "tmp/bar"; + EXPECT_NO_THROW(instance.checkWriteConfigFile(file)); + EXPECT_EQ("tmp/bar", file); + instance.setConfigFile("foo"); + file = "/tmp/bar"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "tmp/bar"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "foo/bar"; + EXPECT_THROW(instance.checkWriteConfigFile(file), BadValue); + file = "bar"; + EXPECT_NO_THROW(instance.checkWriteConfigFile(file)); + EXPECT_EQ("bar", file); +} + // Verify process name accessors TEST_F(DaemonTest, getSetProcName) { Daemon instance; @@ -318,7 +358,6 @@ TEST_F(DaemonTest, exitValue) { EXPECT_EQ(77, instance.getExitValue()); } - // More tests will appear here as we develop Daemon class. -}; +} diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc index 495091dad2..3b01ee6445 100644 --- a/src/lib/util/filesystem.cc +++ b/src/lib/util/filesystem.cc @@ -81,11 +81,14 @@ setUmask() { } Path::Path(string const& full_name) { + dir_present_ = false; if (!full_name.empty()) { - bool dir_present = false; // Find the directory. size_t last_slash = full_name.find_last_of('/'); if (last_slash != string::npos) { + // Found a directory so note the fact. + dir_present_ = true; + // Found the last slash, so extract directory component and // set where the scan for the last_dot should terminate. parent_path_ = full_name.substr(0, last_slash); @@ -94,14 +97,11 @@ Path::Path(string const& full_name) { // do any more searching. return; } - - // Found a directory so note the fact. - dir_present = true; } // Now search backwards for the last ".". size_t last_dot = full_name.find_last_of('.'); - if ((last_dot == string::npos) || (dir_present && (last_dot < last_slash))) { + if ((last_dot == string::npos) || (dir_present_ && (last_dot < last_slash))) { // Last "." either not found or it occurs to the left of the last // slash if a directory was present (so it is part of a directory // name). In this case, the remainder of the string after the slash @@ -123,7 +123,7 @@ Path::Path(string const& full_name) { string Path::str() const { - return (parent_path_ + ((parent_path_.empty() || parent_path_ == "/") ? string() : "/") + stem_ + extension_); + return (parent_path_ + (dir_present_ ? "/" : "") + stem_ + extension_); } string @@ -131,6 +131,11 @@ Path::parentPath() const { return (parent_path_); } +string +Path::parentDirectory() const { + return (parent_path_ + (dir_present_ ? "/" : "")); +} + string Path::stem() const { return (stem_); @@ -165,10 +170,9 @@ Path::replaceExtension(string const& replacement) { Path& Path::replaceParentPath(string const& replacement) { string const trimmed_replacement(trim(replacement)); - if (trimmed_replacement.empty()) { + dir_present_ = (trimmed_replacement.find_last_of('/') != string::npos); + if (trimmed_replacement.empty() || (trimmed_replacement == "/")) { parent_path_ = string(); - } else if (trimmed_replacement == "/") { - parent_path_ = trimmed_replacement; } else if (trimmed_replacement.at(trimmed_replacement.size() - 1) == '/') { parent_path_ = trimmed_replacement.substr(0, trimmed_replacement.size() - 1); } else { diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h index eaf2701b08..951dd622db 100644 --- a/src/lib/util/filesystem.h +++ b/src/lib/util/filesystem.h @@ -75,6 +75,14 @@ struct Path { /// @return parent path of current path. std::string parentPath() const; + /// @brief Get the parent directory. + /// + /// Empty if no directory is present, the parent path follwed by + /// a slash otherwise. + /// + /// @return parent directory of current path. + std::string parentDirectory() const; + /// @brief Get the base name of the file without the extension. /// /// Counterpart for std::filesystem::path::stem. @@ -121,6 +129,9 @@ struct Path { Path& replaceParentPath(std::string const& replacement = std::string()); private: + /// @brief Is a directory present. + bool dir_present_; + /// @brief Parent path. std::string parent_path_; diff --git a/src/lib/util/tests/filesystem_unittests.cc b/src/lib/util/tests/filesystem_unittests.cc index b246f5bdb3..40723726c1 100644 --- a/src/lib/util/tests/filesystem_unittests.cc +++ b/src/lib/util/tests/filesystem_unittests.cc @@ -109,9 +109,55 @@ TEST(PathTest, components) { Path fname("/alpha/beta/gamma.delta"); EXPECT_EQ("/alpha/beta/gamma.delta", fname.str()); EXPECT_EQ("/alpha/beta", fname.parentPath()); + EXPECT_EQ("/alpha/beta/", fname.parentDirectory()); EXPECT_EQ("gamma", fname.stem()); EXPECT_EQ(".delta", fname.extension()); EXPECT_EQ("gamma.delta", fname.filename()); + + // The root. + Path root("/"); + EXPECT_EQ("/", root.str()); + EXPECT_EQ("", root.parentPath()); + EXPECT_EQ("/", root.parentDirectory()); + EXPECT_EQ("", root.stem()); + EXPECT_EQ("", root.extension()); + EXPECT_EQ("", root.filename()); + + // In the root directory. + Path inroot("/gamma.delta"); + EXPECT_EQ("/gamma.delta", inroot.str()); + EXPECT_EQ("", inroot.parentPath()); + EXPECT_EQ("/", inroot.parentDirectory()); + EXPECT_EQ("gamma", inroot.stem()); + EXPECT_EQ(".delta", inroot.extension()); + EXPECT_EQ("gamma.delta", inroot.filename()); + + // No directory. + Path nodir("gamma.delta"); + EXPECT_EQ("gamma.delta", nodir.str()); + EXPECT_EQ("", nodir.parentPath()); + EXPECT_EQ("", nodir.parentDirectory()); + EXPECT_EQ("gamma", nodir.stem()); + EXPECT_EQ(".delta", nodir.extension()); + EXPECT_EQ("gamma.delta", nodir.filename()); + + // Relative name. + Path relative("../alpha/gamma.delta"); + EXPECT_EQ("../alpha/gamma.delta", relative.str()); + EXPECT_EQ("../alpha", relative.parentPath()); + EXPECT_EQ("../alpha/", relative.parentDirectory()); + EXPECT_EQ("gamma", relative.stem()); + EXPECT_EQ(".delta", relative.extension()); + EXPECT_EQ("gamma.delta", relative.filename()); + + // Multiple extensions. + Path extensions("/alpha/beta/gamma.delta.epsilon"); + EXPECT_EQ("/alpha/beta/gamma.delta.epsilon", extensions.str()); + EXPECT_EQ("/alpha/beta", extensions.parentPath()); + EXPECT_EQ("/alpha/beta/", extensions.parentDirectory()); + EXPECT_EQ("gamma.delta", extensions.stem()); + EXPECT_EQ(".epsilon", extensions.extension()); + EXPECT_EQ("gamma.delta.epsilon", extensions.filename()); } /// @brief Check replaceExtension. @@ -131,30 +177,37 @@ TEST(PathTest, replaceExtension) { TEST(PathTest, replaceParentPath) { Path fname("a.b"); EXPECT_EQ("", fname.parentPath()); + EXPECT_EQ("", fname.parentDirectory()); EXPECT_EQ("a.b", fname.str()); fname.replaceParentPath("/just/some/dir/"); EXPECT_EQ("/just/some/dir", fname.parentPath()); + EXPECT_EQ("/just/some/dir/", fname.parentDirectory()); EXPECT_EQ("/just/some/dir/a.b", fname.str()); fname.replaceParentPath("/just/some/dir"); EXPECT_EQ("/just/some/dir", fname.parentPath()); + EXPECT_EQ("/just/some/dir/", fname.parentDirectory()); EXPECT_EQ("/just/some/dir/a.b", fname.str()); fname.replaceParentPath("/"); - EXPECT_EQ("/", fname.parentPath()); + EXPECT_EQ("", fname.parentPath()); + EXPECT_EQ("/", fname.parentDirectory()); EXPECT_EQ("/a.b", fname.str()); fname.replaceParentPath(""); EXPECT_EQ("", fname.parentPath()); + EXPECT_EQ("", fname.parentDirectory()); EXPECT_EQ("a.b", fname.str()); fname = Path("/first/a.b"); EXPECT_EQ("/first", fname.parentPath()); + EXPECT_EQ("/first/", fname.parentDirectory()); EXPECT_EQ("/first/a.b", fname.str()); fname.replaceParentPath("/just/some/dir"); EXPECT_EQ("/just/some/dir", fname.parentPath()); + EXPECT_EQ("/just/some/dir/", fname.parentDirectory()); EXPECT_EQ("/just/some/dir/a.b", fname.str()); }