More tests for PB, some bug fixes

src/tl/tl/tlProtocolBuffer.cc

@@ -63,8 +63,7 @@ ProtocolBufferReader::skip ()
 
   } else if (m_type == PB_LEN) {
 
-    size_t value = 0;
-    read (value);
+    size_t value = read_varint ();
     skip_bytes (value);
 
   }
@@ -93,8 +92,7 @@ ProtocolBufferReader::read (std::string &s)
     error (tl::to_string (tr ("Expected a LEN wire type for a string")));
   }
 
-  size_t value = 0;
-  read (value);
+  size_t value = read_varint ();
 
   s = std::string ();
   s.reserve (value);
@@ -110,7 +108,7 @@ ProtocolBufferReader::read (std::string &s)
 void
 ProtocolBufferReader::read (uint32_t &ui32)
 {
-  if (m_type == PB_VARINT || m_type == PB_LEN) {
+  if (m_type == PB_VARINT) {
 
     pb_varint ui64 = read_varint ();
     if (ui64 > std::numeric_limits<uint32_t>::max ()) {
@@ -153,7 +151,7 @@ ProtocolBufferReader::read (int32_t &i32)
 void
 ProtocolBufferReader::read (uint64_t &ui64)
 {
-  if (m_type == PB_VARINT || m_type == PB_LEN) {
+  if (m_type == PB_VARINT) {
 
     ui64 = read_varint ();
 
@@ -203,8 +201,7 @@ ProtocolBufferReader::open ()
     error (tl::to_string (tr ("Expected a LEN wire type for a submessage")));
   }
 
-  size_t value = 0;
-  read (value);
+  size_t value = read_varint ();
   if (! m_seq_counts.empty ()) {
     //  take out the following bytes from the current sequence
     m_seq_counts.back () -= value;
@@ -284,6 +281,15 @@ ProtocolBufferReader::read_varint ()
 void
 ProtocolBufferReader::skip_bytes (size_t n)
 {
+  m_pos_before = m_pos;
+  m_pos += n;
+  if (! m_seq_counts.empty ()) {
+    if (m_seq_counts.back () < n) {
+      error (tl::to_string (tr ("sequence underflow")));
+    }
+    m_seq_counts.back () -= n;
+  }
+
   const size_t chunk_size = 1024;
   while (n > 0) {
     size_t l = std::min (chunk_size, n);

src/tl/unit_tests/tlProtocolBufferTests.cc

@@ -205,28 +205,26 @@ struct Root {
   const Child &get_child () const { return m_child; }
 };
 
-TEST (100_BasicStruct)
+static tl::PBElementList child_struct =
+    tl::pb_make_member (&Child::txt, 1) +
+    tl::pb_make_member (&Child::d, 2) +
+    tl::pb_make_element (&Child::begin_children, &Child::end_children, &Child::add_child, 3, &child_struct);
+
+static tl::PBStruct<Root> structure ("pbtest-struct", 88888888,
+  tl::pb_make_member (&Root::begin_subs, &Root::end_subs, &Root::add_sub, 1) +
+  tl::pb_make_member (&Root::begin_isubs, &Root::end_isubs, &Root::add_isub, 2) +
+  tl::pb_make_element (&Root::begin_children, &Root::end_children, &Root::add_child, 3, &child_struct) +
+  tl::pb_make_element (&Root::get_child, &Root::set_child, 4,
+    tl::pb_make_member (&Child::txt, 1) +
+    tl::pb_make_member (&Child::d, 2)
+  ) +
+  tl::pb_make_member (&Root::m, 5) +
+  tl::pb_make_member (&Root::get_mi, &Root::set_mi, 6) +
+  tl::pb_make_member (&Root::b, 7)
+);
+
+static void build_struct (Root &root)
 {
-  Root root;
-
-  tl::PBElementList child_struct =
-      tl::pb_make_member (&Child::txt, 1) +
-      tl::pb_make_member (&Child::d, 2) +
-      tl::pb_make_element (&Child::begin_children, &Child::end_children, &Child::add_child, 3, &child_struct);
-
-  tl::PBStruct<Root> structure ("pbtest-struct", 88888888,
-    tl::pb_make_member (&Root::begin_subs, &Root::end_subs, &Root::add_sub, 1) +
-    tl::pb_make_member (&Root::begin_isubs, &Root::end_isubs, &Root::add_isub, 2) +
-    tl::pb_make_element (&Root::begin_children, &Root::end_children, &Root::add_child, 3, &child_struct) +
-    tl::pb_make_element (&Root::get_child, &Root::set_child, 4,
-      tl::pb_make_member (&Child::txt, 1) +
-      tl::pb_make_member (&Child::d, 2)
-    ) +
-    tl::pb_make_member (&Root::m, 5) +
-    tl::pb_make_member (&Root::get_mi, &Root::set_mi, 6) +
-    tl::pb_make_member (&Root::b, 7)
-  );
-
   root.add_sub (0.5);
   root.add_sub (7.5);
   root.add_isub (420000000);
@@ -264,6 +262,12 @@ TEST (100_BasicStruct)
   sc.txt = "single";
   sc.d = 4.2e6;
   root.set_child (sc);
+}
+
+TEST (100_BasicStruct)
+{
+  Root root;
+  build_struct (root);
 
   std::string fn = tl::combine_path (tl::testtmp (), "pb_test.pb");
 
@@ -405,21 +409,21 @@ struct TestClassEnumConverter
   }
 };
 
+tl::PBStruct<TestClass> tc_structure ("pbtest-tc", 1,
+  tl::pb_make_member (&TestClass::e, 2, TestClassEnumConverter ())
+);
+
 TEST (101_Converter)
 {
   TestClass tc;
 
-  tl::PBStruct<TestClass> structure ("pbtest-tc", 1,
-    tl::pb_make_member (&TestClass::e, 2, TestClassEnumConverter ())
-  );
-
   tc.e = TestClass::A;
   std::string fn = tl::combine_path (tl::testtmp (), "pb_101a.pb");
 
   {
     tl::OutputStream os (fn);
     tl::ProtocolBufferWriter writer (os);
-    structure.write (writer, tc);
+    tc_structure.write (writer, tc);
   }
 
   tc = TestClass ();
@@ -428,7 +432,7 @@ TEST (101_Converter)
   try {
     tl::InputStream is (fn);
     tl::ProtocolBufferReader reader (is);
-    structure.parse (reader, tc);
+    tc_structure.parse (reader, tc);
   } catch (tl::Exception &ex) {
     error = ex.msg ();
   }
@@ -442,7 +446,7 @@ TEST (101_Converter)
   {
     tl::OutputStream os (fn);
     tl::ProtocolBufferWriter writer (os);
-    structure.write (writer, tc);
+    tc_structure.write (writer, tc);
   }
 
   tc = TestClass ();
@@ -451,7 +455,7 @@ TEST (101_Converter)
   try {
     tl::InputStream is (fn);
     tl::ProtocolBufferReader reader (is);
-    structure.parse (reader, tc);
+    tc_structure.parse (reader, tc);
   } catch (tl::Exception &ex) {
     error = ex.msg ();
   }
@@ -459,3 +463,151 @@ TEST (101_Converter)
   EXPECT_EQ (tc.e, TestClass::B);
 }
 
+TEST (101_ExternalFiles)
+{
+  Root root_au;
+  build_struct (root_au);
+
+  std::string fn = tl::combine_path (tl::testsrc (), "testdata/pb/struct1.pb");
+
+  Root root;
+  std::string error;
+  try {
+    tl::InputStream is (fn);
+    tl::ProtocolBufferReader reader (is);
+    structure.parse (reader, root);
+  } catch (tl::Exception &ex) {
+    error = ex.msg ();
+  }
+
+  EXPECT_EQ (error, "");
+  EXPECT_EQ (root == root_au, true);
+}
+
+TEST (102_InvalidHeader)
+{
+  std::string fn = tl::combine_path (tl::testsrc (), "testdata/pb/struct2_invalid_header.pb");
+
+  Root root;
+  std::string error;
+  try {
+    tl::InputStream is (fn);
+    tl::ProtocolBufferReader reader (is);
+    structure.parse (reader, root);
+  } catch (tl::Exception &ex) {
+    error = ex.msg ();
+  }
+
+  EXPECT_EQ (std::string (error, 0, 49), "Expected header field with ID 88888888 (got 4444)");
+}
+
+TEST (103_InvalidHeader)
+{
+  std::string fn = tl::combine_path (tl::testsrc (), "testdata/pb/struct3_invalid_header.pb");
+
+  Root root;
+  std::string error;
+  try {
+    tl::InputStream is (fn);
+    tl::ProtocolBufferReader reader (is);
+    structure.parse (reader, root);
+  } catch (tl::Exception &ex) {
+    error = ex.msg ();
+  }
+
+  EXPECT_EQ (std::string (error, 0, 71), "Expected header field with string 'pbtest-struct' (got 'xxxxxxxxxxxxx')");
+}
+
+//  "last one wins", unknown IDs are skipped
+TEST (104_Deviations)
+{
+  Root root_au;
+  build_struct (root_au);
+
+  std::string fn = tl::combine_path (tl::testsrc (), "testdata/pb/struct4_deviations.pb");
+
+  Root root;
+  std::string error;
+  try {
+    tl::InputStream is (fn);
+    tl::ProtocolBufferReader reader (is);
+    structure.parse (reader, root);
+  } catch (tl::Exception &ex) {
+    error = ex.msg ();
+  }
+
+  EXPECT_EQ (error, "");
+  EXPECT_EQ (root == root_au, true);
+}
+
+TEST (105_Int32Overflow)
+{
+  std::string fn = tl::combine_path (tl::testsrc (), "testdata/pb/struct5_overflow.pb");
+
+  TestClass tc;
+  std::string error;
+  try {
+    tl::InputStream is (fn);
+    tl::ProtocolBufferReader reader (is);
+    tc_structure.parse (reader, tc);
+  } catch (tl::Exception &ex) {
+    error = ex.msg ();
+  }
+
+  EXPECT_EQ (std::string (error, 0, 21), "32 bit value overflow");
+}
+
+TEST (106_InvalidType)
+{
+  std::string fn = tl::combine_path (tl::testsrc (), "testdata/pb/struct6_invalid_type.pb");
+
+  TestClass tc;
+  std::string error;
+  try {
+    tl::InputStream is (fn);
+    tl::ProtocolBufferReader reader (is);
+    tc_structure.parse (reader, tc);
+  } catch (tl::Exception &ex) {
+    error = ex.msg ();
+  }
+
+  EXPECT_EQ (std::string (error, 0, 34), "Expected a VARINT or I32 wire type");
+}
+
+TEST (107_InvalidType)
+{
+  //  uses I32 to represent enum - works too, not just VARINT
+  std::string fn = tl::combine_path (tl::testsrc (), "testdata/pb/struct7_invalid_type.pb");
+
+  TestClass tc;
+  tc.e = TestClass::B;
+  std::string error;
+  try {
+    tl::InputStream is (fn);
+    tl::ProtocolBufferReader reader (is);
+    tc_structure.parse (reader, tc);
+  } catch (tl::Exception &ex) {
+    error = ex.msg ();
+  }
+
+  EXPECT_EQ (error, "");
+  EXPECT_EQ (tc.e, TestClass::A);
+}
+
+TEST (108_InvalidType)
+{
+  std::string fn = tl::combine_path (tl::testsrc (), "testdata/pb/struct8_invalid_type.pb");
+
+  TestClass tc;
+  std::string error;
+  try {
+    tl::InputStream is (fn);
+    tl::ProtocolBufferReader reader (is);
+    tc_structure.parse (reader, tc);
+  } catch (tl::Exception &ex) {
+    error = ex.msg ();
+  }
+
+  EXPECT_EQ (std::string (error, 0, 34), "Expected a VARINT or I32 wire type");
+}
+

testdata/pb/struct5_overflow.pb

@@ -0,0 +1,2 @@
+
+	pbtest-tc€Ô<E282AC>ª?

testdata/pb/struct8_invalid_type.pb

@@ -0,0 +1,2 @@
+
+	pbtest-tcabc