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

Negative infinity round-trips as invalid JSON #1405

Open
Paril opened this issue May 18, 2022 · 4 comments
Open

Negative infinity round-trips as invalid JSON #1405

Paril opened this issue May 18, 2022 · 4 comments

Comments

@Paril
Copy link

Paril commented May 18, 2022

Describe the bug
inf and -inf don't round trip if useSpecialFloats is false.

To Reproduce
Steps to reproduce the behavior:

	Json::Value testJson(Json::arrayValue);
	testJson[0] = 25.0;
	testJson[1] = 0.0;
	testJson[2] = -std::numeric_limits<float>::infinity();
	Json::StreamWriterBuilder builder;
	builder["indentation"] = "\t";
        // both false/true are incorrect
	//builder["useSpecialFloats"] = true;
	const std::unique_ptr<Json::StreamWriter> writer(builder.newStreamWriter());
	std::stringstream ss;
	writer->write(testJson, &ss);
	Json::Reader reader;
	Json::Value json;

	bool is_valid = reader.parse(ss, json);
	// is_valid == false
        // errors list -1e+9999 as invalid number

Expected behavior
Should at least parse rather than hard fail.

Desktop (please complete the following information):

  • OS: Windows
  • Ninja version 1.10.2
@BillyDonahue
Copy link
Contributor

I'm a little stuck on this one.

The description says: "inf and -inf don't round trip if useSpecialFloats is false."
But then the repro steps have:

    // both false/true are incorrect
    // builder["useSpecialFloats"] = true;

What do you mean by incorrect there?

So it's hard to know what your expectation is. You have formatted a -infinity without special floats, so you are essentially formatting an unrepresentable number. There's no right answer available to jsoncpp here.

Expected behavior: "Should at least parse rather than hard fail."

What value would you expect the parse to produce? The -infinity isn't recoverable because that's not what's in the json.

@Paril
Copy link
Author

Paril commented May 21, 2022

I eventually realized that I was using the builder that didn't allow options on the reading, so I ended up fixing that part - I can read/write inf & -inf with that enabled and all is well in my use-case.

I think there's still an issue, though; if you write an inf out, it writes a value that it then can't read back. I know it's technically not the same value as inf, but I'd expect the library to still be able to read back in something it wrote instead of throwing a hard failure that you can't ignore (none of the options allow you to 'read around' this bad value). That's the part that seems like a bug or unintended behavior. I think people would prefer it error on writing, so that they can catch this as it's going out, rather than having a value in the written JSON that the reader can't read (if you're not reading the JSON back in immediately and instead saving for later, you wouldn't know this timebomb was waiting until you got to reading it, and it's not immediately clear where +1e+9999 would have come from)

@BillyDonahue
Copy link
Contributor

I agree with everything you're saying. I would prefer that it error out on writing.
If a Value is unrepresentable I don't think it's correct to make up a representation like that.
I wouldn't know how to fix it exactly. It's also possible to expand the reader so that it reads 1e+9999 as inf, which would allow it to read what it wrote, but that doesn't sound right.

@Paril
Copy link
Author

Paril commented May 23, 2022

It is strange that it ends up using 1e+9999 and -1e+9999 as infinity - is this value even representable with a double? FWIW, Chrome does seem to handle this same value as infinity:

Not sure about Firefox or other parsers, but it seems like this is a bit of a pseudo-standard.

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