json
json copied to clipboard
Check we don't overflow when casting down integers during parsing
Hi,
I've faced the following issue when using this library: I stored some numbers in small integers (because I expect to use values in a small interval in practice) but the static_cast
made during parsing lead to invalid values because of the overflow.
I believe it would be nice to have this runtime check to notice possible overflows at runtime.
I am a bit concerned that adding overflow exceptions (though correct) are changing the behavior of the library in a breaking (and maybe also surprising) way.
🔴 Amalgamation check failed! 🔴
The source code has not been amalgamated. @ArnaudBienner Please read and follow the Contribution Guidelines.
coverage: 100.0%. remained the same when pulling da5800cdccebdd531a09399c89df13ea6e57de21 on ArnaudBienner:develop into 8c391e04fe4195d8be862c97f38cfe10e2a3472e on nlohmann:develop.
Thank you all for your feedback :)
Turns out the check was (by chance) working in the sample case I wrote for unit testing :( I should have been more careful and more exhaustive in my testing, sorry about that. I've added a few more unit tests.
In addition to fixing the check, I also change the code to:
- Not perform the check if the source type is smaller than the destination type: as discussed, this is not needed in this case. I didn't managed to use
enable_if
to have two different version of that function. In my PR I'm usingif constexpr
to achieve a similar effect, but this is a C++17 feature, even if I know this project is C++11: this was just to make a point. I can either drop it completely, or if you have a way to define macros to support multiple C++/compilers versions and features, we can do this as well. - Always perform the check in case the source number is floating points: in this case the
sizeof
check is not relevant - Ignore the check when dealing with infinity numbers: I realized when running unit tests this was already handled specifically in the code elsewhere