danielaparker / jsoncons

A C++, header-only library for constructing JSON and JSON-like data formats, with JSON Pointer, JSON Patch, JSON Schema, JSONPath, JMESPath, CSV, MessagePack, CBOR, BSON, UBJSON
https://danielaparker.github.io/jsoncons
Other
726 stars 164 forks source link

Issue with assignment of an object_iterator to const_object_iterator using empty object #372

Closed danielaparker closed 2 years ago

danielaparker commented 2 years ago

Discussed in https://github.com/danielaparker/jsoncons/discussions/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 ::value && std::is_convertible::value>::type> random_access_iterator_wrapper(const random_access_iterator_wrapper& other) : it_(other.it_), has_value_(other.has_value_) { } ` => now it works. Can anybody help on this?
danielaparker commented 2 years ago

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.