Merge pull request #722 from AntelopeIO/check_negative

[1.0.1] Verify index >= 0 in apply_diff and fix warnings
AntelopeIO · Sep 10, 2024 · e5c0ee0 · e5c0ee0
2 parents eb39cdc + 2f18e3c
commit e5c0ee0
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 14 deletions.
diff --git a/libraries/libfc/include/fc/container/ordered_diff.hpp b/libraries/libfc/include/fc/container/ordered_diff.hpp
@@ -26,10 +26,15 @@ namespace fc {
  * @param Container container type for ordered diff and for diff_result
  */
 template <typename T, typename SizeType = size_t, template<typename Y, typename...> typename Container = std::vector>
-requires std::equality_comparable<T> && std::random_access_iterator<typename Container<T>::iterator>
+requires std::equality_comparable<T>
+         && std::random_access_iterator<typename Container<T>::iterator>
+         && std::is_unsigned_v<SizeType>
+         && std::is_unsigned_v<typename Container<T>::size_type>
+         && (std::numeric_limits<typename Container<T>::size_type>::max() >= std::numeric_limits<SizeType>::max())
 class ordered_diff {
 public:
    using size_type = SizeType;
+   using container_size_type = typename Container<T>::size_type;
 
    struct diff_result {
       Container<SizeType>                remove_indexes;
@@ -99,23 +104,33 @@ class ordered_diff {
       return result;
    }
 
-   /// @param diff the diff_result created from diff(source, target), apply_diff(std::move(source), diff_result) => target
+   /// @param diff_in (input) the diff_result created from diff(source, target), apply_diff(std::move(source), diff_result) => target
    /// @param container the source of diff(source, target) to modify using the diff_result to produce original target
    /// @return the modified container now equal to original target
    template <typename X>
    requires std::same_as<std::decay_t<X>, diff_result>
-   static Container<T> apply_diff(Container<T>&& container, X&& diff) {
+   static Container<T> apply_diff(Container<T>&& container, X&& diff_in) {
+      X diff = std::forward<X>(diff_in);
+
       // Remove from the source based on diff.remove_indexes
-      std::ptrdiff_t offset = 0;
-      for (SizeType index : diff.remove_indexes) {
-         FC_ASSERT(index + offset < container.size(), "diff.remove_indexes index ${idx} + offset ${o} not in range ${s}",
-                   ("idx", index)("o", offset)("s", container.size()));
-         container.erase(container.begin() + index + offset);
-         --offset;
+      for (container_size_type i = 0; i < diff.remove_indexes.size(); ++i) {
+         FC_ASSERT(i == 0 || diff.remove_indexes[i] > diff.remove_indexes[i-1],
+                   "diff.remove_indexes not strictly monotonically increasing: current index ${c}, previous index ${p}",
+                   ("c", diff.remove_indexes[i])("p", diff.remove_indexes[i-1]));
+
+         assert(diff.remove_indexes[i] >= i);
+         auto updated_index = diff.remove_indexes[i] - i;
+         FC_ASSERT(updated_index < container.size(), "diff.remove_indexes index ${idx} - i ${i} not in range ${s}",
+                   ("idx", diff.remove_indexes[i])("i", i)("s", container.size()));
+         container.erase(container.begin() + updated_index);
       }
 
       // Insert into the source based on diff.insert_indexes
-      for (auto& [index, value] : diff.insert_indexes) {
+      for (container_size_type i = 0; i < diff.insert_indexes.size(); ++i) {
+         FC_ASSERT(i == 0 || diff.insert_indexes[i].first > diff.insert_indexes[i-1].first,
+                   "diff.insert_indexes not strictly monotonically increasing: current index ${c}, previous index ${p}",
+                   ("c", diff.insert_indexes[i].first)("p", diff.insert_indexes[i-1].first));
+         auto& [index, value] = diff.insert_indexes[i];
          FC_ASSERT(index <= container.size(), "diff.insert_indexes index ${idx} not in range ${s}",
                    ("idx", index)("s", container.size()));
          container.insert(container.begin() + index, std::move(value));

diff --git a/libraries/libfc/test/test_ordered_diff.cpp b/libraries/libfc/test/test_ordered_diff.cpp
@@ -38,15 +38,15 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try {
    { // All elements removed
       vector<char> source = {'a', 'b', 'c', 'd', 'e'};
       vector<char> target;
-      auto result = ordered_diff<char, int>::diff(source, target);
-      source = ordered_diff<char, int>::apply_diff(std::move(source), result);
+      auto result = ordered_diff<char, unsigned int>::diff(source, target);
+      source = ordered_diff<char, unsigned int>::apply_diff(std::move(source), result);
       BOOST_TEST(source == target);
    }
    { // All elements removed, size 1
       vector<char> source = {'a'};
       vector<char> target;
-      auto result = ordered_diff<char, int>::diff(source, target);
-      source = ordered_diff<char, int>::apply_diff(std::move(source), result);
+      auto result = ordered_diff<char, unsigned int>::diff(source, target);
+      source = ordered_diff<char, unsigned int>::apply_diff(std::move(source), result);
       BOOST_TEST(source == target);
    }
    { // All elements inserted