Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

to/from_msgpack only works with standard typization #1021

Closed
phyz777 opened this issue Mar 22, 2018 · 6 comments
Closed

to/from_msgpack only works with standard typization #1021

phyz777 opened this issue Mar 22, 2018 · 6 comments
Assignees
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON confirmed solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@phyz777
Copy link

phyz777 commented Mar 22, 2018

The to/from_msgpack functions only work for standard typization of nlohmann::basic_json.

Using nlohmann::basic_json<std::map, std::vector, std::string, bool, std::int32_t, std::uint32_t, float> produces the following error:

parse error at 1472: expected a MessagePack string; last byte: 0x68

The error occurs when I put an object into a msgpack, send it over the network, then reconstruct the object using from_msgpack. When I use nlohmann::json instead, everything works fine.

System is linux, compiler is g++, version 7.2.1, nlohmann::json is version 3.1.2.

@nlohmann
Copy link
Owner

Could you provide a small example that triggers the error?

@nlohmann nlohmann added aspect: binary formats BSON, CBOR, MessagePack, UBJSON state: needs more info the author of the issue needs to provide more details labels Mar 22, 2018
@phyz777
Copy link
Author

phyz777 commented Mar 24, 2018

I am working on it.
Meanwhile, I came across something similar. I am not even sure whether this is one is a bug:

#include <iostream>
#include "json.hpp"

// using json=nlohmann::json;
using json=nlohmann::basic_json<std::map, std::vector, std::string, bool, std::int32_t, std::uint32_t, float>;

int main()
{
    double Value2 = 1000.0;

    json j;
    j["Key1"] = "Value1";
    j["Key2"] = Value2;

    std::vector<uint8_t> v = json::to_msgpack(j);
    json k = json::from_msgpack(v);

    std::cout << k.dump(4) << std::endl;

    return 0;
}

Is nlohmann::json supposed to safely convert types where necessary? This one throws nlohmann::detail::parse_error

@nlohmann nlohmann added confirmed and removed state: needs more info the author of the issue needs to provide more details labels Mar 27, 2018
@nlohmann
Copy link
Owner

The issue is that while the library respects the provided number types in the serialization, it does not add the correct prefixes. For instance, it serializes the float value correctly with 4 bytes, but adds a 0xcb byte (float 64) as type prefix.

To fix this issue, the serialization must derive the MessagePack (and most probably also CBOR and UBJSON) type prefix from the templated type. I am not sure about a nice way, but I think functions like std::is_floating_point would be my friend.

@nlohmann nlohmann self-assigned this Mar 28, 2018
nlohmann added a commit that referenced this issue Mar 28, 2018
nlohmann added a commit that referenced this issue Mar 28, 2018
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 28, 2018
@nlohmann
Copy link
Owner

I opened branch https://github.com/nlohmann/json/tree/feature/issue1021 which supports float as floating-point type for all binary formats.

@phyz777 Could you check if this fixes your issue?

@nlohmann
Copy link
Owner

nlohmann commented Apr 4, 2018

@phyz777 Could you check if this fixes your issue?

@nlohmann nlohmann added this to the Release 3.1.3 milestone Apr 8, 2018
@nlohmann
Copy link
Owner

nlohmann commented Apr 8, 2018

I merged the fix. If there is still an issue, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON confirmed solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants