diff --git a/SilKit/source/config/Test_YamlParser.cpp b/SilKit/source/config/Test_YamlParser.cpp index 0b3e0da7c..ab5ac1aac 100644 --- a/SilKit/source/config/Test_YamlParser.cpp +++ b/SilKit/source/config/Test_YamlParser.cpp @@ -641,4 +641,145 @@ TEST_F(Test_YamlParser, yaml_throw_on_missing_keyword) { SilKit::ConfigurationError); } +TEST_F(Test_YamlParser, yaml_throw_on_healthcheck_as_sequence) +{ + // HealthCheck is a mapping; the leading "- " mistypes it as a single-element list. + auto healthCheckAsSequence = R"( +HealthCheck: + - SoftResponseTimeout: 1 +)"; + + EXPECT_THROW( + { + auto cfg = Deserialize(healthCheckAsSequence); + }, + SilKit::ConfigurationError); +} + +TEST_F(Test_YamlParser, yaml_throw_on_object_as_sequence) +{ + // A mapping mistyped as a sequence must be rejected at every nesting level. + const std::initializer_list mistypedConfigurations = { + R"( +HealthCheck: + - SoftResponseTimeout: 1 +)", + R"( +Logging: + - FlushLevel: Critical +)", + R"( +Middleware: + - RegistryUri: silkit://localhost:8500 +)", + R"( +Experimental: + - Metrics: + CollectFromRemote: true +)", + R"( +CanControllers: +- Name: CAN1 + Replay: + - UseTraceSource: Source1 +)", + R"( +Experimental: + Metrics: + - CollectFromRemote: true +)", + }; + + for (const auto configuration : mistypedConfigurations) + { + EXPECT_THROW( + { + auto&& cfg = Deserialize(configuration); + (void)cfg; + }, + SilKit::ConfigurationError); + } +} + +TEST_F(Test_YamlParser, yaml_throw_on_stream_document_as_sequence) +{ + // A single "---" document that is itself a bare sequence is the same mistype. + auto streamDocAsSequence = R"(--- +- SoftResponseTimeout: 1 +)"; + + EXPECT_THROW( + { + auto&& cfg = Deserialize(streamDocAsSequence); + (void)cfg; + }, + SilKit::ConfigurationError); +} + +TEST_F(Test_YamlParser, yaml_throw_on_multi_document_stream) +{ + // A configuration must be a single YAML document; two "---" documents are rejected. + auto multiDocument = R"(--- +ParticipantName: Node0 +--- +ParticipantName: Node1 +)"; + + EXPECT_THROW( + { + auto&& cfg = Deserialize(multiDocument); + (void)cfg; + }, + SilKit::ConfigurationError); +} + +TEST_F(Test_YamlParser, yaml_healthcheck_as_map_is_accepted) +{ + // Regression guard: the correctly-typed object must still parse. + auto config = Deserialize(R"( +HealthCheck: + SoftResponseTimeout: 1 + HardResponseTimeout: 2 +)"); + + ASSERT_TRUE(config.healthCheck.softResponseTimeout.has_value()); + ASSERT_TRUE(config.healthCheck.hardResponseTimeout.has_value()); + EXPECT_EQ(config.healthCheck.softResponseTimeout.value(), 1ms); + EXPECT_EQ(config.healthCheck.hardResponseTimeout.value(), 2ms); +} + +TEST_F(Test_YamlParser, yaml_empty_object_keeps_defaults) +{ + // An empty object (null node) keeps its defaults and must not throw. + ParticipantConfiguration defaults{}; + + auto config = Deserialize(R"( +HealthCheck: +)"); + + EXPECT_EQ(config.healthCheck.softResponseTimeout, defaults.healthCheck.softResponseTimeout); + EXPECT_EQ(config.healthCheck.hardResponseTimeout, defaults.healthCheck.hardResponseTimeout); +} + +TEST_F(Test_YamlParser, yaml_sequence_typed_fields_still_parse) +{ + // Genuinely sequence-typed fields (lists of objects) must keep working. + auto config = Deserialize(R"( +CanControllers: +- Name: CAN1 + Network: CAN2 +- Name: CAN2 +LinControllers: +- Name: LIN1 +)"); + + ASSERT_EQ(config.canControllers.size(), 2u); + EXPECT_EQ(config.canControllers.at(0).name, "CAN1"); + ASSERT_TRUE(config.canControllers.at(0).network.has_value()); + EXPECT_EQ(config.canControllers.at(0).network.value(), "CAN2"); + EXPECT_EQ(config.canControllers.at(1).name, "CAN2"); + ASSERT_EQ(config.linControllers.size(), 1u); + EXPECT_EQ(config.linControllers.at(0).name, "LIN1"); +} + } // anonymous namespace diff --git a/SilKit/source/config/Test_YamlValidator.cpp b/SilKit/source/config/Test_YamlValidator.cpp index bac23c1e7..b8380f807 100644 --- a/SilKit/source/config/Test_YamlValidator.cpp +++ b/SilKit/source/config/Test_YamlValidator.cpp @@ -40,6 +40,38 @@ TEST_F(Test_YamlValidator, validate_without_warnings) EXPECT_TRUE(warnings.empty()) << "Warnings: " << warnings; } +TEST_F(Test_YamlValidator, validate_rejects_multi_document_stream) +{ + // A stray "---" mid-config splits it into two documents; the validator must reject it. + auto yamlString = R"yaml(Description: Log to Stdout with Level Info +Logging: +--- + Sinks: + - Level: Info + Type: Stdout +)yaml"; + + std::stringstream warnings; + bool yamlValid = ValidateWithSchema(yamlString, warnings); + EXPECT_FALSE(yamlValid) << "A multi-document stream must not validate"; + EXPECT_THAT(warnings.str(), testing::HasSubstr("one YAML document")); +} + +TEST_F(Test_YamlValidator, validate_accepts_single_leading_document_marker) +{ + // A single leading "---" is one document and stays valid. + auto yamlString = R"yaml(--- +schemaVersion: 1 +ParticipantName: CanDemoParticipant +)yaml"; + + std::stringstream warnings; + bool yamlValid = ValidateWithSchema(yamlString, warnings); + EXPECT_TRUE(yamlValid) << "A single leading '---' document must validate. Warnings: " + << warnings.str(); + EXPECT_TRUE(warnings.str().empty()) << "Warnings: " << warnings.str(); +} + TEST_F(Test_YamlValidator, validate_unknown_toplevel) { auto yamlString = R"yaml( diff --git a/SilKit/source/config/YamlParser.hpp b/SilKit/source/config/YamlParser.hpp index 7ccfee96c..b5688a44f 100644 --- a/SilKit/source/config/YamlParser.hpp +++ b/SilKit/source/config/YamlParser.hpp @@ -46,6 +46,8 @@ auto Deserialize(const std::string& input) -> T // Extract a reference to the root node of the document tree. auto root = tree.crootref(); + VSilKit::EnsureSingleDocument(root); + R reader{parser, root}; T result{}; reader.Read(result); diff --git a/SilKit/source/config/YamlParserUtils.cpp b/SilKit/source/config/YamlParserUtils.cpp index 60a462622..e2b8712c5 100644 --- a/SilKit/source/config/YamlParserUtils.cpp +++ b/SilKit/source/config/YamlParserUtils.cpp @@ -69,6 +69,16 @@ auto GetRapidyamlCallbacks() -> ryml::Callbacks return ryml::Callbacks{nullptr, RapidyamlAllocate, RapidyamlFree, RapidyamlError}; } +void EnsureSingleDocument(ryml::ConstNodeRef root) +{ + if (root.is_stream() && root.num_children() > 1) + { + throw SilKit::ConfigurationError{ + "configuration must contain exactly one YAML document, but multiple " + "\"---\"-separated documents were found"}; + } +} + } // namespace VSilKit diff --git a/SilKit/source/config/YamlParserUtils.hpp b/SilKit/source/config/YamlParserUtils.hpp index 9adba1d4f..13f11f850 100644 --- a/SilKit/source/config/YamlParserUtils.hpp +++ b/SilKit/source/config/YamlParserUtils.hpp @@ -21,4 +21,7 @@ auto MakeConfigurationError(ryml::Location location, const std::string_view mess auto GetRapidyamlCallbacks() -> ryml::Callbacks; +// Throws unless the configuration is a single YAML document. +void EnsureSingleDocument(ryml::ConstNodeRef root); + } // namespace VSilKit diff --git a/SilKit/source/config/YamlReader.hpp b/SilKit/source/config/YamlReader.hpp index 415fb23b7..454ef47c5 100644 --- a/SilKit/source/config/YamlReader.hpp +++ b/SilKit/source/config/YamlReader.hpp @@ -227,25 +227,49 @@ class BasicYamlReader auto GetChildSafe(const std::string& name) const -> Impl { - if (HasKey(name)) + if (IsMap()) { return MakeImpl(_node.find_child(ryml::to_csubstr(name))); } - if (IsSequence()) + if (_node.is_stream()) { - for (const auto& child : _node.cchildren()) + // Single-document stream (leading "---"): descend into the document. A + // document that is a bare sequence is the same mistype rejected below. + for (const auto& doc : _node.cchildren()) { - if (child.is_container() && HasKey(child, name)) + if (doc.is_seq()) + { + ThrowExpectedMapping(name); + } + if (doc.is_map()) { - return MakeImpl(child.find_child(ryml::to_csubstr(name))); + auto&& found = doc.find_child(ryml::to_csubstr(name)); + if (!found.invalid()) + { + return MakeImpl(found); + } } } + return MakeImpl({}); + } + + if (IsSequence()) + { + ThrowExpectedMapping(name); } + // Absent, null or empty node: treat the key as not present. return MakeImpl({}); } + [[noreturn]] void ThrowExpectedMapping(const std::string& name) const + { + std::ostringstream s; + s << "expected a mapping with key \"" << name << "\", but got a sequence"; + throw MakeConfigurationError(s.str()); + } + auto MakeImpl(ryml::ConstNodeRef node_) const -> Impl { return Impl{_parser, node_}; diff --git a/SilKit/source/config/YamlValidator.cpp b/SilKit/source/config/YamlValidator.cpp index 03498847a..65dab13d2 100644 --- a/SilKit/source/config/YamlValidator.cpp +++ b/SilKit/source/config/YamlValidator.cpp @@ -631,6 +631,9 @@ bool ValidateWithSchema(const std::string& yamlString, std::ostream& warnings) auto tree = ryml::parse_in_arena(&parser, cinput); auto root = tree.crootref(); + + VSilKit::EnsureSingleDocument(root); + if (root.is_doc() && (root.is_map() || root.is_seq())) { std::string version; diff --git a/docs/changelog/versions/latest.md b/docs/changelog/versions/latest.md index f34bd81bd..c5e897cf6 100644 --- a/docs/changelog/versions/latest.md +++ b/docs/changelog/versions/latest.md @@ -11,4 +11,9 @@ - `config`: default format of file logging is set to JSON. Set "Format: Simple" in the sink to get previous behaviour. - `docs`: added description of the logging "Format". -- `orchestration`: setting a simulation step duration of zero now raises a `SilKitError` instead of being silently accepted. \ No newline at end of file + +## Fixed + +- `config`: a mapping that is mistyped as a YAML sequence (e.g. a stray leading `- ` turning `HealthCheck` into a list) is now rejected with a clear error instead of being silently accepted. +- `config`: a configuration containing multiple `---`-separated YAML documents is now rejected with a clear error. A single leading `---` remains valid. +- `orchestration`: setting a simulation step duration of zero now raises a `SilKitError` instead of being silently accepted. diff --git a/docs/configuration/configuration.rst b/docs/configuration/configuration.rst index f5f178b92..41abf9c2e 100644 --- a/docs/configuration/configuration.rst +++ b/docs/configuration/configuration.rst @@ -67,9 +67,17 @@ This gives users the ability to run a simulation with multiple instances of a pa Many IDEs automatically support participant configuration schema support when the participant configuration file ends with the suffix ``.silkit.json/yaml``. -A participant configuration file is written in YAML syntax according to a specified schema. -It starts with the ``SchemaVersion``, the ``Description`` for the configuration and the ``ParticipantName``. +A participant configuration file is written in YAML syntax according to a specified schema. +It starts with the ``SchemaVersion``, the ``Description`` for the configuration and the ``ParticipantName``. This is followed by further sections for ``Includes``, ``Middleware``, ``Logging``, ``HealthCheck``, ``Tracing``, ``Extentions`` and sections for the different services of the |ProductName|. + +.. admonition:: Note + + A configuration must be a single YAML document. + A single leading document marker (``---``) is allowed, but a configuration that + contains multiple ``---``-separated documents is rejected with an error. + This most commonly happens when a stray ``---`` is placed in the middle of a file. + The outline of a participant configuration file is as follows: .. code-block:: yaml