diff --git a/OloEngine/src/OloEngine/Networking/Replication/ComponentReplicator.cpp b/OloEngine/src/OloEngine/Networking/Replication/ComponentReplicator.cpp index cfb833f5c0..644d531437 100644 --- a/OloEngine/src/OloEngine/Networking/Replication/ComponentReplicator.cpp +++ b/OloEngine/src/OloEngine/Networking/Replication/ComponentReplicator.cpp @@ -3,8 +3,31 @@ #include "OloEngine/Scene/Components.h" #include "OloEngine/Serialization/Archive.h" +#include + namespace OloEngine { + namespace + { + // Snapshot bytes arrive from the network, which is untrusted: a NaN / ±inf + // float poisons transform math (and, downstream, integer tick casts — + // see SnapshotInterpolator). Replace any non-finite wire float with a safe + // fallback. See docs/agent-rules/cpp-coding-quality.md §2. + [[nodiscard]] f32 SanitizeWireFloat(f32 value, f32 fallback) + { + return std::isfinite(value) ? value : fallback; + } + + [[nodiscard]] glm::vec3 SanitizeWireVec3(const glm::vec3& value, const glm::vec3& fallback) + { + return { + SanitizeWireFloat(value.x, fallback.x), + SanitizeWireFloat(value.y, fallback.y), + SanitizeWireFloat(value.z, fallback.z), + }; + } + } // namespace + void ComponentReplicator::Serialize(FArchive& ar, TransformComponent& component) { OLO_PROFILE_FUNCTION(); @@ -12,11 +35,14 @@ namespace OloEngine ar << component.Translation.x << component.Translation.y << component.Translation.z; glm::vec3 euler = component.GetRotationEuler(); ar << euler.x << euler.y << euler.z; + ar << component.Scale.x << component.Scale.y << component.Scale.z; if (ar.IsLoading()) { - component.SetRotationEuler(euler); + // Validate every float pulled off the wire before it reaches the scene. + component.Translation = SanitizeWireVec3(component.Translation, glm::vec3(0.0f)); + component.Scale = SanitizeWireVec3(component.Scale, glm::vec3(1.0f)); + component.SetRotationEuler(SanitizeWireVec3(euler, glm::vec3(0.0f))); } - ar << component.Scale.x << component.Scale.y << component.Scale.z; } void ComponentReplicator::Serialize(FArchive& ar, Rigidbody2DComponent& component) @@ -45,6 +71,18 @@ namespace OloEngine ar << component.m_Mass; ar << component.m_InitialLinearVelocity.x << component.m_InitialLinearVelocity.y << component.m_InitialLinearVelocity.z; ar << component.m_InitialAngularVelocity.x << component.m_InitialAngularVelocity.y << component.m_InitialAngularVelocity.z; + if (ar.IsLoading()) + { + // Untrusted wire floats: mass must be finite and strictly positive; velocities finite. + // Zero is rejected too — a dynamic body with zero mass divides by zero in the Jolt + // mass-properties override (JoltBody.cpp), so default it to the safe positive minimum. + if (!std::isfinite(component.m_Mass) || component.m_Mass <= 0.0f) + { + component.m_Mass = 1.0f; + } + component.m_InitialLinearVelocity = SanitizeWireVec3(component.m_InitialLinearVelocity, glm::vec3(0.0f)); + component.m_InitialAngularVelocity = SanitizeWireVec3(component.m_InitialAngularVelocity, glm::vec3(0.0f)); + } } std::unordered_map ComponentReplicator::s_Registry; diff --git a/OloEngine/src/OloEngine/Networking/Replication/SnapshotInterpolator.cpp b/OloEngine/src/OloEngine/Networking/Replication/SnapshotInterpolator.cpp index 9d11d2ced3..e6cf7d76f4 100644 --- a/OloEngine/src/OloEngine/Networking/Replication/SnapshotInterpolator.cpp +++ b/OloEngine/src/OloEngine/Networking/Replication/SnapshotInterpolator.cpp @@ -8,6 +8,7 @@ #include "OloEngine/Serialization/Archive.h" #include +#include #include #include @@ -68,7 +69,11 @@ namespace OloEngine f32 const delayTicks = m_RenderDelay * static_cast(m_ServerTickRate); f32 const renderTick = static_cast(m_LatestReceivedTick) - delayTicks; - if (renderTick < 0.0f) + // Defend the integer cast below: a non-finite renderTick (which a NaN slips + // past `< 0.0f`, since every comparison with NaN is false) would make the + // static_cast undefined behaviour. m_RenderDelay is validated at the + // setter, but guard here too — wire/config data is untrusted. + if (!std::isfinite(renderTick) || renderTick < 0.0f) { return; } @@ -165,6 +170,15 @@ namespace OloEngine void SnapshotInterpolator::SetRenderDelay(f32 seconds) { + // Render delay feeds the tick math in Interpolate()/GetRenderTick(); a + // non-finite or negative value would poison renderTick and the u32 cast + // (NaN slips past `< 0.0f`). Reject it and keep the previous valid delay. + if (!std::isfinite(seconds) || seconds < 0.0f) + { + OLO_CORE_WARN("[SnapshotInterpolator] Ignoring invalid render delay {} (must be finite and >= 0); keeping {}", seconds, m_RenderDelay); + return; + } + m_RenderDelay = seconds; } diff --git a/OloEngine/src/OloEngine/Task/Scheduler.cpp b/OloEngine/src/OloEngine/Task/Scheduler.cpp index 0011cb371d..df9d2f84fc 100644 --- a/OloEngine/src/OloEngine/Task/Scheduler.cpp +++ b/OloEngine/src/OloEngine/Task/Scheduler.cpp @@ -20,8 +20,10 @@ #include "OloEngine/Debug/Instrumentor.h" #include +#include #include #include +#include // Platform cache line size (typically 64 bytes on x86/x64) #ifndef PLATFORM_CACHE_LINE_SIZE @@ -42,6 +44,41 @@ namespace OloEngine::LowLevelTasks static bool g_TaskGraphUseDynamicThreadCreation = false; static bool g_TaskGraphConfigInitialized = false; + std::optional ParseOversubscriptionRatio(const char* envValue) + { + if (envValue == nullptr) + { + return std::nullopt; + } + + // Parse with std::strtof rather than std::atof: atof has no error reporting and + // undefined behaviour on overflow, whereas strtof reports "not a number at all" + // via endPtr and overflow via ±HUGE_VALF (== ±inf). "inf"/"nan" likewise parse to + // non-finite values. All of those must be rejected before they reach the + // worker-budget math (inf >= 1.0f is true; the product ceil(workers * inf) then + // casts to i32, which is undefined behaviour) — the endPtr and isfinite guards + // below do exactly that. + char* endPtr = nullptr; + const f32 value = std::strtof(envValue, &endPtr); + if (endPtr == envValue) + { + // No characters consumed → the value is not a number. + return std::nullopt; + } + if (*endPtr != '\0') + { + // Trailing, unconsumed characters → a partially-numeric string like "2.0abc". + // strtof would silently parse the "2.0" prefix; require the whole string instead. + return std::nullopt; + } + if (!std::isfinite(value) || value < 1.0f || value > kMaxOversubscriptionRatio) + { + return std::nullopt; + } + + return value; + } + // @brief Parse command-line or environment configuration for task graph settings // // This mirrors UE5.7's approach of parsing command-line arguments like: @@ -96,10 +133,15 @@ namespace OloEngine::LowLevelTasks if (const char* EnvOversubscriptionRatio = std::getenv("OLO_TASK_GRAPH_OVERSUBSCRIPTION_RATIO")) { - float Value = static_cast(std::atof(EnvOversubscriptionRatio)); - if (Value >= 1.0f) + if (const std::optional Ratio = ParseOversubscriptionRatio(EnvOversubscriptionRatio)) + { + g_TaskGraphOversubscriptionRatio = *Ratio; + } + else { - g_TaskGraphOversubscriptionRatio = Value; + OLO_CORE_WARN("[TaskGraph] Ignoring invalid OLO_TASK_GRAPH_OVERSUBSCRIPTION_RATIO='{}' " + "(must be finite and within [1, {}]); keeping {}", + EnvOversubscriptionRatio, kMaxOversubscriptionRatio, g_TaskGraphOversubscriptionRatio); } } } diff --git a/OloEngine/src/OloEngine/Task/Scheduler.h b/OloEngine/src/OloEngine/Task/Scheduler.h index eb31890781..8012721790 100644 --- a/OloEngine/src/OloEngine/Task/Scheduler.h +++ b/OloEngine/src/OloEngine/Task/Scheduler.h @@ -18,9 +18,23 @@ #include #include +#include namespace OloEngine::LowLevelTasks { + // Upper bound accepted for the task-graph oversubscription ratio. The ratio + // multiplies the worker-thread count, so an unbounded (or non-finite) value + // read from config would blow up the thread budget — clamp it to something sane. + inline constexpr f32 kMaxOversubscriptionRatio = 64.0f; + + // Parse and validate the OLO_TASK_GRAPH_OVERSUBSCRIPTION_RATIO environment value. + // Environment config is untrusted input: a non-finite (NaN / ±inf) or out-of-range + // value must be rejected before it reaches the scheduler's worker-budget math + // (std::strtof yields inf for "inf", and `inf >= 1.0f` is true). Returns the + // validated ratio, or std::nullopt when the value is null/unparsable/out of the + // accepted [1, kMaxOversubscriptionRatio] range. Pure + side-effect free for testing. + [[nodiscard]] std::optional ParseOversubscriptionRatio(const char* envValue); + // @enum EQueuePreference // @brief Preference for which queue to use when launching tasks enum class EQueuePreference diff --git a/OloEngine/tests/CMakeLists.txt b/OloEngine/tests/CMakeLists.txt index 2833d91fd8..6989840821 100644 --- a/OloEngine/tests/CMakeLists.txt +++ b/OloEngine/tests/CMakeLists.txt @@ -203,6 +203,8 @@ add_executable(OloEngine-Tests Networking/ChatTest.cpp Networking/PersistenceTest.cpp Networking/MMOOptimizationTest.cpp + # Task / Scheduler config tests + Task/SchedulerConfigTest.cpp # Localization tests LocalizationTest.cpp # Dialogue system tests diff --git a/OloEngine/tests/Networking/ComponentReplicatorTest.cpp b/OloEngine/tests/Networking/ComponentReplicatorTest.cpp index 65e0e7dcb2..19cee42737 100644 --- a/OloEngine/tests/Networking/ComponentReplicatorTest.cpp +++ b/OloEngine/tests/Networking/ComponentReplicatorTest.cpp @@ -5,6 +5,10 @@ #include "OloEngine/Scene/Components.h" #include "OloEngine/Serialization/Archive.h" +#include +#include +#include + TEST(ComponentReplicatorTest, TransformRoundtrip) { using namespace OloEngine; @@ -161,3 +165,181 @@ TEST_F(ComponentRegistryTest, RegisteredTransformSerializerWorks) EXPECT_FLOAT_EQ(loaded.Scale.y, 3.0f); EXPECT_FLOAT_EQ(loaded.Scale.z, 4.0f); } + +// ── Untrusted wire-float hardening ─────────────────────────────────── +// Snapshot bytes come from the network and are untrusted. A NaN/±inf float must +// be replaced with a safe fallback on deserialize, never reach the scene. + +// Serialize a raw transform payload (9 floats, in wire order: translation, euler, +// scale) so we can inject arbitrary bit patterns the way a malicious peer would. +// (u8 / f32 are global typedefs from Core/Base.h, not members of namespace OloEngine.) +static std::vector MakeRawTransformBytes(std::array values) +{ + using namespace OloEngine; + + std::vector buffer; + FMemoryWriter writer(buffer); + writer.ArIsNetArchive = true; + for (f32& v : values) + { + writer << v; + } + return buffer; +} + +TEST(ComponentReplicatorTest, TransformRejectsNonFiniteWireFloats) +{ + using namespace OloEngine; + + const f32 nan = std::numeric_limits::quiet_NaN(); + const f32 inf = std::numeric_limits::infinity(); + + // Every field poisoned: translation, rotation euler, and scale. + auto buffer = MakeRawTransformBytes({ nan, inf, -inf, nan, inf, -inf, nan, inf, -inf }); + + TransformComponent loaded; + FMemoryReader reader(buffer); + reader.ArIsNetArchive = true; + ComponentReplicator::Serialize(reader, loaded); + + EXPECT_FALSE(reader.IsError()); + + // Nothing non-finite survives into the component. + EXPECT_TRUE(std::isfinite(loaded.Translation.x)); + EXPECT_TRUE(std::isfinite(loaded.Translation.y)); + EXPECT_TRUE(std::isfinite(loaded.Translation.z)); + EXPECT_TRUE(std::isfinite(loaded.Scale.x)); + EXPECT_TRUE(std::isfinite(loaded.Scale.y)); + EXPECT_TRUE(std::isfinite(loaded.Scale.z)); + EXPECT_TRUE(std::isfinite(loaded.GetRotationEuler().x)); + EXPECT_TRUE(std::isfinite(loaded.GetRotationEuler().y)); + EXPECT_TRUE(std::isfinite(loaded.GetRotationEuler().z)); + + // Fallbacks: translation/rotation → 0, scale → 1. + EXPECT_FLOAT_EQ(loaded.Translation.x, 0.0f); + EXPECT_FLOAT_EQ(loaded.Translation.y, 0.0f); + EXPECT_FLOAT_EQ(loaded.Translation.z, 0.0f); + EXPECT_FLOAT_EQ(loaded.Scale.x, 1.0f); + EXPECT_FLOAT_EQ(loaded.Scale.y, 1.0f); + EXPECT_FLOAT_EQ(loaded.Scale.z, 1.0f); +} + +TEST(ComponentReplicatorTest, TransformKeepsFiniteFieldsWhenSanitizing) +{ + using namespace OloEngine; + + const f32 nan = std::numeric_limits::quiet_NaN(); + + // Only Translation.y is poisoned; the rest must round-trip untouched. + auto buffer = MakeRawTransformBytes({ 1.0f, nan, 3.0f, 0.0f, 0.0f, 0.0f, 2.0f, 4.0f, 8.0f }); + + TransformComponent loaded; + FMemoryReader reader(buffer); + reader.ArIsNetArchive = true; + ComponentReplicator::Serialize(reader, loaded); + + EXPECT_FLOAT_EQ(loaded.Translation.x, 1.0f); + EXPECT_FLOAT_EQ(loaded.Translation.y, 0.0f); // sanitized + EXPECT_FLOAT_EQ(loaded.Translation.z, 3.0f); + EXPECT_FLOAT_EQ(loaded.Scale.x, 2.0f); + EXPECT_FLOAT_EQ(loaded.Scale.y, 4.0f); + EXPECT_FLOAT_EQ(loaded.Scale.z, 8.0f); +} + +TEST(ComponentReplicatorTest, Rigidbody3DRejectsNonFiniteWireFloats) +{ + using namespace OloEngine; + + const f32 nan = std::numeric_limits::quiet_NaN(); + const f32 inf = std::numeric_limits::infinity(); + + // Wire layout: i32 bodyType, f32 mass, vec3 linear velocity, vec3 angular velocity. + std::vector buffer; + { + FMemoryWriter writer(buffer); + writer.ArIsNetArchive = true; + i32 bodyType = 0; + f32 mass = nan; + std::array vel = { inf, -inf, nan, inf, -inf, nan }; + writer << bodyType; + writer << mass; + for (f32& v : vel) + { + writer << v; + } + } + + Rigidbody3DComponent loaded; + FMemoryReader reader(buffer); + reader.ArIsNetArchive = true; + ComponentReplicator::Serialize(reader, loaded); + + EXPECT_FALSE(reader.IsError()); + + EXPECT_TRUE(std::isfinite(loaded.m_Mass)); + EXPECT_GE(loaded.m_Mass, 0.0f); + EXPECT_TRUE(std::isfinite(loaded.m_InitialLinearVelocity.x)); + EXPECT_TRUE(std::isfinite(loaded.m_InitialLinearVelocity.y)); + EXPECT_TRUE(std::isfinite(loaded.m_InitialLinearVelocity.z)); + EXPECT_TRUE(std::isfinite(loaded.m_InitialAngularVelocity.x)); + EXPECT_TRUE(std::isfinite(loaded.m_InitialAngularVelocity.y)); + EXPECT_TRUE(std::isfinite(loaded.m_InitialAngularVelocity.z)); +} + +TEST(ComponentReplicatorTest, Rigidbody3DRejectsNegativeMass) +{ + using namespace OloEngine; + + std::vector buffer; + { + FMemoryWriter writer(buffer); + writer.ArIsNetArchive = true; + i32 bodyType = 0; + f32 mass = -5.0f; + std::array vel = { 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f }; + writer << bodyType; + writer << mass; + for (f32& v : vel) + { + writer << v; + } + } + + Rigidbody3DComponent loaded; + FMemoryReader reader(buffer); + reader.ArIsNetArchive = true; + ComponentReplicator::Serialize(reader, loaded); + + EXPECT_GE(loaded.m_Mass, 0.0f); +} + +TEST(ComponentReplicatorTest, Rigidbody3DRejectsZeroMass) +{ + using namespace OloEngine; + + // A finite, non-negative-but-zero mass passes an `< 0` guard yet is a degenerate + // dynamic-body mass (divide-by-zero in the Jolt mass-properties override). It must + // be defaulted to a safe, strictly-positive minimum on deserialize. + std::vector buffer; + { + FMemoryWriter writer(buffer); + writer.ArIsNetArchive = true; + i32 bodyType = 0; + f32 mass = 0.0f; + std::array vel = { 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f }; + writer << bodyType; + writer << mass; + for (f32& v : vel) + { + writer << v; + } + } + + Rigidbody3DComponent loaded; + FMemoryReader reader(buffer); + reader.ArIsNetArchive = true; + ComponentReplicator::Serialize(reader, loaded); + + EXPECT_TRUE(std::isfinite(loaded.m_Mass)); + EXPECT_GT(loaded.m_Mass, 0.0f); +} diff --git a/OloEngine/tests/Networking/SnapshotInterpolatorTest.cpp b/OloEngine/tests/Networking/SnapshotInterpolatorTest.cpp index c5a785e4a3..77517757db 100644 --- a/OloEngine/tests/Networking/SnapshotInterpolatorTest.cpp +++ b/OloEngine/tests/Networking/SnapshotInterpolatorTest.cpp @@ -6,6 +6,9 @@ #include "OloEngine/Scene/Components.h" #include "OloEngine/Serialization/Archive.h" +#include +#include + // Helper to create a serialized snapshot (UUID + transform) without a Scene. static std::vector MakeSnapshot(u64 uuid, const glm::vec3& translation) { @@ -97,3 +100,85 @@ TEST(SnapshotInterpolatorTest, InterpolateNeedsAtLeastTwoSnapshots) // No Scene to pass, but the method guards against < 2 entries early. EXPECT_EQ(interp.GetBuffer().Size(), 1u); } + +// ── Untrusted render-delay hardening ───────────────────────────────── +// SetRenderDelay feeds the tick math: a non-finite delay makes renderTick NaN, +// which slips past the `renderTick < 0.0f` guard (every NaN comparison is false) +// and then UB-casts to u32. The setter must reject such values and keep the +// previous valid delay; GetRenderTick must stay finite. + +TEST(SnapshotInterpolatorTest, SetRenderDelayRejectsNaN) +{ + using namespace OloEngine; + + SnapshotInterpolator interp; + interp.SetRenderDelay(0.2f); + + interp.SetRenderDelay(std::numeric_limits::quiet_NaN()); + + // Invalid value ignored — prior delay retained. + EXPECT_FLOAT_EQ(interp.GetRenderDelay(), 0.2f); + EXPECT_TRUE(std::isfinite(interp.GetRenderDelay())); +} + +TEST(SnapshotInterpolatorTest, SetRenderDelayRejectsPositiveInfinity) +{ + using namespace OloEngine; + + SnapshotInterpolator interp; + interp.SetRenderDelay(0.25f); + + interp.SetRenderDelay(std::numeric_limits::infinity()); + + EXPECT_FLOAT_EQ(interp.GetRenderDelay(), 0.25f); +} + +TEST(SnapshotInterpolatorTest, SetRenderDelayRejectsNegativeInfinity) +{ + using namespace OloEngine; + + SnapshotInterpolator interp; + interp.SetRenderDelay(0.15f); + + interp.SetRenderDelay(-std::numeric_limits::infinity()); + + EXPECT_FLOAT_EQ(interp.GetRenderDelay(), 0.15f); +} + +TEST(SnapshotInterpolatorTest, SetRenderDelayRejectsNegative) +{ + using namespace OloEngine; + + SnapshotInterpolator interp; + interp.SetRenderDelay(0.3f); + + interp.SetRenderDelay(-1.0f); + + EXPECT_FLOAT_EQ(interp.GetRenderDelay(), 0.3f); +} + +TEST(SnapshotInterpolatorTest, SetRenderDelayAcceptsZero) +{ + using namespace OloEngine; + + // Zero is a legitimate (no-delay) value and must still be accepted. + SnapshotInterpolator interp; + interp.SetRenderDelay(0.0f); + EXPECT_FLOAT_EQ(interp.GetRenderDelay(), 0.0f); +} + +TEST(SnapshotInterpolatorTest, RenderTickStaysFiniteAfterPoisonedDelay) +{ + using namespace OloEngine; + + SnapshotInterpolator interp; + interp.SetServerTickRate(20); + interp.SetRenderDelay(0.1f); + interp.PushSnapshot(10, MakeSnapshot(1, { 0.0f, 0.0f, 0.0f })); + + // A rejected NaN must not corrupt the cached delay → render tick stays finite. + interp.SetRenderDelay(std::numeric_limits::quiet_NaN()); + + EXPECT_TRUE(std::isfinite(interp.GetRenderTick())); + EXPECT_FLOAT_EQ(interp.GetRenderTick(), 8.0f); +} diff --git a/OloEngine/tests/Task/SchedulerConfigTest.cpp b/OloEngine/tests/Task/SchedulerConfigTest.cpp new file mode 100644 index 0000000000..796b29f687 --- /dev/null +++ b/OloEngine/tests/Task/SchedulerConfigTest.cpp @@ -0,0 +1,96 @@ +// OLO_TEST_LAYER: unit +#include "OloEnginePCH.h" +#include + +#include "OloEngine/Task/Scheduler.h" + +// Hardening for OLO_TASK_GRAPH_OVERSUBSCRIPTION_RATIO: the value is read from the +// environment (untrusted config) and feeds the worker-thread budget math, where +// `ceil(workers * ratio)` is cast to i32. std::strtof("inf") yields +inf, and the +// old `inf >= 1.0f` guard let it through — non-finite / out-of-range values must +// be rejected. ParseOversubscriptionRatio is the pure boundary that does that. + +namespace +{ + using OloEngine::LowLevelTasks::kMaxOversubscriptionRatio; + using OloEngine::LowLevelTasks::ParseOversubscriptionRatio; +} // namespace + +TEST(SchedulerConfigTest, NullEnvValueRejected) +{ + EXPECT_FALSE(ParseOversubscriptionRatio(nullptr).has_value()); +} + +TEST(SchedulerConfigTest, EmptyStringRejected) +{ + // std::strtof("") consumes no characters → rejected as not-a-number. + EXPECT_FALSE(ParseOversubscriptionRatio("").has_value()); +} + +TEST(SchedulerConfigTest, GarbageStringRejected) +{ + // Unparsable → strtof consumes no characters → rejected as not-a-number. + EXPECT_FALSE(ParseOversubscriptionRatio("not-a-number").has_value()); +} + +TEST(SchedulerConfigTest, PartiallyNumericStringRejected) +{ + // strtof parses the "2.0" prefix and stops at "abc"; requiring the whole string be + // consumed rejects it rather than silently accepting the leading number. + EXPECT_FALSE(ParseOversubscriptionRatio("2.0abc").has_value()); + EXPECT_FALSE(ParseOversubscriptionRatio("2.0 ").has_value()); + EXPECT_FALSE(ParseOversubscriptionRatio("2x").has_value()); +} + +TEST(SchedulerConfigTest, PositiveInfinityRejected) +{ + // The crux: "inf" parses to +inf and must NOT pass the >= 1.0 guard. + EXPECT_FALSE(ParseOversubscriptionRatio("inf").has_value()); + EXPECT_FALSE(ParseOversubscriptionRatio("INF").has_value()); + EXPECT_FALSE(ParseOversubscriptionRatio("infinity").has_value()); +} + +TEST(SchedulerConfigTest, NegativeInfinityRejected) +{ + EXPECT_FALSE(ParseOversubscriptionRatio("-inf").has_value()); +} + +TEST(SchedulerConfigTest, NaNRejected) +{ + EXPECT_FALSE(ParseOversubscriptionRatio("nan").has_value()); + EXPECT_FALSE(ParseOversubscriptionRatio("NaN").has_value()); +} + +TEST(SchedulerConfigTest, BelowMinimumRejected) +{ + EXPECT_FALSE(ParseOversubscriptionRatio("0.5").has_value()); + EXPECT_FALSE(ParseOversubscriptionRatio("0").has_value()); + EXPECT_FALSE(ParseOversubscriptionRatio("-2.0").has_value()); +} + +TEST(SchedulerConfigTest, AboveMaximumRejected) +{ + EXPECT_FALSE(ParseOversubscriptionRatio("1000").has_value()); + EXPECT_FALSE(ParseOversubscriptionRatio("1e9").has_value()); +} + +TEST(SchedulerConfigTest, ValidRatioAccepted) +{ + auto result = ParseOversubscriptionRatio("2.0"); + ASSERT_TRUE(result.has_value()); + EXPECT_FLOAT_EQ(*result, 2.0f); +} + +TEST(SchedulerConfigTest, LowerBoundInclusive) +{ + auto result = ParseOversubscriptionRatio("1.0"); + ASSERT_TRUE(result.has_value()); + EXPECT_FLOAT_EQ(*result, 1.0f); +} + +TEST(SchedulerConfigTest, UpperBoundInclusive) +{ + auto result = ParseOversubscriptionRatio("64"); + ASSERT_TRUE(result.has_value()); + EXPECT_FLOAT_EQ(*result, kMaxOversubscriptionRatio); +} diff --git a/docs/agent-rules/cpp-coding-quality.md b/docs/agent-rules/cpp-coding-quality.md index 9dc807fc30..c04c0549a1 100644 --- a/docs/agent-rules/cpp-coding-quality.md +++ b/docs/agent-rules/cpp-coding-quality.md @@ -81,6 +81,14 @@ if (!std::isfinite(value) || value <= 0.0f) value = defaultValue; ``` +This applies at **every** untrusted-float boundary, not just YAML — network deserialization +(`Networking/Replication/ComponentReplicator.cpp`) and environment-variable config +(`Task/Scheduler.cpp`) included. Guard **before** the value reaches arithmetic that casts to +an integer: a non-finite float must be rejected before `static_cast` / `static_cast`, +because the cast is undefined behaviour for `NaN`/`±inf`, and a `NaN` silently slips past sign +checks (`x < 0.0f` is `false` for `NaN`). When validating a setter/parser, prefer a pure helper +that returns the sanitized value (or `std::optional` to signal rejection) so it stays unit-testable. + --- ## 3. Use `auto` to avoid redundant type names