Fix for issue 370 : C++ generator deserialization (#371)

See issue #370

src/nunavut/lang/cpp/support/serialization.j2

@@ -57,6 +57,7 @@ static_assert(__cplusplus >= 201402L,
 #include <cstdint> // for memset
 #include <array> // for std::array
 #include <algorithm> // for std::max, std::min
+#include <limits> // for std::numeric_limits
 #include <utility> // for std::move
 #include <type_traits> // std::underlying_type, std::aligned_storage
 
@@ -236,19 +237,30 @@ public:
         return derived_bitspan(self.data_.data(), self.data_.size(), self.offset_bits_ + bits);
     }
 
-    /// Create a copy of current bitspan, converting current offset into  pointer
+    /// Create a copy of current bitspan, converting current offset into pointer
     /// and shrinking size of underlying byte span.
     /// This operation is safe even on empty containers since we are only creating
-    /// an invalid pointer, but dont dereference it - it is guarded by asserts in this
+    /// an invalid pointer, but don't dereference it - it is guarded by asserts in this
     /// class and in byte span.
-    derived_bitspan subspan({{ typename_unsigned_bit_length }} bits=0) const noexcept{
-        auto& self  = *static_cast<const derived_bitspan*>(this);
-        const {{ typename_unsigned_bit_length }} offset_bits = self.offset_bits_ + bits;
-        const {{ typename_unsigned_length }} offset_bytes = (offset_bits) / 8U;
-        const {{ typename_unsigned_bit_length }} offset_bits_mod = (offset_bits) % 8U;
-        const {{ typename_unsigned_length }} newSize = {# -#}
-            (offset_bytes < self.data_.size())?(self.data_.size() - offset_bytes):(0U);
-        return derived_bitspan(self.data_.data() + offset_bytes, newSize, offset_bits_mod);
+    derived_bitspan subspan(
+        const {{ typename_unsigned_bit_length }} bits_at = 0,
+        const {{ typename_unsigned_bit_length }} size_bits = std::numeric_limits<{{ typename_unsigned_bit_length }}>::max()
+        ) const noexcept {
+        auto& self = *static_cast<const derived_bitspan*>(this);
+        const {{ typename_unsigned_bit_length }} offset_bits = self.offset_bits_ + bits_at;
+        const {{ typename_unsigned_length }} offset_bytes = offset_bits / 8U;
+        const {{ typename_unsigned_bit_length }} offset_bits_mod = offset_bits % 8U;
+        {{ typename_unsigned_length }} new_size_bytes = 0U;
+        if (offset_bytes < self.data_.size())
+        {
+            new_size_bytes = self.data_.size() - offset_bytes;
+            if (size_bits < (std::numeric_limits<std::size_t>::max() - 7U))
+            {
+                // Above `-7` is to prevent overflow in the below line.
+                new_size_bytes = std::min(new_size_bytes, (offset_bits_mod + size_bits) / 8U);
+            }
+        }
+        return derived_bitspan(self.data_.data() + offset_bytes, new_size_bytes, offset_bits_mod);
     }
 
     void add_offset({{ typename_unsigned_bit_length }} bits) noexcept{

src/nunavut/lang/cpp/templates/deserialization.j2

@@ -230,12 +230,16 @@
         {
             return -nunavut::support::Error::RepresentationBadDelimiterHeader;
         }
-        const {{ typename_unsigned_length }} {{ref_delimiter}} = {{ ref_size_bytes }};
+        const {{ typename_unsigned_length }} {{ ref_delimiter }} = {{ ref_size_bytes }};
 {% endif %}
 
         {{ assert('in_buffer.offset_alings_to_byte()') }}
         {
+{% if t is DelimitedType %}
+            const auto {{ ref_err }} = deserialize({{ reference }}, in_buffer.subspan(0U, {{ ref_delimiter }} * 8U));
+{% else %}
             const auto {{ ref_err }} = deserialize({{ reference }}, in_buffer.subspan());
+{% endif %}
             if({{ ref_err }}){
                 {{ ref_size_bytes }} = {{ ref_err }}.value();
             }else{

verification/cpp/suite/test_bitarray.cpp

@@ -92,6 +92,48 @@ TEST(BitSpan, Subspan)
     ASSERT_EQ(nunavut::support::Error::SerializationBufferTooSmall, res.error());
 }
 
+TEST(BitSpan, ConstSubspan)
+{
+    const std::array<const uint8_t, 2> srcArray{ 0xAA, 0xFF };
+    nunavut::support::const_bitspan sp(srcArray);
+    {
+        const auto res = sp.subspan(0U, 8U);
+        ASSERT_EQ(0U, res.offset());
+        ASSERT_EQ(8U, res.size());
+        ASSERT_EQ(0xAAU, res.aligned_ref());
+    }
+    {
+        const auto res = sp.subspan(8U, 8U);
+        ASSERT_EQ(0U, res.offset());
+        ASSERT_EQ(8U, res.size());
+        ASSERT_EQ(0xFFU, res.aligned_ref());
+    }
+    {
+        const auto res = sp.subspan(12U, 4U);
+        ASSERT_EQ(4U, res.offset());
+        ASSERT_EQ(4U, res.size());
+        ASSERT_EQ(0xFFU, res.aligned_ref());
+    }
+    {
+        auto res = sp.subspan(0U, 32U);
+        ASSERT_EQ(0U, res.offset());
+        ASSERT_EQ(16U, res.size());
+    }
+    {
+        auto res = sp.subspan(3U, 32U);
+        ASSERT_EQ(3U, res.offset());
+        ASSERT_EQ(13U, res.size());
+
+        res = res.subspan(3U, 32U);
+        ASSERT_EQ(6U, res.offset());
+        ASSERT_EQ(10U, res.size());
+
+        res = res.subspan(3U);
+        ASSERT_EQ(1U, res.offset());
+        ASSERT_EQ(7U, res.size());
+    }
+}
+
 TEST(BitSpan, AlignedPtr) {
     std::array<uint8_t,5> srcArray{ 1, 2, 3, 4, 5 };
     {

verification/cpp/suite/test_serialization.cpp

@@ -11,6 +11,8 @@
 #include "regulated/basics/Struct__0_1.hpp"
 #include "regulated/basics/Service_0_1.hpp"
 #include "regulated/basics/Primitive_0_1.hpp"
+#include "regulated/delimited/BDelimited_1_0.hpp"
+#include "regulated/delimited/BDelimited_1_1.hpp"
 
 
 static_assert(
@@ -400,6 +402,105 @@ TEST(Serialization, StructReference)
 }
 
 
+/// Test for issue #370:
+/// During de-serialization of a nested (not top level) delimited type (with @extent) its header size
+/// is not taken into account but instead whole remaining part of the buffer is in use - as a result
+/// "garbage" bytes could end up in the result deserialized instance when input buffer is longer than expected.
+///
+TEST(Serialization, Issue370)
+{
+    regulated::delimited::BDelimited_1_0 obj_0_1{};
+    obj_0_1.var.push_back({{0x85, 0x86}, -53});
+    obj_0_1.var.push_back({{0x87, 0x88}, -54});
+    obj_0_1.fix.push_back({0xF1, 0xF2});
+    obj_0_1.fix.push_back({0xF3, 0xF4});
+
+    const uint8_t reference[] = {
+        0x02,  // byte  0:
+        0x04,  // byte  1:
+        0x00,  // byte  2:
+        0x00,  // byte  3:
+        0x00,  // byte  4:
+        0x02,  // byte  5:
+        0x85,  // byte  6:
+        0x86,  // byte  7:
+        0xCB,  // byte  8: -53
+        0x04,  // byte  9:
+        0x00,  // byte 10:
+        0x00,  // byte 11:
+        0x00,  // byte 12:
+        0x02,  // byte 13:
+        0x87,  // byte 14:
+        0x88,  // byte 15:
+        0xCA,  // byte 16: -54
+        0x02,  // byte 17:
+        0x02,  // byte 18:
+        0x00,  // byte 19:
+        0x00,  // byte 20:
+        0x00,  // byte 21:
+        0xF1,  // byte 22:
+        0xF2,  // byte 23:
+        0x02,  // byte 24:
+        0x00,  // byte 25:
+        0x00,  // byte 26:
+        0x00,  // byte 37:
+        0xF3,  // byte 38:
+        0xF4,  // byte 39:
+        0x55,  // byte 40: canary  1
+        0x55,  // byte 41: canary  2
+        0x55,  // byte 42: canary  3
+        0x55,  // byte 43: canary  4
+        0x55,  // byte 44: canary  5
+        0x55,  // byte 45: canary  6
+        0x55,  // byte 46: canary  7
+        0x55,  // byte 47: canary  8
+        0x55,  // byte 48: canary  9
+        0x55,  // byte 49: canary 10
+        0x55,  // byte 50: canary 11
+        0x55,  // byte 51: canary 12
+        0x55,  // byte 52: canary 13
+        0x55,  // byte 53: canary 14
+        0x55,  // byte 54: canary 15
+        0x55,  // byte 55: canary 16
+    };
+
+    uint8_t buf[sizeof(reference)];
+    (void) memset(&buf[0], 0x55U, sizeof(buf));  // fill out canaries
+    auto result = serialize(obj_0_1, buf);
+    ASSERT_TRUE(result) << "Error is " << static_cast<int>(result.error());
+    EXPECT_EQ(sizeof(reference) - 16U, result.value());
+    for(size_t i=0; i< sizeof(reference); i++)
+    {
+        ASSERT_EQ(reference[i], buf[i]) << "Failed at " << i;
+    }
+
+    regulated::delimited::BDelimited_1_1 obj_1_1{};
+
+    result = deserialize(obj_1_1, reference);
+    ASSERT_TRUE(result) << "Error was " << result.error();
+    ASSERT_EQ(sizeof(reference) - 16U, result.value());   // 16 trailing bytes implicitly truncated away
+
+    EXPECT_EQ(obj_1_1.var.size(), 2);
+    EXPECT_EQ(obj_1_1.var[0].a.size(), 2);
+    EXPECT_EQ(obj_1_1.var[0].a[0], 0x85);
+    EXPECT_EQ(obj_1_1.var[0].a[1], 0x86);
+    EXPECT_EQ(obj_1_1.var[1].a.size(), 2);
+    EXPECT_EQ(obj_1_1.var[1].a[0], 0x87);
+    EXPECT_EQ(obj_1_1.var[1].a[1], 0x88);
+
+    EXPECT_EQ(obj_1_1.fix.size(), 2);
+    EXPECT_EQ(obj_1_1.fix[0].a.size(), 3);
+    EXPECT_EQ(obj_1_1.fix[0].a[0], 0xF1);
+    EXPECT_EQ(obj_1_1.fix[0].a[1], 0xF2);
+    EXPECT_EQ(obj_1_1.fix[0].a[2], 0x00);   // Without fix we get incorrect 0x02 here
+    EXPECT_EQ(obj_1_1.fix[0].b, 0x00);
+    EXPECT_EQ(obj_1_1.fix[1].a.size(), 3);
+    EXPECT_EQ(obj_1_1.fix[1].a[0], 0xF3);
+    EXPECT_EQ(obj_1_1.fix[1].a[1], 0xF4);
+    EXPECT_EQ(obj_1_1.fix[1].a[2], 0x00);   // Without fix we get incorrect 0x55 here
+    EXPECT_EQ(obj_1_1.fix[1].b, 0x00);      // Without fix we get incorrect 0x55 here
+}
+
 
 TEST(Serialization, Primitive)
 {