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
715 stars 163 forks source link

Crash in jsoncons::ojson::merge_or_update with empty source data #435

Closed jkalmar closed 1 year ago

jkalmar commented 1 year ago

Describe the bug

Having some json parsed in ojson and then merging some other empty ojson leads to crash.

I updated recently to latest commit. It was working without any crash in commit: 7928fcca2

Enumerate the steps to reproduce the bug

  1. create 1 ojson, fill some data
  2. create another ojson, do not fill any data
  3. call merge_or_update on the 1st ojson
  4. crash

Include a small, self-contained example if possible

This leads to crash:

   jsoncons::ojson oj;
   jsoncons::ojson oj_source;

   oj[ "k" ] = "v";

   oj.merge_or_update( oj_source );

This is working:

   jsoncons::ojson oj;
   jsoncons::ojson oj_source;

   oj[ "k" ] = "v";
   oj_source[ "k2" ] = 2;

   oj.merge_or_update( oj_source );

The backtrace:

#0  0x000055c308a6a67d in std::vector<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char> > >, std::allocator<jsoncons::key_value<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char> > > > >::begin (this=0xd78b207b00000408) at /usr/include/c++/9/bits/stl_vector.h:818
#1  0x000055c308a88398 in jsoncons::order_preserving_json_object<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char> >, std::vector>::begin (this=0xd78b207b00000400) at /home/test/jsoncons/json_object.hpp:1236
#2  0x000055c308c70d3a in jsoncons::order_preserving_json_object<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char> >, std::vector>::merge_or_update (this=0x55c30b994450, source=...) at /home/test/jsoncons/json_object.hpp:1609
#3  0x000055c308c6ee1f in jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char> >::merge_or_update (this=0x7ffe7fa972c0, source=...)
    at /home/test/jsoncons/basic_json.hpp:5127

It seems that in the older version (commit 7928fcca2) there was a method object_value() that initialized the ojson internal storage if it was empty. This method was used in merge_or_update that prevented this crash in case empty ojson object is passed as source.

        object& object_value()
        {
            switch (storage_kind())
            {
                case json_storage_kind::empty_object_value:
                    create_object_implicitly();
                    JSONCONS_FALLTHROUGH;
                case json_storage_kind::object_value:
                    return cast<object_storage>().value();
                default:
                    JSONCONS_THROW(json_runtime_error<std::domain_error>("Bad object cast"));
                    break;
            }
        }

In the new commit (dbfc31c53191ebf54d87a665a0eb437f177cb861) there is no such method and the merge_or_update only uses method cast to retrieve the internal data storage. This returns some invalid data and leads to crash.

(gdb) fr
#2  0x000055c308c70d3a in jsoncons::order_preserving_json_object<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char> >, std::vector>::merge_or_update (this=0x55c30b994450, source=...) at /home/test/jsoncons/json_object.hpp:1609
1609                for (auto it = source.begin(); it != source.end(); ++it)
(gdb) p source
$5 = (const jsoncons::order_preserving_json_object<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char> >, std::vector> &) <error reading variable>

What compiler, architecture, and operating system?

What jsoncons library version?

danielaparker commented 1 year ago

Thanks for reporting the issue, should be fixed on master.

jkalmar commented 1 year ago

Tested with 3d50397.

Thanks for quick fix.