Correct handling of corner-cases involving custom iterators:

- Under some conditions, comparing `ReadViewFwdRange::iterators`
  for equality could derefence an empty `std::unique_ptr` which
  will result in a crash.
- Misuse of the `equal` API could result in a `std::bad_cast`
  exception being thrown from when iterating transactions or
  SLEs from the `OpenView`, `RawStateTable` and `Ledger` classes.

src/ripple/app/ledger/Ledger.cpp

@@ -100,8 +100,9 @@ public:
     bool
     equal(base_type const& impl) const override
     {
-        auto const& other = dynamic_cast<sles_iter_impl const&>(impl);
-        return iter_ == other.iter_;
+        if (auto const p = dynamic_cast<sles_iter_impl const*>(&impl))
+            return iter_ == p->iter_;
+        return false;
     }
 
     void
@@ -148,8 +149,9 @@ public:
     bool
     equal(base_type const& impl) const override
     {
-        auto const& other = dynamic_cast<txs_iter_impl const&>(impl);
-        return iter_ == other.iter_;
+        if (auto const p = dynamic_cast<txs_iter_impl const*>(&impl))
+            return iter_ == p->iter_;
+        return false;
     }
 
     void

src/ripple/ledger/detail/ReadViewFwdRange.h

@@ -126,7 +126,7 @@ public:
     private:
         ReadView const* view_ = nullptr;
         std::unique_ptr<iter_base> impl_;
-        boost::optional<value_type> mutable cache_;
+        std::optional<value_type> mutable cache_;
     };
 
     static_assert(std::is_nothrow_move_constructible<iterator>{}, "");

src/ripple/ledger/detail/ReadViewFwdRange.ipp

@@ -52,11 +52,12 @@ auto
 ReadViewFwdRange<ValueType>::iterator::operator=(iterator const& other)
     -> iterator&
 {
-    if (this == &other)
-        return *this;
-    view_ = other.view_;
-    impl_ = other.impl_ ? other.impl_->copy() : nullptr;
-    cache_ = other.cache_;
+    if (this != &other)
+    {
+        view_ = other.view_;
+        impl_ = other.impl_ ? other.impl_->copy() : nullptr;
+        cache_ = other.cache_;
+    }
     return *this;
 }
 
@@ -65,9 +66,13 @@ auto
 ReadViewFwdRange<ValueType>::iterator::operator=(iterator&& other) noexcept
     -> iterator&
 {
-    view_ = other.view_;
-    impl_ = std::move(other.impl_);
-    cache_ = std::move(other.cache_);
+    if (this != &other)
+    {
+        view_ = other.view_;
+        impl_ = std::move(other.impl_);
+        cache_ = std::move(other.cache_);
+    }
+
     return *this;
 }
 
@@ -76,7 +81,11 @@ bool
 ReadViewFwdRange<ValueType>::iterator::operator==(iterator const& other) const
 {
     assert(view_ == other.view_);
-    return impl_->equal(*other.impl_);
+
+    if (impl_ != nullptr && other.impl_ != nullptr)
+        return impl_->equal(*other.impl_);
+
+    return impl_ == other.impl_;
 }
 
 template <class ValueType>
@@ -107,7 +116,7 @@ auto
 ReadViewFwdRange<ValueType>::iterator::operator++() -> iterator&
 {
     impl_->increment();
-    cache_ = boost::none;
+    cache_.reset();
     return *this;
 }
 

src/ripple/ledger/impl/OpenView.cpp

@@ -45,8 +45,9 @@ public:
     bool
     equal(base_type const& impl) const override
     {
-        auto const& other = dynamic_cast<txs_iter_impl const&>(impl);
-        return iter_ == other.iter_;
+        if (auto const p = dynamic_cast<txs_iter_impl const*>(&impl))
+            return iter_ == p->iter_;
+        return false;
     }
 
     void

src/ripple/ledger/impl/RawStateTable.cpp

@@ -61,9 +61,13 @@ public:
     bool
     equal(base_type const& impl) const override
     {
-        auto const& other = dynamic_cast<sles_iter_impl const&>(impl);
-        assert(end1_ == other.end1_ && end0_ == other.end0_);
-        return iter1_ == other.iter1_ && iter0_ == other.iter0_;
+        if (auto const p = dynamic_cast<sles_iter_impl const*>(&impl))
+        {
+            assert(end1_ == p->end1_ && end0_ == p->end0_);
+            return iter1_ == p->iter1_ && iter0_ == p->iter0_;
+        }
+
+        return false;
     }
 
     void

src/ripple/shamap/SHAMap.h

@@ -564,7 +564,13 @@ private:
     pointer item_ = nullptr;
 
 public:
-    const_iterator() = default;
+    const_iterator() = delete;
+
+    const_iterator(const_iterator const& other) = default;
+    const_iterator&
+    operator=(const_iterator const& other) = default;
+
+    ~const_iterator() = default;
 
     reference
     operator*() const;
@@ -578,7 +584,7 @@ public:
 
 private:
     explicit const_iterator(SHAMap const* map);
-    const_iterator(SHAMap const* map, pointer item);
+    const_iterator(SHAMap const* map, std::nullptr_t);
     const_iterator(SHAMap const* map, pointer item, SharedPtrNodeStack&& stack);
 
     friend bool
@@ -586,16 +592,16 @@ private:
     friend class SHAMap;
 };
 
-inline SHAMap::const_iterator::const_iterator(SHAMap const* map)
-    : map_(map), item_(nullptr)
+inline SHAMap::const_iterator::const_iterator(SHAMap const* map) : map_(map)
 {
-    auto temp = map_->peekFirstItem(stack_);
-    if (temp)
+    assert(map_ != nullptr);
+
+    if (auto temp = map_->peekFirstItem(stack_))
         item_ = temp->peekItem().get();
 }
 
-inline SHAMap::const_iterator::const_iterator(SHAMap const* map, pointer item)
-    : map_(map), item_(item)
+inline SHAMap::const_iterator::const_iterator(SHAMap const* map, std::nullptr_t)
+    : map_(map)
 {
 }
 
@@ -622,8 +628,7 @@ SHAMap::const_iterator::operator->() const
 inline SHAMap::const_iterator&
 SHAMap::const_iterator::operator++()
 {
-    auto temp = map_->peekNextItem(item_->key(), stack_);
-    if (temp)
+    if (auto temp = map_->peekNextItem(item_->key(), stack_))
         item_ = temp->peekItem().get();
     else
         item_ = nullptr;

src/test/shamap/SHAMap_test.cpp

@@ -37,7 +37,6 @@ static_assert(!std::is_move_constructible<SHAMap>{}, "");
 static_assert(!std::is_move_assignable<SHAMap>{}, "");
 
 static_assert(std::is_nothrow_destructible<SHAMap::const_iterator>{}, "");
-static_assert(std::is_default_constructible<SHAMap::const_iterator>{}, "");
 static_assert(std::is_copy_constructible<SHAMap::const_iterator>{}, "");
 static_assert(std::is_copy_assignable<SHAMap::const_iterator>{}, "");
 static_assert(std::is_move_constructible<SHAMap::const_iterator>{}, "");