Issue with assignment of an object_iterator to const_object_iterator using empty object

[danielaparker]

Discussed in #368

^{Originally posted by awolff64 May 19, 2022}
We have a strange problem that only happens with XCode on OSX. It works fine on Window and Linux.

We have an empty object assigning the result of a find() to a const_object_iterator. This should be the object's end().
The subsequent check for end() is not true and later on we run into problems because we assume the iterator is valid.

   Json::const_object_iterator it;

it = m_Definition.find("Min");

    if (it == m_Definition.object_range().end())
    {
          ...
    }

Our findings:
The iterator returned by find has has_value_ set to false.
The constructed const_iterator hat a has value of true.
=> comparison fails

We changed the random_access_iterator_wrapper (with less than partial understanding) to init has_value_ from the the other:

template <class Iter, class=typename std::enable_if<!std::is_same<Iter,Iterator>::value && std::is_convertible<Iter,Iterator>::value>::type> random_access_iterator_wrapper(const random_access_iterator_wrapper<Iter>& other) : it_(other.it_), has_value_(other.has_value_) { }
=> now it works.

Can anybody help on this?

[danielaparker]

You're right.

jsoncons object iterators are based on std::vector iterators, i.e. std::vector<key_value_type>::iterator and std::vector<key_value_type>::const_iterator. jsoncons empty objects require begin/end iterators that compare equal, but empty objects don't have an instance of a std::vector<key_value_type>. We first experimented with using default constructed std::vector<key_value_type> iterators for empty objects, that seemed to work on all linux and windows environments, but tests revealed that for some xcode versions, the default constructed std::vector iterators weren't being initialized, and there went equality.

So we introduced random_access_iterator_wrapper whose sole purpose was to have well specified default constructed iterators. Unfortunately we didn't include a test case involving non-const to const iterator conversion. The constructor that you highlight is the one that performs that conversion, and as you found, has_value_ needs to be initialized from its argument. The effect of that bug is that it == m_Definition.object_range().end() ends up comparing two default constructed iterators, which is precisely the issue that random_access_iterator_wrapper was meant to solve.

The fix is on master and will go in patch release 0.168.7.