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

RPC methods expecting floats\doubles could be called with an integral type which would throw an exception #18

Closed
eniv opened this issue Feb 25, 2021 · 7 comments

Comments

@eniv
Copy link

eniv commented Feb 25, 2021

Hi,
I'm not sure, if there's much that could be done about this issue, but implicit type deductions for numbers in JSON for modern C++ are problematic when a handler expects a float\double param and the RPC is using an integral param. I will try to explain with an example:

json foo (const float b) {
  ...
}

JsonRpc2Server rpc_handler;
rpc_handler.Add("foo", GetHandle(foo), {"bar"});

If foo is invoked like this: {"method":"foo","params":{"bar":10}}, then nlohmann would determine that 10 is an unsigned number and the dispatcher will throw (invalid parameter: must be float, but is unsigned integer for parameter "bar") when it tries to invoke the method.

The only workaround I've found so far is to setup foo to expect a string, and then invoke it like this: {"method":"foo","params":{"bar":"10"}}.
If anyone has other\better ideas, I'd be happy to hear those.
Thanks.

@cinemast
Copy link
Contributor

cinemast commented Mar 1, 2021

Hi!

Well, I think if the method accepts float, the request param should also be a float. Maybe I am not getting your point here. But if you send "bar": 10.0, everything should be fine right?

@eniv
Copy link
Author

eniv commented Mar 1, 2021

Hi,
If you sent "bar":10.0 the same thing would happen.

nlohmann: "The parser will store number values as integers where possible. 1.0 will hence be treated as 1. JSON itself has no concept of "number type", so there is no way to "force " a library to choose a specific type."

JSON for modern C++ parser determines that the number is an integer\unsigned or float based on the strategy explained here.

For a case like this, I believe you would have to explicitly cast the de-serialized JSON value to a float as opposed to using the default type mapping.

@cinemast
Copy link
Contributor

cinemast commented Mar 1, 2021

Ah, now I get it. Thanks for the additional info. The specific cast makes sense to me yes. Could you open a PR?

@eniv
Copy link
Author

eniv commented Mar 1, 2021

I ended up adding a specific check for this case in typemapper.hpp, such that check_param_type doesn't fall through to else if (x.type() != expectedType)
I determined that it works with some basic testing.

@cinemast
Copy link
Contributor

image

I am trying to reproduce this behavior, but a literal of 3.0 still gives me type float.
Are you using a different version of nlohmann?

Could you provide an example snippet?

@eniv
Copy link
Author

eniv commented Mar 13, 2021

I updated to the latest version of nlohmann a few weeks when I ran into this.
To clarify further:

std::string s = R"({"x": 3.0})";
json s_json = json::parse(s);

does get interpreted as float, but some serializers (for example JSON.stringify in JavaScript) will serialize 3.0 to just a 3. In which case the equivalent test case would be:

std::string s = R"({"x": 3})";
json s_json = json::parse(s);

which gets interpreted as an integer, but is also a valid float number.

@cinemast
Copy link
Contributor

Thanks, I think I covered it here: 3d8a8fe#diff-282a49776a9b2c47f2c8b946be3e71b07b31c446312a53997755c8b404c5285cR211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants