From 0853aa88d34326bd1aef4a7dbdd1e8ce7e6a4b81 Mon Sep 17 00:00:00 2001 From: Grigorii Sokolov Date: Fri, 11 Apr 2025 20:57:30 +0300 Subject: [PATCH] fix impossibility of a compound choice inside a set/sequence - The bug was inspired by N1 codec migration to the new version of MED. As it was discovered, a med::choice with a compound header could not be placed into a med::set or med::sequence as a mandatory member. - It happened because of a process of postponed compound header tag filling. A tag was filled out during a med::choice encode process, just before encoding. That is, by the moment an outer container encoding, the choice header tag remained untouched. Hereby, pre-encode check of all mandatory fields filled caused an error reporting the emptyness of the mandatory field. - The fix moves the compound header tag filling from the encode stage to the earlier stage, namely while choice::ref() API is called. - this change a protocol of working with a choice. Now a user can call med::choice::ref() only once. To reuse a choice / refill some choice member, med::choice::clear() must be called prior to it.. - The change also requires to distinguish a context within the ref() call happens. The decode stage, when the choice::ref() API is called by the engine, is the case. During the choice::ref call the header tag was already filled by the recent binary data reading and must not be touched. - To achieve it the new member method bool is_tag_set() was introduced. The new method must be presented in every class/struct where the get_tag() method is defined. --- med/choice.hpp | 23 +++++++++----- ut/choice.cpp | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ ut/copy.cpp | 1 + ut/diameter.cpp | 1 + ut/length.cpp | 1 + 5 files changed, 99 insertions(+), 8 deletions(-) diff --git a/med/choice.hpp b/med/choice.hpp index 89f38da..9e5aad2 100644 --- a/med/choice.hpp +++ b/med/choice.hpp @@ -174,14 +174,7 @@ struct choice_enc : choice_if } else { - //tag of this IE isn't in header - if constexpr (!explicit_meta_in()) - { - using tag_t = get_info_t>; - tag_t const tag{}; - //TODO: how to not modify? - const_cast(to).header().set_tag(tag.get()); - } + if(!to.header().is_tag_set()) { MED_THROW_EXCEPTION(unknown_tag, name(), to.index()); } med::encode(encoder, to.header()); //skip 1st TAG meta-info as it's encoded in header sl::ie_encode>>(encoder, to.template as()); @@ -256,6 +249,9 @@ struct choice_header template struct choice_header
{ + // each compound header for a choice must have bool is_tag_set() method together with get_tag() + static_assert(std::is_same().is_tag_set())>()); + static constexpr bool plain_header = false; //compound header e.g. a sequence using header_type = HEADER; @@ -319,6 +315,17 @@ class choice : public IE m_index = idx; new (&m_storage) type{}; } + if constexpr (!choice::plain_header) + { + // we must not do it on a decode stage when the header tag has been already set + if(!this->header().is_tag_set()) + { + using TAG_TYPE = get_meta_tag_t>; + static_assert(!std::is_void()); + TAG_TYPE tag; + this->header().set_tag(tag.get()); + } + } return *ie; } diff --git a/ut/choice.cpp b/ut/choice.cpp index 9784b49..3178a60 100644 --- a/ut/choice.cpp +++ b/ut/choice.cpp @@ -112,6 +112,7 @@ struct UNKNOWN : med::sequence< struct U8 : med::value{}; struct U16 : med::value{}; struct U32 : med::value{}; +struct string : med::ascii_string<> {}; //choice based on plain value selector struct plain : med::choice< @@ -124,6 +125,31 @@ struct plain : med::choice< using PLAIN = M; +struct hdr : med::sequence< + M, + O +> +{ + bool is_tag_set() const { return get().is_set(); } + auto get_tag() const { return get().get(); } + void set_tag(uint8_t v) { return ref().set(v); } +}; + +//choice based on a compound header selector +struct compound : med::choice< hdr, + M< C<0x00>, U8 >, + M< C<0x02>, U16 >, + M< C<0x04>, string > +> +{}; + +struct SEQ : med::sequence< + O< U32 > + , M< U16 > + , M< compound > +> +{}; + } //end: namespace cho using namespace std::string_view_literals; @@ -214,4 +240,59 @@ TEST(choice, nibble_tag) EXPECT_FALSE(pf->is_set()); } #endif +TEST(choice, tag_in_compound) +{ + uint8_t buffer[128]; + med::encoder_context ctx{ buffer }; + + cho::compound msg; + msg.ref().set("12345678"sv); + ASSERT_TRUE(msg.header().is_tag_set()); + + msg.header().clear(); + ASSERT_FALSE(msg.header().is_tag_set()); + + msg.ref(); + ASSERT_TRUE(msg.header().is_tag_set()); + + msg.header().clear(); + msg.ref().set("12345678"sv); + ASSERT_TRUE(msg.header().is_tag_set()); + + msg.header().ref().set(0xff); + + encode(med::octet_encoder{ctx}, msg); + EXPECT_STREQ( + "04 00 FF 31 32 33 34 35 36 37 38 ", + as_string(ctx.buffer()) + ); + + msg.clear(); + ASSERT_FALSE(msg.header().is_tag_set()); + + med::decoder_context dctx; + dctx.reset(ctx.buffer().get_start(), ctx.buffer().get_offset()); + decode(med::octet_decoder{dctx}, msg); + + auto* pf = msg.get(); + ASSERT_NE(nullptr, pf); + ASSERT_TRUE(msg.header().is_tag_set()); +} + +TEST(choice, m_choice_in_seq) +{ + uint8_t buffer[128]; + med::encoder_context ctx{ buffer }; + + SEQ msg; + + msg.ref().set(1); + msg.ref().ref().set("12345678"sv); + + encode(med::octet_encoder{ctx}, msg); + EXPECT_STREQ( + "00 01 04 31 32 33 34 35 36 37 38 ", + as_string(ctx.buffer()) + ); +} //NOTE: choice compound is tested in length.cpp ppp::proto diff --git a/ut/copy.cpp b/ut/copy.cpp index bcfb537..ddddeb3 100644 --- a/ut/copy.cpp +++ b/ut/copy.cpp @@ -23,6 +23,7 @@ struct hdr : med::sequence< M > { + bool is_tag_set() const { return get().is_set(); } auto get_tag() const { return get().get(); } void set_tag(uint8_t v) { return ref().set(v); } }; diff --git a/ut/diameter.cpp b/ut/diameter.cpp index 6d218a0..66d532b 100644 --- a/ut/diameter.cpp +++ b/ut/diameter.cpp @@ -75,6 +75,7 @@ struct header : med::sequence< M > { + bool is_tag_set() const { return get().is_set(); } std::size_t get_tag() const { return get().get() + (flags().request() ? REQUEST : 0); } void set_tag(std::size_t tag) { ref().set(tag & 0xFFFFFF); flags().request(tag & REQUEST); } diff --git a/ut/length.cpp b/ut/length.cpp index 76186d5..7c3dfc9 100644 --- a/ut/length.cpp +++ b/ut/length.cpp @@ -203,6 +203,7 @@ struct hdr : med::sequence< M > { + bool is_tag_set() const { return get().is_set(); } auto get_tag() const { return get().get(); } void set_tag(uint8_t v) { return ref().set(v); }