Repository: randombit/botan
Pull request #2948 (2022-04-04 13:09:51)

[TLS 1.3] Additional convenience methods for TLS_Data_Reader
Self-explanatory additions, I guess.


Patch:
diff --git a/src/lib/tls/tls_reader.h b/src/lib/tls/tls_reader.h
index d31bf7ef4..6bd5b7e6b 100644
--- a/src/lib/tls/tls_reader.h
+++ b/src/lib/tls/tls_reader.h
@@ -64,6 +64,15 @@ class TLS_Data_Reader final
          return result;
          }
 
+      uint32_t get_uint24_t()
+         {
+         assert_at_least(3);
+         uint32_t result = make_uint32(0,                 m_buf[m_offset],
+                                       m_buf[m_offset+1], m_buf[m_offset+2]);
+         m_offset += 3;
+         return result;
+         }
+
       uint16_t get_uint16_t()
          {
          assert_at_least(2);
@@ -72,6 +81,12 @@ class TLS_Data_Reader final
          return result;
          }
 
+      uint16_t peek_uint16_t() const
+         {
+         assert_at_least(2);
+         return make_uint16(m_buf[m_offset], m_buf[m_offset+1]);
+         }
+
       uint8_t get_byte()
          {
          assert_at_least(1);
@@ -95,6 +110,11 @@ class TLS_Data_Reader final
          return result;
          }
 
+      std::vector<uint8_t> get_tls_length_value(size_t len_bytes)
+         {
+         return get_fixed<uint8_t>(get_length_field(len_bytes));
+         }
+
       template<typename T>
       std::vector<T> get_range(size_t len_bytes,
                                size_t min_elems,
@@ -142,6 +162,8 @@ class TLS_Data_Reader final
             return get_byte();
          else if(len_bytes == 2)
             return get_uint16_t();
+         else if(len_bytes == 3)
+            return get_uint24_t();
 
          throw decode_error("Bad length size");
          }
@@ -195,11 +217,12 @@ void append_tls_length_value(std::vector<uint8_t, Alloc>& buf,
    const size_t T_size = sizeof(T);
    const size_t val_bytes = T_size * vals_size;
 
-   if(tag_size != 1 && tag_size != 2)
+   if(tag_size != 1 && tag_size != 2 && tag_size != 3)
       throw Invalid_Argument("append_tls_length_value: invalid tag size");
 
    if((tag_size == 1 && val_bytes > 255) ||
-      (tag_size == 2 && val_bytes > 65535))
+      (tag_size == 2 && val_bytes > 65535) ||
+      (tag_size == 3 && val_bytes > 16777215))
       throw Invalid_Argument("append_tls_length_value: value too large");
 
    for(size_t i = 0; i != tag_size; ++i)


Test patch:
diff --git a/src/tests/test_tests.cpp b/src/tests/test_tests.cpp
index 2402cf46e..66babbbc1 100644
--- a/src/tests/test_tests.cpp
+++ b/src/tests/test_tests.cpp
@@ -38,7 +38,7 @@ class Test_Tests final : public Test
             {
             Test::Result test_result(testcase_name);
             test_result.test_throws("doesn't throw", []() { });
-            verify_failure("test_throws 1", result, test_result);
+            verify_failure("test_throws", result, test_result);
             }
 
             {
@@ -144,42 +144,21 @@ class Test_Tests final : public Test
             Test::Result test_result(testcase_name);
             test_result.test_throws("test_throws", "expected msg",
                                     []() { throw std::runtime_error("not the message"); });
-            verify_failure("test_throws 2", result, test_result);
+            verify_failure("test_throws", result, test_result);
             }
 
             {
             Test::Result test_result(testcase_name);
             test_result.test_throws("test_throws", "expected msg",
                                     []() { throw std::string("not even a std::exception"); });
-            verify_failure("test_throws 3", result, test_result);
+            verify_failure("test_throws", result, test_result);
             }
 
             {
             Test::Result test_result(testcase_name);
             test_result.test_throws("test_throws", "expected msg",
                                     []() { ; });
-            verify_failure("test_throws 4", result, test_result);
-            }
-
-            {
-            Test::Result test_result(testcase_name);
-            test_result.test_throws<std::invalid_argument>("test_throws", "expected msg",
-                                    []() { throw std::runtime_error("expected msg"); });
-            verify_failure("test_throws 5", result, test_result);
-            }
-
-            {
-            Test::Result test_result(testcase_name);
-            test_result.test_throws<std::invalid_argument>("test_throws",
-                                    []() { throw std::runtime_error("expected msg"); });
-            verify_failure("test_throws 6", result, test_result);
-            }
-
-            {
-            Test::Result test_result(testcase_name);
-            test_result.test_no_throw("test_no_throw",
-                                    []() { throw std::runtime_error("boom!"); });
-            verify_failure("test_throws 7", result, test_result);
+            verify_failure("test_throws", result, test_result);
             }
 
 #if defined(BOTAN_HAS_BIGINT)
