From 882dd307a2452ca3c0d62f9d3779e8588a5dc7db Mon Sep 17 00:00:00 2001 From: Ari Sulistiono Date: Fri, 16 Oct 2026 07:36:48 +0700 Subject: [PATCH] fix(mms): fail closed on orphan confirmed results in serial association --- .../ariec61850/mms/association_runtime.hpp | 4 + src/mms/association_runtime.cpp | 20 +++-- tests/test_mms_runtime.cpp | 78 +++++++++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/include/ariec61850/mms/association_runtime.hpp b/include/ariec61850/mms/association_runtime.hpp index e00fd8d4e..424a0ac5a 100644 --- a/include/ariec61850/mms/association_runtime.hpp +++ b/include/ariec61850/mms/association_runtime.hpp @@ -169,6 +169,10 @@ class MmsAssociationRuntime final { std::stop_token stop_token = {}); void disconnect(std::stop_token stop_token = {}) noexcept; + // Single-flight, association-owner-only API. Concurrent callers or a + // parallel poll/receive thread are unsupported until a dedicated + // association-owned dispatcher correlates outstanding invoke IDs. + // Unexpected confirmed results fail closed instead of being replayed. [[nodiscard]] MmsConfirmedExchangeResult exchange_confirmed( std::span presentation_payload, std::uint32_t expected_invoke_id, diff --git a/src/mms/association_runtime.cpp b/src/mms/association_runtime.cpp index ce69823fd..f5ff43aa3 100644 --- a/src/mms/association_runtime.cpp +++ b/src/mms/association_runtime.cpp @@ -683,13 +683,15 @@ MmsConfirmedExchangeResult MmsAssociationRuntime::exchange_confirmed( "Confirmed exchange payload does not contain the expected invoke ID."); } - MmsPduEnvelope queued; - if (invoke_router_.try_dequeue(expected_invoke_id, queued)) { - return {std::move(queued), {}}; - } - const auto deadline = deadline_after(options_.request_timeout); try { + // This runtime owns one synchronous confirmed exchange at a time. + // A result routed by idle polling or a previous operation cannot + // acknowledge a request that has not been transmitted yet. + if (invoke_router_.queued_confirmed_count() != 0U) { + throw MmsAssociationRuntimeError( + "Unsolicited queued MMS confirmed result before a new request."); + } send_application_payload(presentation_payload, deadline, stop_token); if (request_sent != nullptr) *request_sent = true; add_event(MmsAssociationEventKind::confirmed_request_sent, @@ -700,6 +702,14 @@ MmsConfirmedExchangeResult MmsAssociationRuntime::exchange_confirmed( ++index) { auto received = receive_application_payload(deadline, stop_token); auto envelope = route_received_payload(received); + if (envelope.confirmed_result() && + !envelope.matches_invoke(expected_invoke_id)) { + // A second invoke ID has no legitimate in-flight owner in + // this serial runtime. Fail this association rather than + // caching a response to satisfy a future, unsent request. + throw MmsAssociationRuntimeError( + "Unexpected MMS confirmed result invoke ID in serial exchange."); + } if (envelope.confirmed_result() && envelope.matches_invoke(expected_invoke_id)) { MmsPduEnvelope matched; diff --git a/tests/test_mms_runtime.cpp b/tests/test_mms_runtime.cpp index 995b148a9..ba54b480b 100644 --- a/tests/test_mms_runtime.cpp +++ b/tests/test_mms_runtime.cpp @@ -374,6 +374,82 @@ void confirmed_exchange_timeout_closes_transport_and_can_reconnect() { CHECK(transport.connect_count() == 2U); } + +void confirmed_exchange_rejects_unexpected_invoke_instead_of_caching_it() { + ScriptedTransport transport; + queue_handshake(transport); + mms::MmsAssociationRuntime runtime{transport}; + runtime.connect({"127.0.0.1", 102U}); + + const auto expected_id = runtime.next_invoke_id(); + const auto unexpected_id = runtime.next_invoke_id(); + mms::MmsReadRequest request; + request.invoke_id = expected_id; + request.variables.push_back( + mms::MmsObjectName::domain_specific("LD0", "LLN0$ST$Mod$stVal")); + const auto request_bytes = mms::MmsServiceCodec::encode_read_request_p_data(request); + + mms::MmsReadResponse unexpected; + unexpected.invoke_id = unexpected_id; + unexpected.results.push_back({mms::MmsDataValue::boolean(false), std::nullopt}); + transport.push_receive(wrap_application( + mms::MmsServiceCodec::encode_read_response_p_data(unexpected))); + + // This matching response would previously let the first exchange succeed + // and leave the unrelated response cached for a later unsent request. + mms::MmsReadResponse matching; + matching.invoke_id = expected_id; + matching.results.push_back({mms::MmsDataValue::boolean(true), std::nullopt}); + transport.push_receive(wrap_application( + mms::MmsServiceCodec::encode_read_response_p_data(matching))); + + bool request_sent = false; + check_throws([&] { + static_cast(runtime.exchange_confirmed( + request_bytes, expected_id, {}, &request_sent)); + }); + CHECK(request_sent); + CHECK(transport.sent().size() == 3U); // COTP, AARQ, exactly one MMS request + CHECK(runtime.state() == mms::MmsAssociationRuntimeState::faulted); + CHECK(!transport.connected()); + CHECK(!runtime.last_fault().empty()); +} + +void confirmed_exchange_rejects_idle_orphan_without_sending() { + ScriptedTransport transport; + queue_handshake(transport); + mms::MmsAssociationRuntime runtime{transport}; + runtime.connect({"127.0.0.1", 102U}); + + const auto invoke_id = runtime.next_invoke_id(); + mms::MmsReadResponse orphan; + orphan.invoke_id = invoke_id; + orphan.results.push_back({mms::MmsDataValue::boolean(false), std::nullopt}); + transport.push_receive(wrap_application( + mms::MmsServiceCodec::encode_read_response_p_data(orphan))); + + mms::MmsPduEnvelope idle_received; + CHECK(runtime.try_poll_once_for(std::chrono::milliseconds{1}, idle_received)); + CHECK(idle_received.confirmed_result()); + + mms::MmsReadRequest request; + request.invoke_id = invoke_id; + request.variables.push_back( + mms::MmsObjectName::domain_specific("LD0", "LLN0$ST$Mod$stVal")); + const auto request_bytes = mms::MmsServiceCodec::encode_read_request_p_data(request); + const auto sent_before = transport.sent().size(); + + bool request_sent = true; + check_throws([&] { + static_cast(runtime.exchange_confirmed( + request_bytes, invoke_id, {}, &request_sent)); + }); + CHECK(!request_sent); + CHECK(transport.sent().size() == sent_before); + CHECK(runtime.state() == mms::MmsAssociationRuntimeState::faulted); + CHECK(!transport.connected()); +} + [[nodiscard]] mms::MmsReportControlCandidate make_urcb_candidate() { mms::MmsReportControlCandidate candidate; candidate.domain = "LD0"; @@ -688,6 +764,8 @@ int main() { {"association profile fallback", association_retries_legacy_profile_after_balanced_rejection}, {"association cancellation and reconnect", association_rejects_cancelled_connect_and_can_reconnect}, {"confirmed exchange timeout closes transport and reconnects", confirmed_exchange_timeout_closes_transport_and_can_reconnect}, + {"unexpected confirmed invoke fails closed", confirmed_exchange_rejects_unexpected_invoke_instead_of_caching_it}, + {"orphan confirmed result cannot acknowledge unsent request", confirmed_exchange_rejects_idle_orphan_without_sending}, {"report subscription lifecycle", subscription_runtime_reserves_enables_receives_and_cleans_up}, {"report cleanup after association loss", subscription_marks_cleanup_required_when_association_is_lost}, {"report takeover protection", subscription_does_not_take_over_an_enabled_rcb},