Safely read override values when no config file is present (#2727)

One of the toml read methods was not reading within a try/catch. Re-arranged the methods slightly to avoid duplicating code.

There was also this check before setting the error when reading from a stream:
```
auto pos (stream.tellg ());
if (pos != std::streampos (0))
```

Removing this check has all tests pass nonetheless, and now only the lowest level method is responsible for setting the internal tree and error.

nano/core_test/toml.cpp

@@ -798,3 +798,47 @@ TEST (toml, daemon_config_deserialize_errors)
 	ASSERT_EQ (toml2.get_error ().get_message (), "frontiers_confirmation value is invalid (available: always, auto, disabled)");
 	ASSERT_EQ (conf2.node.frontiers_confirmation, nano::frontiers_confirmation_mode::invalid);
 }
+
+TEST (toml, daemon_read_config)
+{
+	auto path (nano::unique_path ());
+	boost::filesystem::create_directories (path);
+	nano::daemon_config config;
+	std::vector<std::string> invalid_overrides1{ "node.max_work_generate_multiplier=0" };
+	std::string expected_message1{ "max_work_generate_multiplier must be greater than or equal to 1" };
+
+	std::vector<std::string> invalid_overrides2{ "node.websocket.enable=true", "node.foo" };
+	std::string expected_message2{ "Value must follow after a '=' at line 2" };
+
+	// Reading when there is no config file
+	ASSERT_FALSE (boost::filesystem::exists (nano::get_node_toml_config_path (path)));
+	ASSERT_FALSE (nano::read_node_config_toml (path, config));
+	{
+		auto error = nano::read_node_config_toml (path, config, invalid_overrides1);
+		ASSERT_TRUE (error);
+		ASSERT_EQ (error.get_message (), expected_message1);
+	}
+	{
+		auto error = nano::read_node_config_toml (path, config, invalid_overrides2);
+		ASSERT_TRUE (error);
+		ASSERT_EQ (error.get_message (), expected_message2);
+	}
+
+	// Create an empty config
+	nano::tomlconfig toml;
+	toml.write (nano::get_node_toml_config_path (path));
+
+	// Reading when there is a config file
+	ASSERT_TRUE (boost::filesystem::exists (nano::get_node_toml_config_path (path)));
+	ASSERT_FALSE (nano::read_node_config_toml (path, config));
+	{
+		auto error = nano::read_node_config_toml (path, config, invalid_overrides1);
+		ASSERT_TRUE (error);
+		ASSERT_EQ (error.get_message (), expected_message1);
+	}
+	{
+		auto error = nano::read_node_config_toml (path, config, invalid_overrides2);
+		ASSERT_TRUE (error);
+		ASSERT_EQ (error.get_message (), expected_message2);
+	}
+}

nano/lib/rpcconfig.cpp

@@ -270,7 +270,7 @@ nano::error read_rpc_config_toml (boost::filesystem::path const & data_path_a, n
 		}
 		else
 		{
-			toml.read (config_overrides_stream);
+			error = toml.read (config_overrides_stream);
 		}
 	}
 

nano/lib/tomlconfig.cpp

@@ -23,10 +23,6 @@ void nano::tomlconfig::doc (std::string const & key, std::string const & doc)
 	tree->document (key, doc);
 }
 
-/**
-	 * Reads a json object from the stream
-	 * @return nano::error&, including a descriptive error message if the config file is malformed.
-	 */
 nano::error & nano::tomlconfig::read (boost::filesystem::path const & path_a)
 {
 	std::stringstream stream_override_empty;
@@ -40,34 +36,30 @@ nano::error & nano::tomlconfig::read (std::istream & stream_overrides, boost::fi
 	open_or_create (stream, path_a.string ());
 	if (!stream.fail ())
 	{
-		try
-		{
-			read (stream_overrides, stream);
-		}
-		catch (std::runtime_error const & ex)
-		{
-			auto pos (stream.tellg ());
-			if (pos != std::streampos (0))
-			{
-				*error = ex;
-			}
-		}
-		stream.close ();
+		read (stream_overrides, stream);
 	}
 	return *error;
 }
 
-/** Read from two streams where keys in the first will take precedence over those in the second stream. */
-void nano::tomlconfig::read (std::istream & stream_first_a, std::istream & stream_second_a)
-{
-	tree = cpptoml::parse_base_and_override_files (stream_first_a, stream_second_a, cpptoml::parser::merge_type::ignore, true);
-}
-
-void nano::tomlconfig::read (std::istream & stream_a)
+nano::error & nano::tomlconfig::read (std::istream & stream_a)
 {
 	std::stringstream stream_override_empty;
 	stream_override_empty << std::endl;
-	tree = cpptoml::parse_base_and_override_files (stream_override_empty, stream_a, cpptoml::parser::merge_type::ignore, true);
+	return read (stream_override_empty, stream_a);
+}
+
+/** Read from two streams where keys in the first will take precedence over those in the second stream. */
+nano::error & nano::tomlconfig::read (std::istream & stream_first_a, std::istream & stream_second_a)
+{
+	try
+	{
+		tree = cpptoml::parse_base_and_override_files (stream_first_a, stream_second_a, cpptoml::parser::merge_type::ignore, true);
+	}
+	catch (std::runtime_error const & ex)
+	{
+		*error = ex;
+	}
+	return *error;
 }
 
 void nano::tomlconfig::write (boost::filesystem::path const & path_a)

nano/lib/tomlconfig.hpp

@@ -33,8 +33,8 @@ public:
 	void doc (std::string const & key, std::string const & doc);
 	nano::error & read (boost::filesystem::path const & path_a);
 	nano::error & read (std::istream & stream_overrides, boost::filesystem::path const & path_a);
-	void read (std::istream & stream_first_a, std::istream & stream_second_a);
-	void read (std::istream & stream_a);
+	nano::error & read (std::istream & stream_a);
+	nano::error & read (std::istream & stream_first_a, std::istream & stream_second_a);
 	void write (boost::filesystem::path const & path_a);
 	void write (std::ostream & stream_a) const;
 	void open_or_create (std::fstream & stream_a, std::string const & path_a);

nano/node/daemonconfig.cpp

@@ -223,7 +223,7 @@ nano::error nano::read_node_config_toml (boost::filesystem::path const & data_pa
 		}
 		else
 		{
-			toml.read (config_overrides_stream);
+			error = toml.read (config_overrides_stream);
 		}
 	}