diff --git a/src/tests/tests.cpp b/src/tests/tests.cpp
index 9680abb14..17785c4a2 100644
--- a/src/tests/tests.cpp
+++ b/src/tests/tests.cpp
@@ -85,52 +85,46 @@ void Test::Result::note_missing(const std::string& whatever)
       }
    }
 
-bool Test::Result::ThrowExpectations::check(const std::string& test_name, Test::Result& result)
+bool Test::Result::test_throws(const std::string& what, const std::function<void ()>& fn)
    {
-   m_consumed = true;
-
    try
       {
-      m_fn();
-      if(!m_expect_success)
-         return result.test_failure(test_name + " failed to throw expected exception");
+      fn();
+      return test_failure(what + " failed to throw expected exception");
       }
-   catch(const std::exception& ex)
+   catch(std::exception& e)
       {
-      if(m_expect_success)
-         return result.test_failure(test_name + " threw unexpected exception: " + ex.what());
-      if(m_expected_exception_type.has_value() && m_expected_exception_type.value() != typeid(ex))
-         return result.test_failure(test_name +  " threw unexpected exception: " + ex.what());
-      if(m_expected_message.has_value() && m_expected_message.value() != ex.what())
-         return result.test_failure(test_name + " threw exception with unexpected message (expected: '" +
-                                    m_expected_message.value() + "', got: '" + ex.what() + "')");
+      return test_success(what + " threw exception " + e.what());
       }
    catch(...)
       {
-      if(m_expect_success || m_expected_exception_type.has_value() || m_expected_message.has_value())
-         return result.test_failure(test_name + " threw unexpected unknown exception");
+      return test_success(what + " threw unknown exception");
       }
-
-   return result.test_success(test_name + " behaved as expected");
-   }
-
-bool Test::Result::test_throws(const std::string& what, const std::function<void ()>& fn)
-   {
-   return ThrowExpectations(fn).check(what, *this);
    }
 
 bool Test::Result::test_throws(const std::string& what, const std::string& expected, const std::function<void ()>& fn)
    {
-   return ThrowExpectations(fn)
-          .expect_message(expected)
-          .check(what, *this);
-   }
-
-bool Test::Result::test_no_throw(const std::string& what, const std::function<void ()>& fn)
-   {
-   return ThrowExpectations(fn)
-          .expect_success()
-          .check(what, *this);
+   try
+      {
+      fn();
+      return test_failure(what + " failed to throw expected exception");
+      }
+   catch(std::exception& e)
+      {
+      if(expected == e.what())
+         {
+         return test_success(what + " threw exception " + e.what());
+         }
+      else
+         {
+         return test_failure(what + " failed to throw an exception with the expected text:\n  Expected: " + expected +
+                             "\n  Got: " + e.what());
+         }
+      }
+   catch(...)
+      {
+      return test_failure(what + " failed to throw an exception with the expected text:\n  Expected: " + expected);
+      }
    }
 
 bool Test::Result::test_success(const std::string& note)
diff --git a/src/tests/tests.h b/src/tests/tests.h
index 009586c72..e09bb0dca 100644
--- a/src/tests/tests.h
+++ b/src/tests/tests.h
@@ -22,8 +22,6 @@
 #include <unordered_map>
 #include <vector>
 #include <optional>
