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

Serialized value not always can be parsed. #329

Closed
vitalybuka opened this issue Oct 13, 2016 · 6 comments
Closed

Serialized value not always can be parsed. #329

vitalybuka opened this issue Oct 13, 2016 · 6 comments

Comments

@vitalybuka
Copy link

vitalybuka commented Oct 13, 2016

I assume that if value was parsed without exception it should be serializable and parsables again.

    std::stringstream s;
    s << json::parse("22e2222");  // I'd expect exception here
    try {
      auto j2 = json::parse(s.str()); 
    } catch (const std::invalid_argument&) { 
      std::cerr << std::string{data, data + size} << " -> "
      << s.str() << "\n";
      assert(0);
    }
@nlohmann
Copy link
Owner

nlohmann commented Oct 13, 2016

I think the underlying problem is the following:

The code

json j = json::parse("22e2222");
std::cerr << j << std::endl;

outputs inf which is not valid JSON.

This contradicts the paragraph

This implementation does exactly follow this approach, as it uses double precision floating-point numbers. Note values smaller than -1.79769313486232e+308 and values greater than 1.79769313486232e+308 will be stored as NaN internally and be serialized to null.

from the documentation.

It seems as if at some point, the overflow is not detected.

nlohmann added a commit that referenced this issue Oct 13, 2016
@nlohmann nlohmann self-assigned this Oct 13, 2016
@nlohmann
Copy link
Owner

For floating-point numbers, the constructor looks like this:

basic_json(const number_float_t val) noexcept
    : m_type(value_t::number_float), m_value(val)
{
    // replace infinity and NAN by null
    if (not std::isfinite(val))
    {
        m_type = value_t::null;
        m_value = json_value();
    }

    assert_invariant();
}

The replacement of infinity/NAN by null was not done when a value was parsed from a string. Commit d910672 fixes this.

I am currently torn how to cope with numbers that exceed the limits of number_float_t. RFC 7159 states

This specification allows implementations to set limits on the range and precision of numbers accepted. Since software that implements IEEE 754-2008 binary64 (double precision) numbers is generally available and widely used, good interoperability can be achieved by implementations that expect no more precision or range than these provide, in the sense that implementations will approximate JSON numbers within the expected precision.

The question is how to deal with numbers that do not fit to double precision. "set limits on the range and numbers accepted" could be interpreted that numbers outside this range should be rejected which would mean throwing an exception when such a number is encountered.

What do you think?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Oct 13, 2016
@mikea
Copy link

mikea commented Oct 13, 2016

Throwing an exception sounds like a reasonable thing to do.

@vitalybuka
Copy link
Author

+1 for exception

@nlohmann
Copy link
Owner

I think an exception is the way to go (both in the constructor and in the parser). I shall implement this change in the 3.0.0 release as it changes the public API.

@nlohmann nlohmann removed the state: please discuss please discuss the issue or vote for your favorite option label Oct 15, 2016
@nlohmann nlohmann added this to the Release 3.0.0 milestone Oct 15, 2016
nlohmann added a commit that referenced this issue Mar 12, 2017
- If an overflow occurs during parsing a number from a JSON text, an
exception (std::out_of_range for the moment, to be replaced by a
user-defined exception #244) is thrown so that the overflow is detected
early and roundtripping is guaranteed.
- NaN and INF floating-point values can be stored in a JSON value and
are not replaced by null. That is, the basic_json class behaves like
double in this regard (no exception occurs). However, NaN and INF are
serialized to “null”.
- Adjusted test cases appropriately.
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 12, 2017
@nlohmann
Copy link
Owner

This issue is fixed with 8feaf8d as follows:

  • If an overflow occurs during parsing a number from a JSON text, an exception (std::out_of_range for the moment, to be replaced by a user-defined exception Use user-defined exceptions #244) is thrown so that the overflow is detected early and roundtripping is guaranteed.
  • NaN and INF floating-point values can be stored in a JSON value and are not replaced by null. That is, the basic_json class behaves like double in this regard (no exception occurs). However, NaN and INF are serialized to “null”.
  • Adjusted test cases appropriately.

Waiting for Travis to complete. Then this issue can be closed and the different semantics can be described in the wiki.

@nlohmann nlohmann removed the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 12, 2017
nlohmann added a commit that referenced this issue Mar 14, 2017
Since #329, NaN and inf numbers do not yield an exception, but are
stored internally and are dumped as “null”. This commit adjusts the
fuzz testers to deal with this special case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants