Skip to content

Large floats not accepted anymore #1349

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

Open
terokinnunen opened this issue Oct 28, 2021 · 3 comments
Open

Large floats not accepted anymore #1349

terokinnunen opened this issue Oct 28, 2021 · 3 comments

Comments

@terokinnunen
Copy link
Contributor

terokinnunen commented Oct 28, 2021

Describe the bug
Version 1.9.2 no longer accepts 1+e9999 (or 2e+308 and anything above), regardless of allowSpecialFloats setting.

To Reproduce
Steps to reproduce the behavior:

#include <iostream>
#include <json/json.h>
#include <sstream>

int main(int argc, const char* argv[])
{
    Json::CharReaderBuilder reader;
    reader["allowSpecialFloats"] = true;  // Makes no difference

    std::string s = "1e+9999";
    Json::Value val;
    std::string errors;
    std::stringstream json(s);
    if (Json::parseFromStream(reader, json, &val, &errors)) {
        std::cout << s << " -> " << val.asDouble() << "\n";
    } else {
        std::cout << errors;
    }
    return 0;
}

Expected behavior

  • 1e+9999 -> inf

Actual behavior:

* Line 1, Column 1
  '1e+9999' is not a number.

Desktop (please complete the following information):

  • OS: Fedora 34
  • Meson version 0.59.1
  • Ninja version 1.10.2

Additional context

  • Version 1.9.1 accepts value 1e+9999 as infinite, regardless of allowSpecialFloats setting.
  • Version 1.9.2 no longer accepts 2e+308 or above, regardless of allowSpecialFloats setting.

Specifically breaking change seems to be commit 645cd04 "Number fixes (#1053)".

@cdunn2001
Copy link
Contributor

Sorry, I don't like to change/break old behavior. This was our mistake.

But now, I wonder whether we can recover the old behavior without reverting that large PR. Somehow, we need big numbers to map to inf. However, did the old behavior map even bigger, really huge numbers to inf also? If not, would it be ok if it did?

In that case, there is probably a relatively simply way to use inf for all large numbers. Could you submit a PR for that? Otherwise, I'm not sure when we would find time for this.

@terokinnunen
Copy link
Contributor Author

I'll give it a try to fix.

baylesj pushed a commit that referenced this issue Dec 15, 2021
Return 1.9.1 functionality where values too large to fit in
double are converted to positive or negative infinity.

Commit 645cd04 changed functionality so that large floats cause
parse error, while version 1.9.1 accepted them as infinite.
This is problematic because writer outputs infinity values
as `1e+9999`, which could no longer be parsed back.

Fixed also legacy Reader even though it did not parse large values
even before breaking change, due to problematic output/parse asymmetry.

`>>` operator sets value to numeric_limits::max/lowest value if
representation is too large to fit to double. [1][2] In macos
value appears to be parsed to infinity.

> | value in *val*           | description |
> |--------------------------|-------------|
> | numeric_limits::max()    | The sequence represents a value too large for the type of val |
> | numeric_limits::lowest() | The sequence represents a value too large negative for the type of val |

[1] https://www.cplusplus.com/reference/istream/istream/operator%3E%3E/
[2] https://www.cplusplus.com/reference/locale/num_get/get/

Signed-off-by: Tero Kinnunen <[email protected]>

Co-authored-by: Tero Kinnunen <[email protected]>
ccwanggl pushed a commit to CLNBG/jsoncpp that referenced this issue Jul 7, 2023
…e-parsers#1353)

Return 1.9.1 functionality where values too large to fit in
double are converted to positive or negative infinity.

Commit 645cd04 changed functionality so that large floats cause
parse error, while version 1.9.1 accepted them as infinite.
This is problematic because writer outputs infinity values
as `1e+9999`, which could no longer be parsed back.

Fixed also legacy Reader even though it did not parse large values
even before breaking change, due to problematic output/parse asymmetry.

`>>` operator sets value to numeric_limits::max/lowest value if
representation is too large to fit to double. [1][2] In macos
value appears to be parsed to infinity.

> | value in *val*           | description |
> |--------------------------|-------------|
> | numeric_limits::max()    | The sequence represents a value too large for the type of val |
> | numeric_limits::lowest() | The sequence represents a value too large negative for the type of val |

[1] https://www.cplusplus.com/reference/istream/istream/operator%3E%3E/
[2] https://www.cplusplus.com/reference/locale/num_get/get/

Signed-off-by: Tero Kinnunen <[email protected]>

Co-authored-by: Tero Kinnunen <[email protected]>
@chacha21
Copy link

chacha21 commented Nov 12, 2024

Not fixed by 1.9.6

In the following code , for a token of 1e+9999 (written by jsoncpp itself), (is >> value) will let value equal to 0 and will ultimately fall in the error case

bool OurReader::decodeDouble(Token& token, Value& decoded) {
  double value = 0;
  IStringStream is(String(token.start_, token.end_));
  if (!(is >> value)) {
    if (value == std::numeric_limits<double>::max())
      value = std::numeric_limits<double>::infinity();
    else if (value == std::numeric_limits<double>::lowest())
      value = -std::numeric_limits<double>::infinity();
    else if (!std::isinf(value))
      return addError(
          "'" + String(token.start_, token.end_) + "' is not a number.", token);
  }
  decoded = value;
  return true;
}

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