beached / daw_json_link

Fast, convenient JSON serialization and parsing in C++
https://beached.github.io/daw_json_link/
Boost Software License 1.0
460 stars 30 forks source link

Optional member before a vector class member produces seg fault #357

Closed marcelomatus closed 1 year ago

marcelomatus commented 1 year ago

When an optional member is declared before a vector class member, such as version in System:

struct Bus { int uid {}; std::string name; std::optional voltage; };

struct System { std::string name; std::optional version; // ** here is the problem, optional before vector of classes std::vector buses; std::vector buses_res; };

Then the parsing from_json fails with seg fault or memory corruption when the optional field "version" is not present in the input.

beached commented 1 year ago

Do you have the mapping you used, and what version of the library?

beached commented 1 year ago

I just tested with

#include <daw/json/daw_json_link.h>

#include <iostream>
#include <optional>
#include <string>
#include <string_view>
#include <tuple>
#include <vector>

struct voltage {
    double num;
};

struct Bus {
    int uid{ };
    std::string name;
    std::optional<voltage> volts;
};

struct buses {
    int id;
};

struct buses_res {
    int id;
};

struct System {
    std::string name;
    // ** here is the problem, optional before vector of classes
    std::optional<std::string> version;
    std::vector<buses> b;
    std::vector<buses_res> br;
};

namespace daw::json {
    template<>
    struct json_data_contract<voltage> {
        static constexpr char const num[] = "num";
        using type = json_member_list<json_number<num>>;

        static constexpr auto to_json_data( voltage const &v ) {
            return std::forward_as_tuple( v.num );
        }
    };
    template<>
    struct json_data_contract<Bus> {
        static constexpr char const uid[] = "uid";
        static constexpr char const name[] = "name";
        static constexpr char const volts[] = "volts";
        using type = json_member_list<json_number<uid, int>, json_string<name>,
                                      json_class_null<volts, voltage>>;

        static constexpr auto to_json_data( Bus const &b ) {
            return std::forward_as_tuple( b.uid, b.name, b.volts );
        }
    };
    template<>
    struct json_data_contract<buses> {
        static constexpr char const id[] = "id";
        using type = json_member_list<json_number<id, int>>;

        static constexpr auto to_json_data( buses const &b ) {
            return std::forward_as_tuple( b.id );
        }
    };
    template<>
    struct json_data_contract<buses_res> {
        static constexpr char const id[] = "id";
        using type = json_member_list<json_number<id, int>>;

        static constexpr auto to_json_data( buses_res const &b ) {
            return std::forward_as_tuple( b.id );
        }
    };
    template<>
    struct json_data_contract<System> {
        static constexpr char const name[] = "name";
        static constexpr char const version[] = "version";
        static constexpr char const b[] = "b";
        static constexpr char const br[] = "br";
        using type =
          json_member_list<json_string<name>, json_string_null<version>,
                           json_array<b, buses>, json_array<br, buses_res>>;

        static constexpr auto to_json_data( System const &s ) {
            return std::forward_as_tuple( s.name, s.version, s.b, s.br );
        }
    };
} // namespace daw::json

int main( ) {
    std::string_view json_doc = R"json(
{
    "name": "foo",
    "version": null,
    "b": [{
        "id": 1234
    }],
    "br": [{
        "id": 5678
    }]
}
)json";

    auto s = daw::json::from_json<System>( json_doc );
    auto new_doc = daw::json::to_json( s );
    std::cout << new_doc << '\n';
}

And no errors, maybe diff data/mappings or an old version?

marcelomatus commented 1 year ago

The problem is when no version key is present, ie std::string_view json_doc = R"json( { "name": "foo", "b": [{ "id": 1234 }], "br": [{ "id": 5678 }] }

beached commented 1 year ago

you can test the code above, it is in the v3 branch in tests/src/issue_357_test.cpp, but I am unable to replicate and it's a common pattern with the input you provided. I am vaguely remembering an issue around out of order members that might be related, but it's fixed.

marcelomatus commented 1 year ago

I just modified your test a little, by using the more complex Bus class and not providing the "version" key in the data

include <daw/json/daw_json_link.h>

include

include

include

include

include

include

include

struct voltage { double num; };

struct Bus { int uid{ }; std::string name; std::optional volts; };

struct buses_res { int id; };

struct System { std::string name; // ** here is the problem, optional before vector of classes std::optional version; std::vector b; std::vector br; };

namespace daw::json { template<> struct json_data_contract { static constexpr char const num[] = "num"; using type = json_member_list<json_number>;

            static constexpr auto to_json_data( voltage const &v ) {
                    return std::forward_as_tuple( v.num );
            }
    };
    template<>
    struct json_data_contract<Bus> {
            static constexpr char const uid[] = "uid";
            static constexpr char const name[] = "name";
            static constexpr char const volts[] = "volts";
            using type = json_member_list< //
              json_number<uid, int>, json_string<name>,
              json_class_null<volts, voltage>>;

            static constexpr auto to_json_data( Bus const &b ) {
                    return std::forward_as_tuple( b.uid, b.name, b.volts );
            }
    };
    template<>
    struct json_data_contract<buses_res> {
            static constexpr char const id[] = "id";
            using type = json_member_list<json_number<id, int>>;

            static constexpr auto to_json_data( buses_res const &b ) {
                    return std::forward_as_tuple( b.id );
            }
    };
    template<>
    struct json_data_contract<System> {
            static constexpr char const name[] = "name";
            static constexpr char const version[] = "version";
            static constexpr char const b[] = "b";
            static constexpr char const br[] = "br";
            using type =
              json_member_list<json_string<name>,
                               json_string_null<version, std::optional<std::string>>,
                               json_array<b, Bus>, json_array<br, buses_res>>;

            static constexpr auto to_json_data( System const &s ) {
                    return std::forward_as_tuple( s.name, s.version, s.b, s.br );
            }
    };

} // namespace daw::json

int main( ) { // "version": null,

    std::string_view json_doc = R"json(

{ "name": "foo", "b": [{ "uid": 1234, "name": "b1" }], "br": [{ "id": 5678 }] } )json";

    auto s = daw::json::from_json<System>( json_doc );
    auto new_doc = daw::json::to_json( s );
    std::cout << new_doc << '\n';

}

And it produces the seg fault. If you put back the "version", it works.

marcelomatus commented 1 year ago

Sorry, I better attach the file for better reading. I am using the last version from the repository, I just did a git pull and it still produces a seg fault. It seems ti tries to build a Bus object when reading the vector, but it passes a null pointer to the constructor.

daw_json_test.cpp.zip

beached commented 1 year ago

Thanks for finding this. This is due to when the parser must skip a member, for array members it stores the count of the elements so that later there is opportunity to reduce the allocations to 1, instead of more. The method used to count for array elements is wrong and will be disabled for now. It generally doesn't matter much, even in code that used it, as there are other ways to lower allocations. They will perform as the non-skipped members do.

v3.10.0 will have the fix, https://github.com/beached/daw_json_link/pull/358 is pushing it to the release branch after it is done testing

beached commented 1 year ago

Should be fixed https://github.com/beached/daw_json_link/releases/tag/v3.10.0