-#include <variant>
-#include <typeindex>
 
 namespace Botan {
 
@@ -43,19 +41,13 @@ namespace Botan_Tests {
    using Botan::BigInt;
 #endif
 
-class Test_Error : public Botan::Exception
+class Test_Error final : public Botan::Exception
    {
    public:
       explicit Test_Error(const std::string& what) : Exception("Test error", what) {}
       Botan::ErrorType error_type() const noexcept override { return Botan::ErrorType::Unknown; }
    };
 
-class Test_Aborted final : public Test_Error
-   {
-   public:
-      explicit Test_Aborted(const std::string& what) : Test_Error(what) {}
-   };
-
 class Test_Options
    {
    public:
@@ -247,18 +239,6 @@ class Test
                return test_eq(what, expr, expected);
                }
 
-            /**
-             * Require a condition, throw Test_Aborted otherwise
-             * Note: works best when combined with CHECK scopes!
-             */
-            void require(const std::string& what, bool expr, bool expected = true)
-               {
-               if(!confirm(what, expr, expected))
-                  {
-                  throw Test_Aborted("test aborted, because required condition was not met");
-                  }
-               }
-
             template<typename T>
             bool test_is_eq(const T& produced, const T& expected)
                {
@@ -435,75 +415,11 @@ class Test
                               expected.data(), expected.size());
                }
 
-         private:
-            class ThrowExpectations
-               {
-               public:
-                  ThrowExpectations(std::function<void()> fn) : m_fn(std::move(fn)), m_expect_success(false), m_consumed(false) {}
-                  ThrowExpectations(const ThrowExpectations&) = delete;
-                  ThrowExpectations& operator=(const ThrowExpectations&) = delete;
-                  ThrowExpectations(ThrowExpectations&&) = default;
-                  ThrowExpectations& operator=(ThrowExpectations&&) = default;
-
-                  ~ThrowExpectations() { BOTAN_ASSERT_NOMSG(m_consumed); }
-
-                  ThrowExpectations& expect_success()
-                     {
-                     BOTAN_ASSERT_NOMSG(!m_expected_message && !m_expected_exception_type);
-                     m_expect_success = true;
-                     return *this;
-                     }
-
-                  ThrowExpectations& expect_message(const std::string& message)
-                     {
-                     BOTAN_ASSERT_NOMSG(!m_expect_success);
-                     m_expected_message = message;
-                     return *this;
-                     }
-
-                  template <typename ExT>
-                  ThrowExpectations& expect_exception_type()
-                     {
-                     BOTAN_ASSERT_NOMSG(!m_expect_success);
-                     m_expected_exception_type = typeid(ExT);
-                     return *this;
-                     }
-
-                  bool check(const std::string& test_name, Test::Result& result);
-
-               private:
-                  std::function<void()>          m_fn;
-                  bool                           m_expect_success;
-                  std::optional<std::string>     m_expected_message;
-                  std::optional<std::type_index> m_expected_exception_type;
-                  bool                           m_consumed;
-               };
-
-         public:
             bool test_throws(const std::string& what, const std::function<void ()>& fn);
 
             bool test_throws(const std::string& what, const std::string& expected,
                              const std::function<void ()>& fn);
 
-            bool test_no_throw(const std::string& what, const std::function<void ()>& fn);
-
-            template <typename ExceptionT>
-            bool test_throws(const std::string& what, const std::function<void ()>& fn)
-               {
-               return ThrowExpectations(fn)
-                      .expect_exception_type<ExceptionT>()
-                      .check(what, *this);
-               }
-
-            template <typename ExceptionT>
-            bool test_throws(const std::string& what, const std::string& expected, const std::function<void ()>& fn)
-               {
-               return ThrowExpectations(fn)
-                      .expect_exception_type<ExceptionT>()
-                      .expect_message(expected)
-                      .check(what, *this);
-               }
-
             void set_ns_consumed(uint64_t ns)
                {
                m_ns_taken = ns;
@@ -611,78 +527,34 @@ class TestClassRegistration
 #define BOTAN_REGISTER_TEST(category, name, Test_Class)                 \
    const TestClassRegistration<Test_Class> reg_ ## Test_Class ## _tests(category, name)
 
-typedef Test::Result(*test_fn)();
-typedef std::vector<Test::Result> (*test_fn_vec)();
+typedef Test::Result (*test_fn)();
 
 class FnTest : public Test
    {
-   private:
-      using TestFnVariant = std::variant<test_fn, test_fn_vec>;
-
-      template <typename TestFn>
-      std::vector<TestFnVariant> make_variant_vector(TestFn fn)
-         {
-         using T = std::decay_t<decltype(fn)>;
-         static_assert(std::is_same_v<T, test_fn> || std::is_same_v<T, test_fn_vec>,
-                       "functions passed to BOTAN_REGISTER_TEST_FN must either return a "
-                       "single Test::Result or a std::vector of Test::Result");
-         return { fn };
-         }
-
-      template <typename TestFn, typename... TestFns>
-      std::vector<TestFnVariant> make_variant_vector(const TestFn& fn, const TestFns& ...fns)
-         {
-         auto functions = make_variant_vector(fns...);
-         functions.emplace_back(fn);
-         return functions;
-         }
-
    public:
-      template <typename... TestFns>
-      FnTest(TestFns... fns) : m_fns(make_variant_vector(fns...)) {}
+      FnTest(test_fn fn) : m_fn(fn) {}
 
       std::vector<Test::Result> run() override
          {
-         std::vector<Test::Result> result;
-
-         for(auto fn_variant = m_fns.crbegin(); fn_variant != m_fns.crend(); ++fn_variant)
-            {
-            std::visit([&](auto&& fn)
-               {
-               using T = std::decay_t<decltype(fn)>;
-               if constexpr(std::is_same_v<T, test_fn>)
-                  {
-                  result.emplace_back(fn());
-                  }
-               else
-                  {
-                  const auto results = fn();
-                  result.insert(result.end(), results.begin(), results.end());
-                  }
-               },
-            *fn_variant);
-            }
-
-         return result;
+         return {m_fn()};
          }
 
    private:
-      std::vector<TestFnVariant> m_fns;
+      test_fn m_fn;
    };
 
 class TestFnRegistration
    {
    public:
-      template <typename... TestFns>
-      TestFnRegistration(const std::string& category, const std::string& name, TestFns... fn)
+      TestFnRegistration(const std::string& category, const std::string& name, test_fn fn)
          {
-         auto test_maker = [=]() -> std::unique_ptr<Test> { return std::make_unique<FnTest>(fn...); };
+         auto test_maker = [=]() -> std::unique_ptr<Test> { return std::make_unique<FnTest>(fn); };
          Test::register_test(category, name, test_maker);
          }
    };
 
-#define BOTAN_REGISTER_TEST_FN(category, name, ...) \
-   static const TestFnRegistration reg_ ## fn_name(category, name, __VA_ARGS__)
+#define BOTAN_REGISTER_TEST_FN(category, name, fn_name) \
+   const TestFnRegistration reg_ ## fn_name(category, name, fn_name)
 
 class VarMap
    {
@@ -784,58 +656,6 @@ class Text_Based_Test : public Test
       std::vector<uint64_t> m_cpu_flags;
    };
 
-/**
- * This is a convenience wrapper to write small self-contained and in particular
- * exception-safe unit tests. If some (unexpected) exception is thrown in one of
- * the CHECK-scopes, it will fail the particular test gracefully with a human-
- * understandable failure output.
- *
- * Example Usage:
- *
- * ```
- * std::vector<Test::Result> test_something()
- *    {
- *    return
- *       {
- *       CHECK("some unit test name", [](Test::Result& r)
- *          {
- *          r.confirm("some observation", 1+1 == 2);
- *          }),
- *       CHECK("some other unit test name", [](Test::Result& r)
- *          {
- *          // ...
- *          })
- *       };
- *    }
- *
- * BOTAN_REGISTER_TEST_FN("some_category", "some_test_name", test_something);
- * ```
- */
-template<typename FunT>
-Test::Result CHECK(const char* name, FunT check_fun)
-   {
-   Botan_Tests::Test::Result r(name);
-
-   try
-      {
-      check_fun(r);
-      }
-   catch(const Botan_Tests::Test_Aborted&)
-      {
-      // pass, failure was already noted in the responsible `require`
-      }
-   catch(const std::exception& ex)
-      {
-      r.test_failure("failed unexpectedly", ex.what());
-      }
-   catch(...)
-      {
-      r.test_failure("failed with unknown exception");
-      }
-
-   return r;
-   }
-
 }
 
 #endif